Closed Bug 963358 Opened 10 years ago Closed 9 years ago

[e10s] Crashed tab's "Try again" button does not always reload tabs

Categories

(Firefox :: Tabbed Browser, defect)

defect
Not set
normal

Tracking

()

VERIFIED FIXED
Firefox 36
Tracking Status
e10s m5+ ---
firefox36 --- affected

People

(Reporter: cpeterson, Assigned: mconley)

References

(Blocks 1 open bug)

Details

(Whiteboard: [Workaround and STR in comment 13])

Attachments

(1 file, 1 obsolete file)

This is an intermittent problem.
Blocks: e10s-it2
tracking-e10s: --- → +
Assignee: nobody → ally
I think I noticed this while working on bug 961360 as well.
The intermittency of this problem might be what Tom described on bug 961360 comment 9, i.e. it works for the first window but not for subsequent ones.
Assignee: ally → wmccloskey
I wasn't able to reproduce this, so closing WFM for now. We can re-open if someone sees it again.
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → WORKSFORME
I just had this problem. But I can't make these tabs crash again... Happened for twitter.com, transip.nl, newsblur.com. For some sites it worked after a couple of attempts (fetch.spec.whatwg.org).
Status: RESOLVED → REOPENED
Resolution: WORKSFORME → ---
Note also that bug 961360 definitely did not appear fixed. Had to press it for each tab.
I think this bug never got fixed, but was just hidden by other problems from bug 961360. I thought that bug 1057663 would have helped but apparently not. However, I expect this to be fixed by 1065785. Let's keep this open to verify that it gets fixed.

(In reply to Anne (:annevk) from comment #6)
> Note also that bug 961360 definitely did not appear fixed. Had to press it
> for each tab.

Bug 961360 was reversed by bug 1068210.
Depends on: 1065785
I still see this problem.
I also see this still. It's easy for me to reproduce on my Macbook with my tab crasher add-on (https://bugzilla.mozilla.org/attachment.cgi?id=8499038).

My STR:

1) Install Tab Crasher
2) Open two tabs, and browse to a few pages in one
3) Open a second tab and browser to a few pages in it
4) Hit the Tab Crash button in the toolbar to bring all of the tabs down
5) Immediately hit the "Try again" button in the tab you're in.

ER:

Should restore the tab you selected.

AR:

Nothing. browser.js doesn't even see the click event, apparently.

Note that if you switch to the other tab and click Try Again in it, it often works.
I see this too.
BuildID
Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:36.0) Gecko/20100101 Firefox/36.0 ID:20141022030202 CSet: ae4d9b4ff2ee
Further testing reveals that I can reproduce this only for tabs that have flash or silverlight OOP content, other tabs reload just fine.
(Silverlight and Flash currently crashes for me due to https://bugzilla.mozilla.org/show_bug.cgi?id=1087410 )

Steps to reproduce:
Open some Tabs
Browse to http://bubblemark.com/silverlight2.html or http://kurier.at/video/film-at/schedls-filmschau-maze-runner/91521810
click to activate -> crash (due to 1087410) 
Clicking Reload Tab in the tab which hosts content with plugins will do nothing.
Clicking Reload Tab in a Tab without active plugins will work.

Expected Result:
Reload Tab should work after all crashes.

Hope this helps in nailing this issue down.
Flags: needinfo?(cpeterson)
Nice find! I think everyone has seen this bug before but on simple tests no one can reproduce. Given the reliable STR now we should probably re-triage it.

worth checking if Bill has seen this independently with his recent work on plugins.
Flags: needinfo?(wmccloskey)
Blocks: e10s-plugins
Flags: needinfo?(cpeterson)
Summary: [e10s] Crashed tab's "Try again" button does not always reload the tab → [e10s] Crashed tab's "Try again" button does not always reload tabs that have (Flash or Silverlight) plugin content
I wasn't able to reproduce with Martin's steps (I didn't crash when activating those plugins, since bug 1087410 is fixed), but I was able to reproduce with this:

1) Visit http://www.cbc.ca/
2) As the page is half-way through loading, crash the tab with my tab crasher add-on: https://bugzilla.mozilla.org/attachment.cgi?id=8499038
3) Hit "Try again" to try to revive the tab

ER:

Tab should revive

AR:

Tab does not revive, and I still see a floating status bar on the bottom saying that it's still loading content from cbc.ca


Workaround: Type a URL into the URL bar and press enter. This will revive the tab automatically. You can then hit "back" to go back to where you were when you crashed.
Flags: needinfo?(wmccloskey)
Whiteboard: [Workaround and STR in comment 13]
Using tab Crasher as in comment 13 I can still reproduce on the following 2 pages with flash
http://kurier.at/video/film-at/schedls-filmschau-maze-runner/91521810
http://www.cbc.ca/

Works (i.e. Tab revives) for the following pages with silverlight
http://bubblemark.com/silverlight2.html
http://www2.netflix.com/WiPlayer?movieid=70219878&trkid=13462100&tctx=-99%2C-99%2C3159c717-c478-40fd-933b-bd1f6d83d7c2-9771939

BuildID: Mozilla/5.0 (Windows NT 6.3; Win64; x64; rv:36.0) Gecko/20100101 Firefox/36.0 ID:20141114030206 CSet: 7f0d92595432
More Testing: This is reproducible (i.e. tabs do not revive) even if Flash is set to be activated only on click and NOT activated on the page loaded.
Tested with 
http://kurier.at/video/film-at/schedls-filmschau-maze-runner/91521810
http://www.cbc.ca/
Same BuildID as commend 15
No longer blocks: e10s-m1
I can't staaaaand this bug. So I did some digging.

We attach a click event listener in browser.js called BrowserOnClick to handle clicks on certain things, like the Try Again button, and tiles in about:newtab.

That click event listener is added in browser.js's TabProgressListener::onStateChange, under certain conditions. I've determined that in the tabs where Try Again does not work, the reason is because this event listener has not been set.
Instead of attaching / listening for click events in BrowserOnClick for the
Try Again button, we instead bubble up an event when the Try Again button
is clicked. That event has a listener set up for it that is established during
delayedStartup, so it has a very high probability of being set. This bypasses
some error cases where we hadn't successfully attached BrowserOnClick in the past.
Comment on attachment 8527276 [details] [diff] [review]
[e10s] Crashed tab page's Try Again button doesn't always work. r=?

I think this'll make clicking Try Again far more successful far more often.
Attachment #8527276 - Flags: review?(wmccloskey)
(Sorry for stealing, but this was really bugging me)
Assignee: wmccloskey → mconley
Comment on attachment 8527276 [details] [diff] [review]
[e10s] Crashed tab page's Try Again button doesn't always work. r=?

I forgot to add a check that the sender of the event is about:tabcrashed.
Attachment #8527276 - Flags: review?(wmccloskey)
Instead of attaching / listening for click events in BrowserOnClick for the
Try Again button, we instead bubble up an event when the Try Again button
is clicked. That event has a listener set up for it that is established during
delayedStartup, so it has a very high probability of being set. This bypasses
some error cases where we hadn't successfully attached BrowserOnClick in the past.
Attachment #8527276 - Attachment is obsolete: true
Comment on attachment 8527754 [details] [diff] [review]
[e10s] Crashed tab page's Try Again button doesn't always work. r=?

Added about:tabcrashed check (though it seems that arbitrary content firing an event named AboutTabCrashedTryAgain wouldn't normally get handled in browser.js anyway).
Attachment #8527754 - Flags: review?(wmccloskey)
Comment on attachment 8527754 [details] [diff] [review]
[e10s] Crashed tab page's Try Again button doesn't always work. r=?

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

Thanks!
Attachment #8527754 - Flags: review?(wmccloskey) → review+
Thanks for the fast review, billm:

remote:   https://hg.mozilla.org/integration/fx-team/rev/16a220a579a6
Whiteboard: [Workaround and STR in comment 13] → [Workaround and STR in comment 13][fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/16a220a579a6
Status: REOPENED → RESOLVED
Closed: 10 years ago9 years ago
Resolution: --- → FIXED
Whiteboard: [Workaround and STR in comment 13][fixed-in-fx-team] → [Workaround and STR in comment 13]
Target Milestone: --- → Firefox 36
Comment on attachment 8527754 [details] [diff] [review]
[e10s] Crashed tab page's Try Again button doesn't always work. r=?

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

::: browser/base/content/browser.js
@@ +1159,5 @@
> +      if (event.detail.sendCrashReport) {
> +        TabCrashReporter.submitCrashReport(browser);
> +      }
> +#endif
> +      let tab = gBrowser.getTabForBrowser(browser);

It's too bad we couldn't use gBrowser._getTabForContentWindow, since gBrowser.getBrowserForDocument gets the tab and then returns tab.linkedBrowser. So this patch actually gets the tab, then through that gets the linked browser, then uses the browser in conjunction with the tab<->browser map to get the tab. Quite round-a-bout :-/

Do you think _getTabForContentWindow should be made public?
Flags: needinfo?(mconley)
Summary: [e10s] Crashed tab's "Try again" button does not always reload tabs that have (Flash or Silverlight) plugin content → [e10s] Crashed tab's "Try again" button does not always reload tabs
(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #27)
> Comment on attachment 8527754 [details] [diff] [review]
> [e10s] Crashed tab page's Try Again button doesn't always work. r=?
> 
> Review of attachment 8527754 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: browser/base/content/browser.js
> @@ +1159,5 @@
> > +      if (event.detail.sendCrashReport) {
> > +        TabCrashReporter.submitCrashReport(browser);
> > +      }
> > +#endif
> > +      let tab = gBrowser.getTabForBrowser(browser);
> 
> It's too bad we couldn't use gBrowser._getTabForContentWindow, since
> gBrowser.getBrowserForDocument gets the tab and then returns
> tab.linkedBrowser. So this patch actually gets the tab, then through that
> gets the linked browser, then uses the browser in conjunction with the
> tab<->browser map to get the tab. Quite round-a-bout :-/
> 
> Do you think _getTabForContentWindow should be made public?

I agree, that's pretty circuitous under the hood.

I'd be fine exposing and using _getTabForContentWindow so long as we document in tabbrowser.xml how expensive iterating a series of CPOWs can be, as well as documenting here in browser.js that we avoid the CPOW look-ups since an about:tabcrashed browser is not going to be remote.

I filed bug 1105450 as a follow-up to look into this.
Flags: needinfo?(mconley)
Flags: qe-verify+
Reproduced with Nightly 36.0a1 from 2014-11-10 with str from comment 13: nothing happens when ‘Try again’ button is clicked.
Verified as fixed on latest Nightly (Build ID: 20150210030231) under Windows 7 64-bit, Ubuntu 14.04 32-bit and Mac OS X 10.9.5 with the same str; since a new style for e10s crash pages was added in bug 1087618, verified that the crashed tab is properly restored by using ‘Restore This Tab’ and ‘Restore All Crashed Tabs’ options and crash reports are submitted accordingly.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.