806736 - Port test_privbrowsing.html to the new per-tab PB APIs

Status: RESOLVED FIXED

(Firefox :: Private Browsing, defect)

Description

[(no longer active)]

http://mxr.mozilla.org/mozilla-central/source/toolkit/components/passwordmgr/test/test_privbrowsing.html

In order to port this test, the file needs to be copied to the same directory (perhaps with "_perwindowpb" appended to its file name), and then instead of setting privateBrowsingEnabled, we need to open a new private browsing window and then run the test on that window.  Note that the original test should only be added to the list of test files in Makefile.in ifndef MOZ_PER_WINDOW_PRIVATE_BROWSING, and the new test file should be added to the list with the reverse condition.

Comment 1

[(no longer active)]

Removed this test from per-window PB builds: https://hg.mozilla.org/mozilla-central/rev/cf47dba80a4e

Comment 2

[Raymond Lee [:raymondlee]]

Attachment: v1

Comment 3

[Josh Matthews [:jdm]]

Comment on attachment 692916 [details] [diff] [review]
v1

Review of attachment 692916 [details] [diff] [review]:
-----------------------------------------------------------------

::: toolkit/components/passwordmgr/test/test_privbrowsing.html
@@ +192,5 @@
> +  var win = mainWindow.OpenBrowserWindow({private: aIsPrivate});
> +  win.addEventListener("load", function onLoad() {
> +    win.removeEventListener("load", onLoad, false);
> +    win.addEventListener("DOMContentLoaded", function onInnerLoad() {
> +      // I am at my wits' end figuring out how to stop this load from occurring. I give up.

Just remove this comment to stop it propagating to other tests.

Comment 4

[Raymond Lee [:raymondlee]]

Attachment: v2

(In reply to Josh Matthews [:jdm] from comment #3)
> > +      // I am at my wits' end figuring out how to stop this load from occurring. I give up.
> 
> Just remove this comment to stop it propagating to other tests.

Removed that comment

Comment 5

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/a7d5ea538c57

Comment 6

[Ryan VanderMeulen [:RyanVM]]

And backed out for Android test failures. Tryserver kthxbai.
https://hg.mozilla.org/integration/mozilla-inbound/rev/b3d7e749388b

https://tbpl.mozilla.org/php/getParsedLog.php?id=18070140&tree=Mozilla-Inbound

76777 ERROR TEST-UNEXPECTED-FAIL | /tests/toolkit/components/passwordmgr/test/test_privbrowsing_perwindowpb.html | uncaught exception - TypeError: mainWindow.OpenBrowserWindow is not a function at http://mochi.test:8888/tests/toolkit/components/passwordmgr/test/test_privbrowsing_perwindowpb.html:192
12-18 15:11:13.792 I/GeckoDump( 1834): 76777 ERROR TEST-UNEXPECTED-FAIL | /tests/toolkit/components/passwordmgr/test/test_privbrowsing_perwindowpb.html | uncaught exception - TypeError: mainWindow.OpenBrowserWindow is not a function at http://mochi.test:8888/tests/toolkit/components/passwordmgr/test/test_privbrowsing_perwindowpb.html:192

Comment 7

[Raymond Lee [:raymondlee]]

(In reply to Ryan VanderMeulen from comment #6)
> And backed out for Android test failures. Tryserver kthxbai.
> https://hg.mozilla.org/integration/mozilla-inbound/rev/b3d7e749388b
> 
> https://tbpl.mozilla.org/php/getParsedLog.php?id=18070140&tree=Mozilla-
> Inbound
> 
> 76777 ERROR TEST-UNEXPECTED-FAIL |
> /tests/toolkit/components/passwordmgr/test/test_privbrowsing_perwindowpb.
> html | uncaught exception - TypeError: mainWindow.OpenBrowserWindow is not a
> function at
> http://mochi.test:8888/tests/toolkit/components/passwordmgr/test/
> test_privbrowsing_perwindowpb.html:192
> 12-18 15:11:13.792 I/GeckoDump( 1834): 76777 ERROR TEST-UNEXPECTED-FAIL |
> /tests/toolkit/components/passwordmgr/test/test_privbrowsing_perwindowpb.
> html | uncaught exception - TypeError: mainWindow.OpenBrowserWindow is not a
> function at
> http://mochi.test:8888/tests/toolkit/components/passwordmgr/test/
> test_privbrowsing_perwindowpb.html:192

@ehsan / josh: The issues only happen in Android.  Any suggestions how to fix this?

Comment 8

[(no longer active)]

Firefox for Android's browser.js does not implement OpenBrowserWindow.  I'm not sure what happens when you try to open a new window on mobile.  Brian, how do you open a new private tab on mobile from a mochitest?

Comment 9

[Brian Nicholson (:bnicholson)]

(In reply to :Ehsan Akhgari from comment #8)
> Brian, how do you open a new private tab on mobile from a mochitest?

I think you can do this:
mainWindow.BrowserApp.addTab(url, { isPrivate: true });

Comment 10

[Raymond Lee [:raymondlee]]

Attachment: v3

I have updated the patch, however, the getPopupNotification(aWindow) in notification_common.js (http://mxr.mozilla.org/mozilla-central/source/toolkit/components/passwordmgr/test/notification_common.js#6) doesn't return undefined for Andriod.  I think chromeWin.PopupNotifications doesn't exist in Andriod.

Also, all tests which invoke getPopupNotifications are excluded for Android e.g. test_notifications.html, test_prompt.html in http://mxr.mozilla.org/mozilla-central/source/testing/mochitest/android.json.  

Furthermore, test_privbrowsing.html is not in that list, I think it's because it has a condition to check whether nsIPrivateBrowsingService exists or not before executing the test, and Andriod doesn't have that API.

Added the test file to excluded list, and everything goes ok.
https://tbpl.mozilla.org/?tree=Try&rev=7f079b94f96c

Without adding the file to excluded list and it fails with no popupNotifications.
https://tbpl.mozilla.org/?tree=Try&rev=929e5cdc04e4

@brian: could you confirm that please?

Comment 11

[(no longer active)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/b51c3d819364

Comment 12

[Brian Nicholson (:bnicholson)]

(In reply to Raymond Lee [:raymondlee] from comment #10)
> Created attachment 698579 [details] [diff] [review]
> v3
> 
> @brian: could you confirm that please?

Yes, that makes sense; I think disabling this was the right thing to do. CC'ing gbrown in case he has any objections.

Comment 13

[Geoff Brown [:gbrown]]

Seems reasonable to me.

Comment 14

[Ed Morley [:emorley]]

Backed out for frequent intermittent failures in test_privbrowsing_perwindowpb.html:
https://tbpl.mozilla.org/?tree=Mozilla-Inbound&jobname=mochitest-5&tochange=772d358e15d4&fromchange=30ec18652409

https://hg.mozilla.org/integration/mozilla-inbound/rev/772d358e15d4

Comment 15

[Raymond Lee [:raymondlee]]

Attachment: v4

I have made some changes to fix frequent intermittent failures.  Pushed to try twice and look good!

https://tbpl.mozilla.org/?tree=Try&rev=015caaaa6e85
https://tbpl.mozilla.org/?tree=Try&rev=c8097176c5b4

Comment 16

[(no longer active)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/bdddc01216a4

Comment 17

[(no longer active)]

Alex, do we need Aurora approval for test-only changes?

Comment 18

[Ed Morley [:emorley]]

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

Comment 19

[(no longer active)]

Raymond, can you please attach a version of this patch which applies cleanly on top of Aurora?  Thanks!

Comment 20

[Raymond Lee [:raymondlee]]

Comment on attachment 699635 [details] [diff] [review]
v4

Ehsan, this last patch in this bug can apply cleanly to the tip of aurora.  

changeset:   123620:1fff5df1ac6a
tag:         tip
user:        Dão Gottwald <[e-mail]>
date:        Mon Jan 14 10:21:10 2013 +0100
summary:     Bug 824825 - Add toolbar button and popup notification icons for tabs with camera / microphone access. r=dolske a=lsblakk

Could you try please?

Comment 21

[(no longer active)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/c06d65b1674a

(I think I posted this on the wrong bug.  Oh well...)

Comment 22

[(no longer active)]

Hmm, this is failing on Aurora:

https://tbpl.mozilla.org/?tree=Mozilla-Aurora&rev=12f52471747d

So I backed it out: https://hg.mozilla.org/releases/mozilla-aurora/rev/1292047d5d18

Raymond, can you please investigate?  Thanks!

Comment 23

[Raymond Lee [:raymondlee]]

(In reply to :Ehsan Akhgari from comment #22)
> Hmm, this is failing on Aurora:
> 
> https://tbpl.mozilla.org/?tree=Mozilla-Aurora&rev=12f52471747d
> 
> So I backed it out:
> https://hg.mozilla.org/releases/mozilla-aurora/rev/1292047d5d18
> 
> Raymond, can you please investigate?  Thanks!

Ehsan, I think the wrong version of patch was applied.  It was using v3, could you try v4 again please?

Comment 24

[Raymond Lee [:raymondlee]]

v4 looks fine on aurora.
https://tbpl.mozilla.org/?tree=Try&rev=3a09a8100339

Comment 25

[(no longer active)]

I'll do that when Aurora reopens.  Thanks!

Comment 26

[(no longer active)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/5020af3460e0