1834701 - Update SearchSERPTelemetryChild to cache information more specifically

Status: RESOLVED FIXED

(Firefox :: Search, task, P1)

Description

[James Teow [:jteow]]

One oversight in the implementation was not accommodating the fact that processes can handle multiple SERPs at once.

This means that one browser can overwrite cached information used by another browser. Closing a browser can clear data for another.

On my Mac, one easy way to test this is opening 4+ SERPs from a single provider. At that point, some SERPs will share the same process. Any cached information that references this.document will be overriden by a more recent load, and so if the browser that overrode it is closed, the "original" browser that was earlier in the process will not only reference the wrong document in some of its references, but some references will be destroyed.

It should be a relatively straightforward fix. Instead of caching information on the instance of SearchAdImpression we should create another element called DocState which contains information specific to each document, and then have a WeakMap whose key is the document and whose value is the DocState.

Comment 1

[James Teow [:jteow]]

Attachment: Bug 1834701 - Refactor SearchSERPTelemetry to have more isolation of page data - r?standard8,scunnane

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a92eeda6e0e0
Refactor SearchSERPTelemetry to have more isolation of page data - r=Standard8

Comment 3

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/a92eeda6e0e0