Open Bug 1646541 Opened 4 years ago Updated 4 years ago

Cleanup/remove nsGlobalWindowInner::ShouldReportForServiceWorkerScope

Categories

(Core :: DOM: Service Workers, task, P3)

task

Tracking

()

People

(Reporter: kmag, Unassigned)

References

Details

No description provided.

Tracking "Figure out GetInProcessTop usage" bugs for Fission M6b.

Fission Milestone: --- → M6b

Andrew, can you review this too?

Component: DOM: Core & HTML → DOM: Service Workers
Flags: needinfo?(bugmail)

In short: The current logic is part of devtools' handling of console messages of ServiceWorkers which has slowly been getting cleaned up. The necessary cleanup is currently tracked as part of bug 1592584 which is targeted at Fission M7 but might be impacted by staffing level changes. The current implementation without that bug fixed will result in a slightly degraded experience for the specific logging mechanism, but that mechanism isn't sufficient for a good experience on its own under parent-intercept. That said, what did work should continue to work for cases where the ServiceWorker of interest to be debugged is the current devtools frame.

I'll prepare a patch to update the comment at the location that indicates the current limitations, references this bug for more depth, and references the M7 bug devtools bugs tracking the modern implementation end state.

PENDING: I still need to better clarify the non-fission status quo for ServiceWorker debugging without fission enabled since we switched to parent intercept and the enhancements that may be necessary post-bug 1592584 or that should be considered as part of it. In particular, as things relate to "devtools.debugger.features.windowless-service-workers".

Restating overview:

  • Console messages have 2 general paths:
    1. Modern: Be reported directly on a global's Console actor.
    2. Legacy: Be reported to the process-wide "@mozilla.org/consoleAPI-storage;1" JS XPCOM service on the main thread where a parameterized ConsoleAPIListener instance filters the global log soup back down to the window of interest.
  • We've been slowly moving towards the modern approach.
  • The GetInProcessScriptableTop() limitation under Fission means that:
    • The nsGlobalWindowInner::ShouldReportForServiceWorkerScope check will only see a portion of the entire docshell/browsing context tree that it saw pre-fission. Specifically, it will only see direct descendant same-origin iframes.
    • With Bug 1568597 fixed (landing soon), the ServiceWorker will end up being in-process with the (potentially) intercepted/registering page.

Restating in depth:

Assignee: nobody → bugmail
Status: NEW → ASSIGNED

I assume we can move this to M7 then?

Flags: needinfo?(nkochar)

(In reply to Andrew Sutherland [:asuth] (he/him) from comment #3)

In short: The current logic is part of devtools' handling of console messages of ServiceWorkers which has slowly been getting cleaned up. The necessary cleanup is currently tracked as part of bug 1592584 which is targeted at Fission M7

Moving to M7 based on this.

Fission Milestone: M6b → M7
Flags: needinfo?(nkochar)
Whiteboard: fission-workers

:cpeterson pinged me about this externally. The current state is:

  • With the devtools bug 1592584 fixed the devtools experience is likely fine and there's no need to block Fission.
  • The use of GetInProcessScriptableTop in nsGlobalWindowInner::ShouldReportForServiceWorkerScope is basically mooted by the above and can probably be ripped out. Its current usage is fine given that it's legacy code.

The next steps re: this bug are:

  • Audit nsGlobalWindowInner::ShouldReportForServiceWorkerScope now in the context of bug 1592584 being fixed and the higher level devtools bug on that, bug 1429805. If we can't remove ShouldReportForServiceWorkerScope then bug 1429805 should be updated and/or have new dependencies filed and a comment added to the use of GetInProcessScriptableTop.

Accordingly, I'm removing this bug from blocking fission.

Assignee: bugmail → nobody
No longer blocks: 1642433
Status: ASSIGNED → NEW
Fission Milestone: M7 → ---
Flags: needinfo?(bugmail)
Priority: -- → P3
Summary: Figure out if GetInProcessScriptableTop usage in nsGlobalWindowInner::ShouldReportForServiceWorkerScope is OK → Cleanup/remove nsGlobalWindowInner::ShouldReportForServiceWorkerScope
Whiteboard: fission-workers
You need to log in before you can comment on or make changes to this bug.