Unbalanced nsGlobal::Suspend/Resume for iframes

RESOLVED FIXED in Firefox 52

Status

()

defect
RESOLVED FIXED
3 years ago
3 months ago

People

(Reporter: farre, Assigned: bkelly)

Tracking

48 Branch
mozilla52
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox52+ fixed)

Details

Attachments

(2 attachments, 1 obsolete attachment)

Reporter

Description

3 years ago
Setting a timeout from an iframe and navigating away before it fires, then going back in history to the same document where the timeout was set should fire the queued timeout.

https://farre.github.io/tests/tests/suspend/index.html
Reporter

Comment 1

3 years ago
(In reply to Andreas Farre [:farre] from comment #0)
> Setting a timeout from an iframe and navigating away before it fires, then
> going back in history to the same document where the timeout was set should
> fire the queued timeout.
> 
> https://farre.github.io/tests/tests/suspend/index.html

That came out a bit less clear than intended. It's not the iframe that gets navigated, but the top window. The iframe is where the timeout is set though.
[Tracking Requested - why for this release]:
This looks to be rather bad regression. 
Regression from bug 1300659 or bug 1303167.
er, that test fails everywhere for me.
Er, yes I can. I wasn't just waiting for long enough after clicking the button.

[Tracking Requested - why for this release]:
See comment 2
And the testcase is expected to fail in Chrome since it doesn't have bfcache.
Fails in my nightly without bug 1300659, so I don't think it regressed from there.  Almost certainly bug 1303167.
Assignee: nobody → bkelly
Blocks: 1303167
Status: NEW → ASSIGNED
Reporter

Updated

3 years ago
Blocks: 1315260
When we Thaw() the parent window the docshell does not know about the frozen child frame.  So the frame is not getting thawed.
This fixes it locally.  Lets see if anything else breaks:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=5dbcba969fa92e44fcb2ff56e942cc0e85e5007e

I will also convert the demo site from comment 0 to a test in a P2 patch.
Tracking 52+ - as smaug notes in Comment 2 this is a rather bad regression.
Comment on attachment 8809059 [details] [diff] [review]
P1 Make sure we thaw child frames properly. r=smaug

This patch ensures we have setup the child docshells before Thawing the window.  This is necessary for the Thaw() to propagate down to the child frame correctly.

This fixes the test case locally and passes existing tests.
Attachment #8809059 - Flags: review?(bugs)
Attachment #8809059 - Flags: review?(bugs) → review+
This patch adds a wpt test for the case demo'd in comment 0.  It passes with the P1 applied and fails without it.

I also included a test for the non-iframe case.  This passes with or without the P1 patch, but seemed reasonable to include.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=64a966fcfb962467482b6fffb1505d12f3f71734
Attachment #8809451 - Flags: review?(bugs)
Comment on attachment 8809451 [details] [diff] [review]
P2 Add wpt test to verify timers are restored after being loaded from bfcache. r=smaug

>+function delay(win, delay) {
>+  return new Promise(resolve => {
>+    win.setTimeout(_ => {
>+      resolve(win);
>+    }, delay);
>+  });
This won't work in browsers where data: urls are cross-origin, and we'll have that too


>+}
>+
>+function navigate_by_name(win, name) {
>+  win.location = make_post_back_url(name);
>+  return wait_for_message(name).then(_ => {
>+    return win;
>+  });
>+}
This should work since .location is special

>+
>+function go_back(win) {
>+  return new Promise(resolve => {
>+    win.onpagehide = e => resolve(win);
>+    win.history.back();
>+  });
>+}
but this shouldn't work
You'll need to use messaging (postMessage) for these.
Attachment #8809451 - Flags: review?(bugs) → review-
Ok.  Lets use a separate file so its same-origin, then.
Attachment #8809451 - Attachment is obsolete: true
Attachment #8809559 - Flags: review?(bugs)
Attachment #8809559 - Flags: review?(bugs) → review+

Comment 14

3 years ago
Pushed by bkelly@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/dbed090b39a4
P1 Make sure we thaw child frames properly. r=smaug
https://hg.mozilla.org/integration/mozilla-inbound/rev/4d1eb932fa31
P2 Add wpt test to verify timers are restored after being loaded from bfcache. r=smaug

Comment 15

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/dbed090b39a4
https://hg.mozilla.org/mozilla-central/rev/4d1eb932fa31
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.