1827770 - Remove QueryContext.view and pass it to onEngagement() instead

Status: RESOLVED FIXED

(Firefox :: Address Bar, task, P2)

Description

[Drew Willcoxon :adw]

QueryContext.view is only used in the onEngagement() implementations of a few providers, plus one use in TelemetryEvent. The TelemetryEvent use can be replaced with this._controller.view. The onEngagement() uses can be replaced with a new view param for onEngagement().

This amounts to reverting the parts of bug 1800810 D162182 that added the view to the context, these files in particular:

• UrlbarInput.sys.mjs
• UrlbarProviderExtension.sys.mjs
• UrlbarUtils.sys.mjs
• head.js

The onEngagement() params are getting a little out of control and should at least be changed to an options object. I'm not sure if this bug is the right place for that, maybe. A deeper follow up should re-evaluate the params altogether (e.g., something nicer than details, seeing if isPrivate is really necessary since context.isPrivate also exists).

(This bug is spun out from bug 1827762, see comments in D174941)

Comment 1

[Marco Bonardo [:mak]]

Looking into this while investigating recent leaks, this may indeed cause unexpected cycles.
My plan is to replace the last onEngagement argument from window to controller.
I may also remove _isPrivate, as suggested.
Hopefully the head.js change won't cause headaches.

Comment 2

[Marco Bonardo [:mak]]

we can't use queryContext.isPrivate for now ,because some onEngagement calls pass undefined for the queryContext... I supect those are the "start" engagement, but we should investigate whether it's possible to always pass a context, because that looks error prone. But it may be in a follow-up, we can use controller.input.isPrivate anyway.

Comment 3

[Marco Bonardo [:mak]]

Attachment: Bug 1827770 - Remove QueryContext.view and pass the controller to onEngagement() instead. r=daleharvey

Comment 4

[Marco Bonardo [:mak]]

Attachment: Bug 1827770 - Remove isPrivate argument from onEngagement() since it can be inferred. r=daleharvey

We can't use context.isPrivate because sometimes context is undefined.
I filed https://bugzilla.mozilla.org/show_bug.cgi?id=1841762 about that.

Depends on D182771

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7e4c261cfee4
Remove QueryContext.view and pass the controller to onEngagement() instead. r=daleharvey
https://hg.mozilla.org/integration/autoland/rev/3a88b97290a0
Remove isPrivate argument from onEngagement() since it can be inferred. r=daleharvey

Comment 6

[Iulian Moraru]

https://hg.mozilla.org/mozilla-central/rev/7e4c261cfee4
https://hg.mozilla.org/mozilla-central/rev/3a88b97290a0