Make docshell responsible for determining a window's outerWindowID to avoid consumers having to create the window to find the ID

RESOLVED FIXED in Firefox 64

Status

()

P1
normal
RESOLVED FIXED
6 months ago
6 months ago

People

(Reporter: Gijs, Assigned: Gijs)

Tracking

(Blocks: 1 bug)

Trunk
mozilla64
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox64 fixed)

Details

(Whiteboard: [fxperf:p2])

Attachments

(1 attachment)

(Assignee)

Description

6 months ago
At the moment if nothing else, https://searchfox.org/mozilla-central/rev/0640ea80fbc8d48f8b197cd363e2535c95a15eb3/toolkit/content/browser-child.js#29-32 will force about:blank window creation, even after we fix bug 1471327. (and, for parent process windows, fetching that ID directly from BrowserWindowTracker -- though in the parent process, https://searchfox.org/mozilla-central/rev/0640ea80fbc8d48f8b197cd363e2535c95a15eb3/toolkit/content/widgets/browser.xml#824-827 touches things first anyway).

We could try delaying all the things that touch outerWindowID, or we could potentially try moving outerWindowID as a thing onto the docshell, so tracking parent process docshells doesn't require instantiating an outer window in them. I think I prefer the latter option, because AFAICT we assume that the window ID never changes for the life of the docshell anyway. Kris, does that sound sane?
Flags: needinfo?(kmaglione+bmo)
(In reply to :Gijs (he/him) from comment #0)

> We could try delaying all the things that touch outerWindowID, or we could
> potentially try moving outerWindowID as a thing onto the docshell, so
> tracking parent process docshells doesn't require instantiating an outer
> window in them. I think I prefer the latter option, because AFAICT we assume
> that the window ID never changes for the life of the docshell anyway.

When I saw the bug title, I was thinking exactly the same. It will probably be easier to make outerWindowID to not create an about:blank than to play whack-a-mole with usages of outerWindowID. Also, I'm _guessing_ that outerWindowID usage might increase with Fission because it will be more necessary to directly identify subframes talking with the parent process
Whiteboard: [fxperf] → [fxperf:p2]
(Assignee)

Comment 2

6 months ago
Talked to bz:

14:58:52 <Gijs> bz: as for problem... we keep track of outer window ids all the way between processes + frontend + network, for "reasons"
14:59:27 <Gijs> bz: right now, both for remote and non-remote browsers, we try to access the outer window for the contained window "soon" after that browser is created
14:59:44 <Gijs> that forces creation of an about:blank content window, even where we wouldn't necessarily otherwise bother
14:59:59 <Gijs> (because to get it from js, right now we need to ask for docShell.contentWindow.outerWindowID)
15:00:05 <Gijs> err
15:00:08 <Gijs> .windowUtils in there as well
15:00:10 <Gijs> but anyway
15:00:33 <Gijs> bz: seemed to me that we assign these ids based on NextWindowID(), and that could just as well live on/in docshell?
15:03:10 <@bz> I see
15:03:16 — @bz did see this bug go by
15:04:13 <@bz> So I would be fine with putting the outer window id on docshell as well
15:04:22 <@bz> And having docshell compute it
15:04:27 <@bz> And then the outer window just copy it when it gets created
15:05:02 <Gijs> bz: right, yep, that makes sense. Sounds like you're saying you want the outer window to keep the id as well, rather than just ask its docshell or something?
15:05:23 <@bz> I would prefer that, yes
15:05:25 <Gijs> bz: I can think of reasons for that, but rather than me making it up, can you elaborate on why? :)
15:05:51 <@bz> Because you can still be referencing the window from JS after you remove the <browser> from the DOM and tear down the docshell

I'll pick this up.
Assignee: nobody → gijskruitbosch+bugs
Status: NEW → ASSIGNED
Component: General → Document Navigation
Flags: needinfo?(kmaglione+bmo)
Priority: P3 → P1
Product: Firefox → Core
Summary: Stop forcing about:blank content to be created by getting the outerWindowID when browser-child.js loads → Make docshell responsible for determining a window's outerWindowID to avoid consumers having to create the window to find the ID
(Assignee)

Updated

6 months ago
Blocks: 1496360
(Assignee)

Updated

6 months ago
No longer depends on: 1471327

Comment 4

6 months ago
Pushed by gijskruitbosch@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/61ad878d2f66
make docshell responsible for outer window IDs, r=bzbarsky

Comment 5

6 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/61ad878d2f66
Status: ASSIGNED → RESOLVED
Last Resolved: 6 months ago
status-firefox64: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla64
You need to log in before you can comment on or make changes to this bug.