1893228 - Move address bar expandos set on browsers to the #browserStates WeakMap

Status: RESOLVED FIXED

(Firefox :: Address Bar, task, P3)

Description

[Marco Bonardo [:mak]]

Code is setting multiple address bar related expandos on browsers, that could be merged into a WeakMap in UrlbarInput to make the code a bit more coherent and allow for better code reuse. I added #browserStates in Bug 1848715 that is an initial attempt at a better approach, but may likely be further improved.
Some examples of things we could move: _urlMetaData, _searchModesByBrowser.
Other things to be investigated: urlbarChangeTracker, _userTypedValue, _urlbarFocused

Comment 1

[Moritz Beier [:mbeier]]

When moving urlbarChangeTracker to the WeakMap in UrlbarInput.sys.mjs, browser-custom-element.js would need a UrlbarInput as a dependency because of the following line:
https://searchfox.org/mozilla-central/rev/0529464f0d2981347ef581f7521ace8b7af7f7ac/toolkit/content/widgets/browser-custom-element.js#705

Any ideas how to solve this?

Comment 2

[Moritz Beier [:mbeier]]

Attachment: Bug 1893228 - Move address bar expandos set on browsers to the #browserStates WeakMap. r?dao

Comment 3

[Marco Bonardo [:mak]]

(In reply to Moritz Beier [:mbeier] from comment #1)

When moving urlbarChangeTracker to the WeakMap in UrlbarInput.sys.mjs, browser-custom-element.js would need a UrlbarInput as a dependency because of the following line:
https://searchfox.org/mozilla-central/rev/0529464f0d2981347ef581f7521ace8b7af7f7ac/toolkit/content/widgets/browser-custom-element.js#705

The second list in comment 0 was intended to be just a pointer for things to investigate, I didn't check whether they could effectively be moved safely. Probably the changer helpers could be moved to UrlbarInput, and the per-browser values kept in the weakmap. Where we do calls like this.mBrowser.urlbarChangeTracker.finishedLoad() we could instead dispatch a custom event or invoke a gURLBar._on_browser_finished_load(this.mBrowser);

That said, we don't have to necessarily fix everything here, it would be fine to handle the safest cases and split more complex ones to follow-up bugs, as well as split each move into its own bug/revision.

Comment 4

[Marco Bonardo [:mak]]

Fwiw I also don't understand all the design complexity behind urlbarChangeTracker, it was implemented in Bug 1241085, with the intent of having something self-documenting, there may be a simpler solution. It's worth evaluating alternatives with Dao (e.g. custom events).

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9b0ce32b35b3
Move address bar expandos set on browsers to the #browserStates WeakMap. r=dao,tabbrowser-reviewers

Comment 6

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/9b0ce32b35b3