Closed Bug 1843846 Opened 1 year ago Closed 1 year ago

Background context tracking should not be confused by iframes

Categories

(WebExtensions :: General, defect, P2)

defect

Tracking

(firefox118 fixed)

RESOLVED FIXED
118 Branch
Tracking Status
firefox118 --- fixed

People

(Reporter: robwu, Assigned: robwu)

References

(Blocks 1 open bug)

Details

(Whiteboard: [addons-jira])

Attachments

(3 files)

The presence of iframes in background contexts can currently confuse the background context tracking logic as follows:

  • expectViewLoad is currently called whenever DOMContentLoaded is dispatched. But because this event bubbles up from child frames to the frame message manager, it is possible for a completed child frame load to prematurely signal completion of the top-level load. Consequently, ext-backgroundPage.js may incorrectly believe the background context to have finished loading, and potentially miss registration of persistent listeners.

  • ProxyContextParent is currently initialized on the first extension API call, or the DOMContentLoaded notification, whichever is first. If the child frame has an extension API call before the top-level background context, then the ProxyContextParent of the frame will be the first one with viewType = "background". That causes extension.backgroundContext to be incorrectly set to that child's context instead of the top-level context.

context.isBackgroundContext is used to determine whether the
background context participates in the event page suspension mechanism.
This should exclude child frames, despite them being tagged as
background contexts as well.

To determine whether a context is the top context, the browsingContext
is forwarded from recvCreateProxyContext to the
ExtensionPageContextParent constructor. To minimize the extent of
changes, browsingContext is not stored in the class instance.
Only the following are stored, derived from browsingContext:

  • xulBrowser (pre-existing)
  • isTopContext

This patch also clarifies the expected types that are involved with
comments and assertions: The availability of browsingContext was
previously not obvious in the code due to the presence of ? in
actor.browsingContext?., introduced in part 1.6 of bug 1688040.

The parent patches and bug are about issues with tracing the correct
context in the parent process. The current implementation in the child
already works correctly (even before the parent patches). To make sure
that this continues to work correctly in the future, add test coverage
for extension.getBackgroundPage().

Severity: -- → S4
Priority: -- → P2

Linking more bugs for context:

  • bug 1688040 introduced weakened conditions that the patch in comment 2 rectifies.
  • Without the fix in this bug, event page timeout handling for filterResponseData (added in bug 1759300) does not work correctly under the circumstances sketched in the initial report.
  • bug 1762225 relies on a correct ExtensionViewLoaded message for background context tracing. An incorrectly timed message can result in missing persistent listeners.
Blocks: 1844217
Pushed by rob@robwu.nl: https://hg.mozilla.org/integration/autoland/rev/25556601cd14 Dispatch ExtensionViewLoaded only for top-level context r=rpl https://hg.mozilla.org/integration/autoland/rev/271e26c0b265 Ignore child frames in backgroundContext r=rpl https://hg.mozilla.org/integration/autoland/rev/aa0faf5572aa Add extra test coverage for getBackgroundPage() r=rpl
Regressions: 1847299
Status: ASSIGNED → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Target Milestone: --- → 118 Branch
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: