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

Status: NEW

(Firefox :: Private Browsing, defect)

Description

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

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+1]]

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