1128443 - "WARNING: content window passed to PrivateBrowsingUtils.isWindowPrivate. Use isContentWindowPrivate instead" from browser.js

Status: NEW

(Firefox :: Private Browsing, defect, P3)

Description

[Henrik Skupin [:whimboo][⌚️UTC+2]]

There are even more instances of usage of isWindowPrivate around in browser.js. I wonder why we haven't converted all of them at once when turning the flip in bug 1069059?

From one of the latest Nightly builds:

WARNING: content window passed to PrivateBrowsingUtils.isWindowPrivate. Use isContentWindowPrivate instead (but only for frame scripts).
pbu_isWindowPrivate@resource://gre/modules/PrivateBrowsingUtils.jsm:25:14
nsBrowserAccess.prototype.openURI@chrome://browser/content/browser.js:14057:21

Associated source:

    let isPrivate = PrivateBrowsingUtils.isWindowPrivate(aOpener || window);
    switch (aWhere) {

I also see lots of other calls like that, but not sure if all of them have to be converted too:

https://dxr.mozilla.org/mozilla-central/search?limit=100&redirect=false&q=isWindowPrivate%20path%3Abrowser%2F

Comment 1

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

The problem is that most of the instances you point out are fine. As long as the argument passed in is a chrome window, there's no problem.

I guess aOpener can be a content window. Mainly we just want to make sure it's not a CPOW. Mike, what's the state of that argument right now? Do we ever pass a CPOW there?

Comment 2

[Mike Conley (:mconley) (:⚙️)]

My patch for bug 1047603 adds a check to ensure that aOpener is never a CPOW - that's just waiting on one last bit of review to land.

That function does not expect a CPOW as aOpener, but (until my patch lands) there are no protections preventing it from occurring.

Comment 3

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Mike, I assume this is fixed now? I cannot repro given that we no longer run Mozmill.

Comment 4

[Mike Conley (:mconley) (:⚙️)]

I don't think this is an issue anymore. We've outlawed CPOW usage in browser code, so this would be throwing.

Comment 5

[Mike Conley (:mconley) (:⚙️)]

Hm, wait, let me rephrase.

I don't think I've run into this warning for a while, but I suppose it's possible we might still run into them. As billm says, at any point, a content nsIDOMWindow or CPOW could be passed to the function. Luckily, in the CPOW case, we'll throw, and won't do synchronous messaging.

Comment 6

[Myk Melez [:myk] [@mykmelez]]

This currently happens if you press the "Help" button (or invoke its keyboard shortcut) in an about:preferences "subdialog" (Fonts, Colors, Languages, Connections, and Sanitize).

Comment 7

[Mathew Hodson]

I can still reproduce this warning on Nightly 113.0a1 (2023-03-31) if I click one of the "Learn more" links from about:preferences.

WARNING: content window passed to PrivateBrowsingUtils.isWindowPrivate. Use isContentWindowPrivate instead (but only for frame scripts).
pbu_isWindowPrivate@resource://gre/modules/PrivateBrowsingUtils.sys.mjs:23:11
getTargetWindow@resource:///modules/URILoadingHelper.sys.mjs:644:57
openLinkIn@resource:///modules/URILoadingHelper.sys.mjs:467:16
openTrustedLinkIn@resource:///modules/URILoadingHelper.sys.mjs:716:10
openTrustedLinkIn@chrome://browser/content/utilityOverlay.js:124:20
handleEvent@chrome://global/content/elements/moz-support-link.mjs:72:26

Comment 8

[ISHIKAWA, Chiaki]

Actually, this happens in C-C Thunderbird mochitest.
At last count, this morning, the warning is issued 137 times during my local mochitest under linux.
The stack dump is slightly different, though.
There are at least two different paths that lead to the warnng.

27:51.51 GECKO(1380071) WARNING: content window passed to PrivateBrowsingUtils.isWindowPrivate. Use isContentWindowPrivate instead (but only for frame scripts).
27:51.51 GECKO(1380071) pbu_isWindowPrivate@resource://gre/modules/PrivateBrowsingUtils.sys.mjs:23:11
27:51.51 GECKO(1380071) _update/useDarkTheme<@resource://gre/modules/LightweightThemeConsumer.sys.mjs:277:36
27:51.51 GECKO(1380071) _update@resource://gre/modules/LightweightThemeConsumer.sys.mjs:292:7
27:51.51 GECKO(1380071) LightweightThemeConsumer@resource://gre/modules/LightweightThemeConsumer.sys.mjs:220:8
27:51.51 GECKO(1380071) observe/<@resource:///modules/MailGlue.sys.mjs:481:15
27:51.51 GECKO(1380071) EventListener.handleEvent*observe@resource:///modules/MailGlue.sys.mjs:473:18
27:52.13 INFO Closing the tab

89:15.66 GECKO(1415128) WARNING: content window passed to PrivateBrowsingUtils.isWindowPrivate. Use isContentWindowPrivate instead (but only for frame scripts).
89:15.66 GECKO(1415128) pbu_isWindowPrivate@resource://gre/modules/PrivateBrowsingUtils.sys.mjs:23:11
89:15.66 GECKO(1415128) isBrowserPrivate@resource://gre/modules/PrivateBrowsingUtils.sys.mjs:43:19
89:15.66 GECKO(1415128) fixupAndLoadURIString@resource://gre/modules/RemoteWebNavigation.sys.mjs:129:56
89:15.66 GECKO(1415128) fixupAndLoadURIString/<@chrome://global/content/elements/browser-custom-element.js:958:28
89:15.66 GECKO(1415128) _wrapURIChangeCall@chrome://global/content/elements/browser-custom-element.js:886:9
89:15.66 GECKO(1415128) fixupAndLoadURIString@chrome://global/content/elements/browser-custom-element.js:957:12
89:15.66 GECKO(1415128) _showAccountInfo@chrome://messenger/content/preferences/compose.js:626:13
89:15.66 GECKO(1415128) onSelectionChanged@chrome://messenger/content/preferences/compose.js:575:10
89:15.66 GECKO(1415128) onselect@about:preferences#compose:1:12
89:15.66 GECKO(1415128) _fireOnSelect@chrome://global/content/elements/richlistbox.js:328:12
89:15.66 GECKO(1415128) selectItem@chrome://global/content/elements/richlistbox.js:459:12
89:15.66 GECKO(1415128) addCloudFileAccount@chrome://messenger/content/preferences/compose.js:646:16
89:15.66 GECKO(1415128) oncommand@about:preferences#compose:1:12
89:15.66 GECKO(1415128) synthesizeMouseAtPoint@chrome://mochikit/content/tests/SimpleTest/EventUtils.js:819:13
89:15.66 GECKO(1415128) synthesizeMouse@chrome://mochikit/content/tests/SimpleTest/EventUtils.js:553:10
89:15.66 GECKO(1415128) synthesizeMouseAtCenter@chrome://mochikit/content/tests/SimpleTest/EventUtils.js:977:10
89:15.66 GECKO(1415128) addRemoveAccounts@chrome://mochitests/content/browser/comm/mail/components/preferences/test/browser/browser_cloudfile.js:291:14
89:15.66 GECKO(1415128) async*handleTask@chrome://mochikit/content/browser-test.js:1145:26
89:15.66 GECKO(1415128) _runTaskBasedTest@chrome://mochikit/content/browser-test.js:1217:18
89:15.66 GECKO(1415128) async*Tester_execTest@chrome://mochikit/content/browser-test.js:1358:14
89:15.66 GECKO(1415128) nextTest/<@chrome://mochikit/content/browser-test.js:1134:14
89:15.66 GECKO(1415128) SimpleTest.waitForFocus/<@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:1058:13

Comment 9

[ISHIKAWA, Chiaki]

(In reply to ISHIKAWA, Chiaki from comment #8)

Actually, this happens in C-C Thunderbird mochitest.
At last count, this morning, the warning is issued 137 times during my local mochitest under linux.
... omission ...

Should I file a separate bugzilla for C-C TB?

Comment 10

[Hartmut Welpmann [:welpy-cw]]

(In reply to ISHIKAWA, Chiaki from comment #8)

27:51.51 GECKO(1380071) WARNING: content window passed to PrivateBrowsingUtils.isWindowPrivate. Use isContentWindowPrivate instead (but only for frame scripts).
27:51.51 GECKO(1380071) pbu_isWindowPrivate@resource://gre/modules/PrivateBrowsingUtils.sys.mjs:23:11
27:51.51 GECKO(1380071) _update/useDarkTheme<@resource://gre/modules/LightweightThemeConsumer.sys.mjs:277:36
27:51.51 GECKO(1380071) _update@resource://gre/modules/LightweightThemeConsumer.sys.mjs:292:7
27:51.51 GECKO(1380071) LightweightThemeConsumer@resource://gre/modules/LightweightThemeConsumer.sys.mjs:220:8
27:51.51 GECKO(1380071) observe/<@resource:///modules/MailGlue.sys.mjs:481:15

Even just running Thunderbird Daily, I get this every time the address book is being opened.

89:15.66 GECKO(1415128) WARNING: content window passed to PrivateBrowsingUtils.isWindowPrivate. Use isContentWindowPrivate instead (but only for frame scripts).
89:15.66 GECKO(1415128) pbu_isWindowPrivate@resource://gre/modules/PrivateBrowsingUtils.sys.mjs:23:11
89:15.66 GECKO(1415128) isBrowserPrivate@resource://gre/modules/PrivateBrowsingUtils.sys.mjs:43:19
89:15.66 GECKO(1415128) fixupAndLoadURIString@resource://gre/modules/RemoteWebNavigation.sys.mjs:129:56
89:15.66 GECKO(1415128) fixupAndLoadURIString/<@chrome://global/content/elements/browser-custom-element.js:958:28
89:15.66 GECKO(1415128) _wrapURIChangeCall@chrome://global/content/elements/browser-custom-element.js:886:9
89:15.66 GECKO(1415128) fixupAndLoadURIString@chrome://global/content/elements/browser-custom-element.js:957:12
89:15.66 GECKO(1415128) _showAccountInfo@chrome://messenger/content/preferences/compose.js:626:13
89:15.66 GECKO(1415128) onSelectionChanged@chrome://messenger/content/preferences/compose.js:575:10
89:15.66 GECKO(1415128) onselect@about:preferences#compose:1:12
89:15.66 GECKO(1415128) _fireOnSelect@chrome://global/content/elements/richlistbox.js:328:12
89:15.66 GECKO(1415128) selectItem@chrome://global/content/elements/richlistbox.js:459:12

This happens when a Filelink provider is selected under Settings|Composition.

While the first one could be mitigated by setting browser.theme.dark-private-windows to false (see https://searchfox.org/mozilla-central/rev/1e8cec3727d6e09f4af41bb3d202b7a4c326ed84/toolkit/modules/LightweightThemeConsumer.sys.mjs#276), I've found no easy workaround for the second one.