2052875 - [IPC Urlbar] Give UrlbarResult a stable id to match results across the actor boundary without relying on row order

Status: ASSIGNED

(Firefox :: Address Bar, task)

Description

[Dão Gottwald [:dao]]

On the Urlbar actor message path, results are reconstructed from the wire (UrlbarResult.fromWire), losing object identity. UrlbarParentController.removeResult and UrlbarView.acknowledgeFeedback locate and validate the target row by result.rowIndex, which (a) relies on the parent's results array order matching the view's row order and (b) can't verify that a wire-reconstructed result truly corresponds to the row at that index.

Giving UrlbarResult a stable id -- assigned when the result is finalized and serialized by toWire/fromWire -- would let both match by id: robust against reordering and identity-independent, and would let the view remove/acknowledge by id end-to-end.

Comment 1

[Dão Gottwald [:dao]]

Attachment: Bug 2052875 - [IPC Urlbar] Give UrlbarResult a stable id. r?mbeier!

Results cross the actor boundary by structured-clone, which drops object
identity, so the message path can't reliably match a reconstructed result back
to the parent's context entry or the view's row. Assign each result a stable id
when it is finalized in the parent and carry it over the wire, so the following
commit can match by id rather than by array position or identity.

Behavior-neutral: nothing reads the id yet.

Comment 2

[Dão Gottwald [:dao]]

Attachment: Bug 2052875 - [IPC Urlbar] Match results by id across the actor boundary. r?mbeier!

Result removal and dismissal/feedback acknowledgment located a result's context
entry and view row by identity or rowIndex. That only worked because the
parent's results order matched the view's row order -- an invariant the message
path can break (a re-query racing a notification), and one acknowledgeFeedback
had to weaken its staleness guard around. Match by the stable id instead, so the
removal/acknowledgment path no longer depends on ordering or identity, and
acknowledgeFeedback's post-await correspondence check is restored.

The removal notification now carries the result id rather than a row index, and
the view locates the row by id. To verify: on the direct path this is behavior-
neutral (same instance, so id-match == indexOf and the reselection is
unchanged); the message-path reselection now clamps to the remaining row count,
so removing the last row can't set an out-of-bounds selection.

Comment 3

[Dão Gottwald [:dao]]

Attachment: Bug 2052875 - [IPC Urlbar] Compare results by id across the actor boundary in urlbar tests. r?mbeier!

A provider's onEngagement runs parent-side, so on the message path it sees
wire-reconstructed details: details.element is dropped (a DOM node can't cross
the actor) and details.result is a wire copy, not the view row's result object.
browser_dynamicResults's pick/shouldNavigate asserted both by object identity,
so they only held in-process. Compare the picked result by the stable id this
bug adds, and expect a null element on the message path; the direct-path
assertions are unchanged. Test-only, both paths green.