http://mxr.mozilla.org/mozilla-central/source/dom/plugins/test/mochitest/test_privatemode.xul 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.

A patch in bug 722850 is modifying this test and should probably land first.

Removed this test from global PB builds: https://hg.mozilla.org/mozilla-central/rev/917b065521cd

s/global/per-window/

In a private window, lastReportedPrivateModeState() always returns false regarding whether it's called before or after queryPrivateModeState() so I guess there is a bug. Furthermore, I don't think we need to check lastReportedPrivateModeState() as the private mode state wouldn't be changed for per-window PB window. @ehsan / josh: what is your opinion?

(In reply to Raymond Lee [:raymondlee] from comment #4) > Created attachment 697386 [details] [diff] [review] > v1 (WIP) > > In a private window, lastReportedPrivateModeState() always returns false > regarding whether it's called before or after queryPrivateModeState() so I > guess there is a bug. Furthermore, I don't think we need to check > lastReportedPrivateModeState() as the private mode state wouldn't be changed > for per-window PB window. > @josh: any info you can provide? thanks!

Hmm. I ran the mochitest-chrome tests for dom/plugins/test and it passed just fine. I can't see any reason why lastReportedPrivateModeState() should be failing from reading the relevant code.

Aha, I see now. I was confused by the fact that you made the test pass in its current state. Stepping through the code, it's functioning as specified: when the window is put into private mode at creation time, there's no content to notify (in this case, the plugin via NPP_SetValue). I think this is a check that you can remove from the test, since the querying check ensures that the plugin can see the correct value if it wants it, but I'll get Benjamin to confirm.

(In reply to Josh Matthews [:jdm] from comment #7) > Aha, I see now. I was confused by the fact that you made the test pass in > its current state. Stepping through the code, it's functioning as specified: > when the window is put into private mode at creation time, there's no > content to notify (in this case, the plugin via NPP_SetValue). I think this > is a check that you can remove from the test, since the querying check > ensures that the plugin can see the correct value if it wants it, but I'll > get Benjamin to confirm. Thanks Josh! Sorry about the confusion. :-)

Removed the lastReportedPrivateModeState() check

Is this ready for review?

Still awaiting Benjamin's input.

I don't know the code in question, so I don't think I can provide useful feedback.

Comment on attachment 702137 [details] [diff] [review] v2 Josh, can you answer comment 7?

Comment on attachment 702137 [details] [diff] [review] v2 Review of attachment 702137 [details] [diff] [review]: ----------------------------------------------------------------- I may have written this code but I have no more recollection of it than anyone else at this point, sorry. The plugin private mode spec is here: https://wiki.mozilla.org/Plugins:PrivateMode This generally looks fine to me though.

Over in bug 817477, I'm preparing to remove support for global private browsing altogether. This is one of the last remaining pieces that I would like us to fix before that bug lands...

Pushed to try and waiting for results https://tbpl.mozilla.org/?tree=Try&rev=10996563b54f