refactor: use upstream AutofillDriverFactory diffs #31676
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Experimental PR related to upgrades. Specifically, #31555 includes https://chromium-review.googlesource.com/c/chromium/src/+/3241451 which removes the getAllFrames() API that our AutofillDriverFactory uses. Upstream had similar getAllFrames() calls and removed them in 2019 with https://chromium-review.googlesource.com/c/chromium/src/+/1826885 which lazy-creates the autofill drivers instead of doing it (indirectly) in the factory's constructor. This PR tries to do the same. This also changes the AutofillDriver class to remove its `receiver_` param; however, it was private and unused so it's possibly OK to remove. The changes here are wrapped in #ifdefs & have // TESTING markers since this is experimental to see if this plays nicely with CI autofill tests.
previously was done in AutofillDriver constructor
ckerr
commented
Nov 2, 2021
ckerr
added
semver/patch
deepak1556
approved these changes
Nov 2, 2021
...maybe? Looking through the two codebases with that goal in mind, it's not clear to me how much work that would take. I will take that task on as a follow-up and merge this in the interim, since right now my main goal is to move the roll forward. |
ckerr
deleted the
refactor/sync-autofill-driver-creation-with-upstream
branch
|
No Release Notes |
ckerr
added a commit
that referenced
this pull request
Nov 3, 2021
* refactor: use upstream AutofillDriverFactory diffs Update our copy of AutofillDriver and AutofillDriverFactory to match chromium.
t57ser
pushed a commit
to t57ser/electron
that referenced
this pull request
Jan 25, 2022
* refactor: use upstream AutofillDriverFactory diffs Update our copy of AutofillDriver and AutofillDriverFactory to match chromium.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Update our AutofillDriver and AutofillDriverFactory code to match upstream.
This was prompted by an upstream change that removes API that we were using. Specifically, #31555 includes https://chromium-review.googlesource.com/c/chromium/src/+/3241451 removes the getAllFrames() API that our AutofillDriverFactory uses.
Our code appears to be derived from components/autofill/content/browser/content_autfill_driver*[cc,h], whose identical use of getAllFrames() removed in 2019 by https://chromium-review.googlesource.com/c/chromium/src/+/1826885. In that change, the upstream factory was changed to create and bind the autofill drivers on command. This PR tries to follow that approach in our derived factory, copying the relevant upstream code from our current roll (97.0.4689.0).
This is a medium-sized refactor, so I've split it out into its own PR (a) to make CI / manual testing possible (since this isn't the only break in the Chromium roll) and (b) for easier code review.
Looks like these files haven't been changed in a substantial way in awhile, so CC'ing @brenca who did the last major work on them, but review welcome from anyone including @electron/wg-upgrades
Notes: none.