Closed Bug 849723 Opened 9 years ago Closed 9 years ago

Opening a frame in a hiddenPrivateDOMWindow fails to initialize

Categories

(Firefox :: Private Browsing, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 22

People

(Reporter: evold, Assigned: jdm)

References

Details

Attachments

(1 file)

If you open the example code (in the linked gist) in scratchpad, run it once, you'll notice frame created within the hiddenPrivateDOMWindow fails to load, and the frame in the hiddenDOMWindow does load. Run the code again and it works as expected.

https://gist.github.com/erikvold/ae702922a66387cad639
Maybe we could sidestep the issue by ensuring that the private hidden window is created the first time a private window is created?
(In reply to Josh Matthews [:jdm] from comment #1)
> Maybe we could sidestep the issue by ensuring that the private hidden window
> is created the first time a private window is created?

I'm not sure why that would help, is the issue a race condition (doesn't seem like it to me) or that there needs to be 2+ `hiddenPrivateDOMWindow`s in order for one to work?
Josh, can you take this please?
Assignee: nobody → josh
Depends on: 815847
No longer blocks: sdk-pwpb-fx21
I'm pretty sure the frame that's appended to the documentElement is overwritten by the load of about:blank, but I'll confirm. The rest of the browser doesn't appear to have any synchronization for the non-private hidden window; it just assumes that it will be usable and is lucky I guess.
Erik, it looks like the only option here is to expose an API to correctly use the private hidden window, given that it is created on-demand.
Comment on attachment 725098 [details] [diff] [review]
Introduce API to use the private hidden window correctly.

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

r=me on the fixed patch.  ;-)

::: docshell/test/chrome/test_private_hidden_window.html
@@ +40,5 @@
>      win.close();
>      win = null;
>    }, false);
> +});
> +//}, 100);

No need to comment this out.

::: toolkit/content/PrivateBrowsingUtils.jsm
@@ +4,5 @@
>  
>  this.EXPORTED_SYMBOLS = ["PrivateBrowsingUtils"];
>  
>  Components.utils.import("resource://gre/modules/Services.jsm");
> +Components.utils.import("resource://gre/modules/Timer.jsm");

Move this inside whenHiddenPrivateWindowReady, please.

@@ +47,5 @@
> +
> +  whenHiddenPrivateWindowReady: function pbu_whenHiddenPrivateWindowReady(cb) {
> +    let win = Services.appShell.hiddenPrivateDOMWindow;
> +    function isNotLoaded() {
> +      return ["complete", "interactive"].indexOf(win.document.readyState) == -1;

Cute!

@@ +50,5 @@
> +    function isNotLoaded() {
> +      return ["complete", "interactive"].indexOf(win.document.readyState) == -1;
> +    }
> +    if (isNotLoaded()) {
> +      setTimeout(function poll() {

How does this even work?!  Did you mean to say Timer.setTimeout?

@@ +56,5 @@
> +          setTimeout(poll, 100);
> +          return;
> +        }
> +        cb(Services.appShell.hiddenPrivateDOMWindow);
> +      }, 100);

I'm inclined to ask you to use a lower timer value...  Something like 4ms perhaps, which is the lowest we get?  100ms is a long time!
Attachment #725098 - Flags: review?(ehsan) → review+
>@@ +50,5 @@
>> +    function isNotLoaded() {
>> +      return ["complete", "interactive"].indexOf(win.document.readyState) == -1;
>> +    }
>> +    if (isNotLoaded()) {
>> +      setTimeout(function poll() {
>
>How does this even work?!  Did you mean to say Timer.setTimeout?

The JSM exports setTimeout and setInterval, not a global Timer object.
(In reply to comment #8)
> >@@ +50,5 @@
> >> +    function isNotLoaded() {
> >> +      return ["complete", "interactive"].indexOf(win.document.readyState) == -1;
> >> +    }
> >> +    if (isNotLoaded()) {
> >> +      setTimeout(function poll() {
> >
> >How does this even work?!  Did you mean to say Timer.setTimeout?
> 
> The JSM exports setTimeout and setInterval, not a global Timer object.

OK, that is so unlike JSMs!  ;-)
Backed out for mochitest-other failures. Y U NO TRY?
https://hg.mozilla.org/integration/mozilla-inbound/rev/c45d34db0d69

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

12:20:56     INFO -  3791 INFO TEST-START | chrome://mochitests/content/chrome/docshell/test/chrome/test_private_hidden_window.html
12:20:56     INFO -  3792 ERROR TEST-UNEXPECTED-FAIL | chrome://mochitests/content/chrome/docshell/test/chrome/test_private_hidden_window.html | uncaught exception - ReferenceError: Timer is not defined at resource://gre/modules/PrivateBrowsingUtils.jsm:48
12:20:56     INFO -  3793 INFO TEST-END | chrome://mochitests/content/chrome/docshell/test/chrome/test_private_hidden_window.html | finished in 45ms
https://hg.mozilla.org/mozilla-central/rev/9e036c0cf981
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 22
Does this bug break panels launched from an add-on bar icon?
out of curiosity, why is it not possible to listen to a DOMContentLoaded or load event on this document?

can we make a bug about that?
Flags: needinfo?(josh)
When the window loads, the inner window is replaced and the listener is overwritten before it fires.
Flags: needinfo?(josh)
You need to log in before you can comment on or make changes to this bug.