commit d36c87462cf13bbbd438cb1586f2d0b2fcd66970
author Muyao Xu <muyaoxu@google.com> Thu Jan 25 17:47:27 2024
committer Chromium LUCI CQ <chromium-scoped@luci-project-accounts.iam.gserviceaccount.com> Thu Jan 25 17:47:27 2024
tree 8dd50d2113f1fe55300f765a5fdf3a4ffdd7d211
parent 700c24b6043878cda45b9bd9d473f0e5e9f80750

Revert "[Autofill] Simplify ContentAutofillDriver initialization"

This reverts commit 3b90869541da848d1791a4ca8b44bc0fd4af8347.

Reason for revert: reverted CL might have caused build failures crbug.com/1521832

Original change's description:
> [Autofill] Simplify ContentAutofillDriver initialization
>
> The DriverInitHooks have two jobs:
> (1) create a {Browser,Android}AutofillManager;
> (2) call some setters on AutofillAgent.
>
> Since the introduction of ContentAutofillClient, the DriverInitHook
> can be conveniently turned into a member function of the client.
>
> This CL introduces *two* new methods in CAC:
> (1) CreateManager(), to be called by CAD's constructor.
> (2) InitAgent(), to be called by the CAD factory.
>
> The point in time they're called is important:
> - CreateManager() is called before OnContentAutofillDriverCreated(),
>   which is necessary for TestAutofillManagerInjector;
> - InitAgent() is called after  OnContentAutofillDriverCreated(),
>   which avoids trouble with double binding mojo remotes when
>   TestAutofillDriverInjector creates a new driver.
>
> Alternatives I considered are:
> - Calling CreateManager() from the factory. This would avoid the
>   potential problem of the manager calling into the driver during
>   the driver's construction. Rejected because this requires
>   CAD::set_autofill_manager(), and since driver and manager
>   have to point to each other, there will always be some kind of
>   chicken-egg problem.
> - Calling InitAgent() from CAD's constructor. Rejected because
>   - tests that mock an AutofillAgent would attempt to bind *two*
>     remotes, which isn't allowed;
>   - unbinding and rebinding remotes
>     - makes writing tests harder;
>     - seems not to work for the renderer -> browser remote.
> - Calling CreateManager() and InitAgent() or only InitAgent() from
>   a virtual CAD::Init() function, which is be called by the CAD
>   factory. Tests would override Init() and only call CreateDriver(),
>   not InitAgent(). Rejected because that'd make writing tests harder.
>
>   The DriverInitHook model worked similarly, but it probably worked
>   out of luck: the DriverInitHooks didn't actually call
>   GetAutofillAgent() in most tests, which happened to avoid the
>   double bind in all relevant tests.
>
> Bug: 1200511
> Change-Id: I339a8b0e7531a76395e5e50dba16f77e3385f56a
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5224630
> Reviewed-by: Jan Keitel <jkeitel@google.com>
> Commit-Queue: Christoph Schwering <schwering@google.com>
> Cr-Commit-Position: refs/heads/main@{#1252040}

Bug: 1200511
Change-Id: I4885522ac5e9a8d0f4f9475856907a22d796dd1d
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5238008
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Owners-Override: Muyao Xu <muyaoxu@google.com>
Auto-Submit: Muyao Xu <muyaoxu@google.com>
Reviewed-by: Muyao Xu <muyaoxu@google.com>
Commit-Queue: Muyao Xu <muyaoxu@google.com>
Cr-Commit-Position: refs/heads/main@{#1252189}