Closed Bug 1745240 Opened 3 years ago Closed 3 years ago

Factor out code to use a unique method to filter/accept browsing context based on the watcher context

Categories

(DevTools :: General, task)

task

Tracking

(firefox98 fixed)

RESOLVED FIXED
98 Branch
Tracking Status
firefox98 --- fixed

People

(Reporter: ochameau, Assigned: ochameau)

References

Details

Attachments

(4 files)

We have lots of duplicated code doing the following check:

if (context.type == "browser-element" && browsingContext.browserId == context.browserId) {
  // Do something with this document as it part of the debugged context
}

But bug 1569859 will make this check slightly more complex, and it will be quite important to ensure applying this change to all the callsites:

if (context.type == "browser-element" && (browsingContext.browserId == context.browserId || browsingContext.opener.browserId == context.browserId)) {
  // Do something with this document as it part of the debugged context
}

(We now also accept popups, whose browserId won't be the same as the one for the tab they originate from)

The goal of this bug is to try to have a shared function for all these checks, so that bug 1569859 can only change this one method instead of the many callsites.

Depends on: 1746952

This boolean helps know for which BrowsingContext we should create a target or track resources.
So that it is part of what defines the context we should debug and will be handy to have
in all filtering functions we use to filter browsing context or platform objects.

Popup debugging (bug 1569859) will force to revisit how we filter out the BrowsingContext
that are meant to be debugged. We won't only accept BrowsingContext based on their browserId.
This would force us to carefuly review all the codes where we filter BrowsingContexts.
And if we later have to tweak this, do this again.

It would be nice to have a unique method to filter things out.
It will also be beneficial once we add new debuggable contexts like workers
as we would only have to tweak this method.

For now, this patch focuses only on Target helpers and JSWindowActor's,
but I'll followup to other server modules.

Note that I'm changing the behavior of getAllRemoteBrowsingContexts
in order to also return the top browsing context by default.
We were having a few places where we were re-adding it after,
but that's not trivial. It is easier to remove it in the rare function that need that.

Track all code which may filter BrowsingContext or WindowGlobal in the server codebase
in order to use a unique filtering method.

Pushed by apoirot@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/e8e4a1f1d8a1 [devtools] Move WatcherActor.isServerTargetSwitchingEnabled into session's context. r=nchevobbe,jdescottes https://hg.mozilla.org/integration/autoland/rev/f751667f77b0 [devtools] Unify target helpers and JS Window actors to use a shared method to filter BrowsingContext/WindowGlobal's. r=nchevobbe,jdescottes https://hg.mozilla.org/integration/autoland/rev/2c11b1939c3e [devtools] Use only one method to filter both BrowsingContext and WindowGlobals. r=nchevobbe,jdescottes https://hg.mozilla.org/integration/autoland/rev/aa51d27a4da5 [devtools] Factorize some more code around isBrowsingContextPartOfContext. r=nchevobbe,jdescottes
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: