commit 8db17789870cb1222d5059567298d4d88b81c41a
author Ken Buchanan <kenrb@chromium.org> Tue Sep 13 20:04:01 2022
committer Chromium LUCI CQ <chromium-scoped@luci-project-accounts.iam.gserviceaccount.com> Tue Sep 13 20:04:01 2022
tree 6b0ac9cfb58b551119473a5a844e8aa47ffeeeb7
parent fd1c54efce16935c33aefe1bafce04d3fcf553ac

[WebAuthn] Refactor PasswordFormPredictionWaiter to split closures

PasswordFormPredictionWaiter uses a BarrierClosure to wait for
WebAuthn credentials and server predictions, which are retrieved
asynchronously. When both arrive, the callback is called and
PasswordFormManager initiates a Fill.

There are a couple of problems with this:
1. If one of server predictions or WebAuthn credentials arrives,
   but not both, the callback will not be called.
2. If ProcessServerPredictions is called twice with empty credentials,
   it will invoke the BarrierClosure twice, and the WebAuthn credential
   callback will be cancelled.

The latter is an observed problem causing WebAuthn credentials to not
be available after page refresh.

Both of these issues are resolved if we keep the outstanding closures
independent. This change does the following:
1. Instead of a BarrierClosure, the waiter class issues a separate
   OnceClosure for each waiting task.
2. The completion method is invoked as soon as all closures have been
   called, if the timer has not yet expired.
3. After the timer expires, any single closure invocation will cause
   the completion method to be called, initiating a Fill.
4. ProcessServerPredictions no longer does anything, including
   calling the closure, if there are no server predictions arriving.
   This change restores older behaviour, changed in
   https://crrev.com/1042080.

Since there is a OnceClosure per waiting task, this resolves the
problem of a single task fulfilling the BarrierClosure by itself.

This also resolves problem (1) described above because if
server predictions or WebAuthn credentials arrive after the timeout,
but the other never resolves, a Fill will still be initiated at
that point.

One consequence is that an unfulfilled closure is left outstanding
indefinitely (the lifetime of the page), but this is true already,
since an earlier change made it so the expiring timer no longer
invalidates weak pointers. However, there isn't an upper limit on
how long it can take for these credentials to arrive, so this is
unavoidable.

Fixed: 1361033
Change-Id: Ic31fa62de1892b20bc7e561c0a5e335200e9cdfb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3892651
Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org>
Commit-Queue: Ken Buchanan <kenrb@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1046529}