2053266 - [IPC Urlbar] Make quicksuggest provider engagement work over the actor message path

Status: ASSIGNED

(Firefox :: Address Bar, task)

Description

[Dão Gottwald [:dao]]

On the IPC Urlbar message path (browser.urlbar.ipc.chromeMessagePassing), provider engagement handling runs parent-side against results/details reconstructed from the actor wire, so several quicksuggest behaviors do not work even though they work on the default in-process path:

• Show-less-frequently: the command does not drop from the open menu at the cap, because view.invalidateResultMenuCommands was not reachable through the view proxy and the refreshed commands only mutated the parent's throwaway result copy.
• Dismissal: a dismissal does not register in the Rust store, because a Rust suggestion's UniFFI suggestionObject class is stripped by structured-clone, so dismissRustSuggestion's instanceof check rejects it; and isResultDismissed could not report dismissal for a wire-reconstructed result.
• Contextual opt-in: the allow/dismiss/learn_more commands never ran parent-side, because onEngagement read the command from details.element (a DOM node that does not cross the actor boundary).
• Impression/abandonment hooks (impression counting, the quicksuggest impression ping) do not fire, because notifyEngagementChange grouped visible results from the parent's view, which has none on the message path.

The default in-process path stays behavior-neutral throughout. Part of Phase 6 (bug 2051959).

Comment 1

[Dão Gottwald [:dao]]

Attachment: Bug 2053266 - [IPC Urlbar] Phase 6: Refresh a suggestion's menu commands over the message path at the show-less-frequently cap. r?mbeier!

At the cap, SuggestProvider.handleShowLessFrequently refreshed the result's baked
menu commands and cleared the view's command cache so the command drops from the
open menu. On the message path both steps were lost: the refresh mutated the
parent's throwaway result copy, and view.invalidateResultMenuCommands wasn't
reachable through the ViewProxy.

Route it through the proxy like acknowledgeFeedback: invalidateResultMenuCommands
now takes (resultId, commands), locates the row by id, replaces its result's baked
commands, and drops that cache entry; handleShowLessFrequently passes the freshly
computed commands (plain objects that survive the wire). Direct-path behavior is
unchanged -- the row's result is the same instance the provider holds.

Comment 2

[Dão Gottwald [:dao]]

Attachment: Bug 2053266 - [IPC Urlbar] Phase 6: Make quicksuggest dismissal register over the message path. r?mbeier!

Parent-side engagement handling ran against the wire-reconstructed result. For a
Rust-backed suggestion, structured-clone strips the UniFFI Suggestion class, so
dismissRustSuggestion's instanceof check rejected the object and the dismissal
silently no-oped.

Resolve the engagement result to the parent's authoritative result by id before
dispatching to providers, so onEngagement (and its dismissResult/removeResult)
operate on the live object -- mirroring removeResult's id match. Also make
isResultDismissed fall back to the dismissal key when suggestionObject isn't a
live Suggestion, so a dismissal check on a wire-reconstructed result reports
correctly.

Await the onQueryResultRemoved round-trip in the mdn dismissal test before
reading the acknowledgment tip, which is async over the wire. Direct-path
behavior is unchanged (the id resolves to the same instance).

Comment 3

[Dão Gottwald [:dao]]

Attachment: Bug 2053266 - [IPC Urlbar] Phase 6: Carry the visible results over the message path so provider impression and abandonment hooks fire. r?mbeier!

notifyEngagementChange grouped the visible results by provider from
controller.view.visibleResults to drive providers' onImpression/onAbandonment
hooks. On the message path the parent's view proxy has no results, so those
hooks never fired -- e.g. impression counting and the quicksuggest impression
ping.

Ship the results shown at engagement (captured content-side, and already used to
build the Glean event) in the engagement wire payload, reconstruct them
parent-side, and pass them to notifyEngagementChange, which prefers them over the
view. The direct path is unchanged: it passes the same view-derived results the
function read before.

Comment 4

[Dão Gottwald [:dao]]

Attachment: Bug 2053266 - [IPC Urlbar] Phase 6: Route the contextual opt-in commands over the message path. r?mbeier!

The contextual opt-in's onEngagement read the picked command from the name
attribute of details.element, but the DOM element doesn't cross the actor
boundary, so allow/dismiss/learn_more never ran parent-side.

Give the controls a data-command (like other picked commands) so the command
rides details.selType over the wire, and read it there. The commands now route
through the menu-command pick path, which doesn't navigate, so close the view
explicitly when one resolves the prompt -- the generic pick path did this before.

The test awaits the impression round-trip (async over the wire now) and checks
the opt-in is no longer offered via a re-query rather than the parent's last
query context, which the child doesn't hold on the message path.