1669961 - JSWindowActorChild .contentWindow getter returns a WindowProxy even when the inner window isn't current

Status: RESOLVED FIXED

(Core :: DOM: Content Processes, defect, P2)

Description

[Julian Descottes [:jdescottes]]

In a JSWindowActorChild, this.document and this.contentWindow.document can point to different objects.

For instance, in the patch at https://phabricator.services.mozilla.com/D92648#2985445 , when the document has not yet reached the "complete" readyState, the two objects will be different. Example of differences:

• this.document.readyState is "complete" while this.contentWindow.document.readyState is "loading"
• this.document.location is null while this.contentWindow.location.href points to the expected URL

Looking at the implementation https://searchfox.org/mozilla-central/rev/f82d5c549f046cb64ce5602bfd894b7ae807c8f8/dom/ipc/jsactor/JSWindowActorChild.cpp#76-109, the document getter is not relying on the contentWindow getter, which explains why the two objects can differ.

Is the difference intentional? If it is, are there guidelines or documentation to know when we should use this.document vs this.contentWindow.document?

Comment 1

[Julian Descottes [:jdescottes]]

Was clarified by a discussion on #Fission

this.document and this.contentWindow.document can be out of sync.
this.contentWindow is a windowproxy which might point to a window from another process.
Under some circumstances, this.document might point to a destroyed document, while this.contentWindow.document will point to the document of another window.

this.document should always be considered as the source of truth and used rather than this.contentWindow.document.

Could be interesting to mention at https://firefox-source-docs.mozilla.org/dom/ipc/jsactors.html#this-contentwindow , but I'm not sure how to phrase the warning properly. Closing for now, feel free to reopen if anyone wants to update the doc.

Comment 2

[Kris Maglione [:kmag]]

Attachment: Bug 1669961: Return null from `.contentWindow` when inner window is inactive. r=nika

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/77c34aa0aaf3
Return null from `.contentWindow` when inner window is inactive. r=nika

Comment 4

[Natalia Csoregi [:nataliaCs]]

Backed out for causing crashtests due to windowUtils not being accessible

backout: https://hg.mozilla.org/integration/autoland/rev/c9af08be84bdbaf03622e7e0f8e4ad747cd74707

push: https://treeherder.mozilla.org/#/jobs?repo=autoland&group_state=expanded&revision=77c34aa0aaf3b522e2e28e2d2d0ee25a3fa5fc42&searchStr=crashtest&selectedTaskRun=K_Dc6RsaR9awX8u1XiHpfQ.0

failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=319422376&repo=autoland&lineNumber=2274

[task 2020-10-22T19:49:04.674Z] 19:49:04 INFO - REFTEST INFO | SET PREFERENCE pref(browser.send_pings,true)
[task 2020-10-22T19:49:04.674Z] 19:49:04 INFO - REFTEST TEST-LOAD | file:///Users/cltbld/tasks/task_1603395997/build/tests/reftest/tests/docshell/base/crashtests/1257730-1.html | 34 / 3856 (0%)
[task 2020-10-22T19:49:04.680Z] 19:49:04 ERROR - REFTEST ERROR | REFTEST TEST-UNEXPECTED-FAIL | file:///Users/cltbld/tasks/task_1603395997/build/tests/reftest/tests/docshell/base/crashtests/1257730-1.html | [CONTENT] flushWindow failed: TypeError: can't access property "windowUtils", win is null
[task 2020-10-22T19:49:04.680Z] 19:49:04 ERROR -
[task 2020-10-22T19:49:04.683Z] 19:49:04 INFO - REFTEST TEST-PASS | docshell/base/crashtests/1257730-1.html | (LOAD ONLY)
[task 2020-10-22T19:49:04.684Z] 19:49:04 INFO - REFTEST TEST-END | docshell/base/crashtests/1257730-1.html

Comment 5

[BugBot [:suhaib / :marco/ :calixte]]

There's a r+ patch which didn't land and no activity in this bug for 2 weeks.
:kmag, could you have a look please?
For more information, please visit auto_nag documentation.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/befecdb77ef7
Return null from `.contentWindow` when inner window is inactive. r=nika

Comment 7

[Sandor Molnar[:smolnar]]

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

Comment 8

[Alexandru Ionescu (needinfo me) [:alexandrui]]

== Change summary for alert #27616 (as of Thu, 12 Nov 2020 16:27:51 GMT) ==

Regressions:

Ratio	Suite	Test	Platform	Options	Absolute values (old vs new)
2%	Base Content JS		macosx1014-64-shippable-qr		2,798,629.33 -> 2,841,002.67

For up to date results, see: https://treeherder.mozilla.org/perfherder/alerts?id=27616