1077363 - Lots of "content window passed to PrivateBrowsingUtils.isWindowPrivate. Use isContentWindowPrivate instead" warnings in log files

Status: RESOLVED FIXED

(Mozilla QA Graveyard :: Infrastructure, defect)

Description

[Daniel Gherasim]

Once Bug 1069059 landed, we recieve a big warning in our logs that we should use isContentWindowPrivate instead of isWindowPrivate in our tests.

So maybe we have to change to that function in our BrowserWindow class, where we use it.

So what we need to do is replace one line:
https://hg.mozilla.org/qa/mozmill-tests/file/cf3e3bcdf350/firefox/lib/ui/browser.js#l32

And check if that works on all platforms for us (OSX, Linux & Windows).

I also see that this should be used only in frame scripts, but I don't know what that means, so asking Bill maybe he can clarify this. Thanks!

Comment 1

[Daniel Gherasim]

Attachment: jenkins.log

This is the warning we get.

Comment 2

[Andrei Eftimie]

Our logs are getting kind of cluttered because of this warning.
Lets fix it sooner than later.

Comment 3

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

P1 and P2 shouldn't be a mentored bug. Especially if we want to get this fixed asap.

(In reply to daniel.gherasim from comment #0)
> I also see that this should be used only in frame scripts, but I don't know
> what that means, so asking Bill maybe he can clarify this. Thanks!

Our scripts are running in chrome scope, and Mozmill doesn't support e10s yet. So feedback from Bill would be kinda appreciated. Because I'm not sure how to read it. I thought we have private browsing mode only for chrome but not for each individual content window. Also setting needinfo on Tim, who originally proposed that output on bug 1062777.

Comment 4

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

Wait. I also see lines like:

> -      if (charset && !PrivateBrowsingUtils.isWindowPrivate(aBrowser.contentWindow))
> +      if (charset && !PrivateBrowsingUtils.isBrowserPrivate(aBrowser))

Isn't that the method we want? This looks to be chrome like.

Comment 5

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

If you have a XUL <browser> element, you definitely want to use isBrowserPrivate.

Comment 6

[Tim Taubert [:ttaubert] (inactive)]

The Firefox UI supports private windows only. The platform and Firefox however do support per-tab private browsing where a private tab could be mixed with non-private tabs in a non-private window, see e.g. bug 899276. We made no promises to support it but we're trying to not break it as it's really not that hard actually.

Comment 7

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

Andreea, can we please also get this onto the list of possible next sprints? It causes lots of log spam on CI. Thanks.

Comment 8

[Andreea Matei [:AndreeaMatei]]

Adding it for the sprint that start on Monday.

Comment 9

[Mihaela Velimiroviciu (:mihaelav)]

That warning message appears also in the terminal (at least on Ubuntu), not only in Jenkins. I changed PrivateBrowsingUtils.isWindowPrivate to isContentWindowPrivate everywhere in our mozmill tests and library, but the message still appears when I run the tests from testPrivateBrowsing, even if the test doesn't use that function.
Could it be that that message is displayed whenever we import/use the PrivateBrowsingUtils module, no matter if we use that function (PrivateBrowsingUtils.isWindowPrivate) or not?

Comment 10

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

Mihaela, have you read my comment 4?

Comment 11

[Mihaela Velimiroviciu (:mihaelav)]

Attachment: warning.log

(In reply to Henrik Skupin (:whimboo) from comment #10)
> Mihaela, have you read my comment 4?

Yes, I've read it and also tried it and I get the same warning. I get the same warning even if I replace that call with just "true" - see the attached logs, when running testAboutPrivateBrowsing.js while having PrivateBrowsingUtils.isWindowPrivate replaced by "true" in the browser.js lib. The lines 37, 47 and 54 from the log correspond to the following calls:
* controller.waitForPageLoad(); 
* browserWindow.open({private: true, method: "callback"}, () => {
* pbWindow.controller.click(moreInfo);

So, I think the fix (if there really is one and this is not intended) needs to be done somewhere else, not in the browser.js lib.

Comment 12

[Mihaela Velimiroviciu (:mihaelav)]

I looked on mozilla-central and found out that PrivateBrowsingUtils.isWindowPrivate function is still widely used and it's called in functions that _may_ be called when running our test:
* http://dxr.mozilla.org/mozilla-central/source/browser/base/content/utilityOverlay.js#16
* gPrivateBrowsingUI.init();( http://dxr.mozilla.org/mozilla-central/source/browser/base/content/browser.js#7268) which is called by gBrowserInit.onLoad (http://dxr.mozilla.org/mozilla-central/source/browser/base/content/browser.js#956)

Comment 13

[Mihaela Velimiroviciu (:mihaelav)]

Henrik, any thoughts regarding comment 12?
I think it's not something we can do to get rid of that warning.

Comment 14

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

I cannot tell, but it would be good to have a testcase we could run with Mozmill and which produces this output.

Comment 15

[Mihaela Velimiroviciu (:mihaelav)]

Attachment: test

The output is very easy to reproduce, running the attached test which only opens the about:privatebrowsing page and waits for it to load.

Comment 16

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

Thanks Mihaela for checking that. Can you also see this output in the browser console when opening about:privatebrowsing manually in a fresh profile?

Comment 17

[Mihaela Velimiroviciu (:mihaelav)]

(In reply to Henrik Skupin (:whimboo) from comment #16)
> Thanks Mihaela for checking that. Can you also see this output in the
> browser console when opening about:privatebrowsing manually in a fresh
> profile?

No, it doesn't appear in the browser console when opening the page manually.

Comment 18

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

Totally nothing related to our tests. It's the about private browsing page which is misbehaving here. I will move this bug over to infrastructure for just the annoyance factor in our jenkins logs. Thanks Mihaela for the minified testcase.

Comment 19

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

This should be fixed now in the next Nightly build.