Bug 1209689 - Crashed tab indicates all tabs have crashed and every tab loads the crashed tab page

So we’ve got this oop-browser-crashed event… does this fire from every <xul:browser>, even if it hadn’t restored yet?

Let’s find out.

Yes, the events do fire. And that means that the message manager and frameloader from each becomes invalidated.

Right now we’ve got a problem with the TabSwitcher. The TabSwitcher is attempting to set the docShell active on the unrestored tab when we switch to it, but at that point, the frame loaders don’t exist.

I think what we should do is expose whether or not a browser is crashed by an attribute on the <xul:browser> itself.

When a tab crashes, set the <xul:tab> crashed attribute to “true”, and set the <xul:browser> crashed attribute to “true”.

When a tab crashes, but the <xul:browser> is not yet restored (browser.__SS_restoreState == TAB_STATE_NEEDS_RESTORE), do not set the <xul:tab> crashed attribute, but do set the <xul:browser> crashed attribute to “true”.

When switching tabs, don’t set the docshell to be active when switching tabs if the … well, I ...

Huh. All this special casing. Only set the <xul:tab> crashed attribute to true if the tab is unrestored, only set the docshell active if the <xul:browser> is unrestored...

It seems like I should be able to connect these somehow.

The old code depended on the about:tabcrashed pages being browsed away from in order to remove browsers from the _crashedBrowsers WeakMap. We’ll need to centralize the logic to account for the possibility of restoration or just flat-out browsing away.

I also seem to be adding in a lot of checks for whether or not the browser is in the crashed-but-restorable state. That doesn’t smell good. :(

There are so many points (like in updateBrowserRemoteness) where we do this check to see if the browser is remote, and then do one thing or another… but we’ve got this new “crashed but restorable” state… blah.

Okay, idea:

Browsers that are restorable must be non-remote. Only upon restoring do we choose to make them remote. That way, they cannot “crash”. This eliminates the state of being crashed but restorable.

Let’s try it! I’m going to stash all of this shit code I just wrote to get here and start from scratch.

So the first step is that we force every tab to be non-remote when we restore a windows tabs.

Re-write regression test to account for changes

Bunch of sessionstore test failures to look at now. :(

1934 INFO TEST-UNEXPECTED-FAIL | browser/components/sessionstore/test/browser_463205.js | value was restored - Got , expected foobar

Stack trace:

chrome://mochikit/content/browser-test.js:test_is:940

chrome://mochitests/content/browser/browser/components/sessionstore/test/browser_463205.js:test_check_urls_before_restoring:23

self-hosted:InterpretGeneratorResume:762

self-hosted:next:720

Tester_execTest@chrome://mochikit/content/browser-test.js:754:9

Tester.prototype.nextTest</<@chrome://mochikit/content/browser-test.js:676:7

The problem here in this test seems to be the transition from remote to non-remote tabs. We start by opening up an about:blank tab, which will run in a “remote” tab in e10s. Then we restore the tab to a chrome:// URL which makes it non-remote. When we do that, I think we fail to restore the form fields properly. Or something.

Also, debugging, this looks like maybe some kind of race…

So break it down into small steps:

A background tab browser is remote
The background tab browser is restored to a non-remote page with some form history
The background tab, even when selected, does not get that form history.

The change I made to restoreTab seems to be introducing the problem.

This bit:

if (restoreImmediately || tabbrowser.selectedBrowser == browser) {
tabbrowser.updateBrowserRemotenessByURL(browser, uri);

}

Before, that wasn’t in a brace.

Why did I add that? Well, I wanted to make it so that when we’re doing a window restore, that we don’t flip remoteness of all of the background tabs. Like, I wanted unrestored tabs to be non-remote.

And I think the problem is because we _do_ do the remoteness switch, but after the point where we’ve queued a message to the page, so it gets cut off, and that’s why the form history stuff doesn’t show up.

So how do I reconcile this?

Is the test testing something realistic? Like, do we care about this breakage? Maybe, yeah - there are a few cases where we might use the form history stuff for non-remote pages, like about:tabcrashed after bug 1212065 .

More precise formulation of the problem:

When we have a remote background tab browser, and we call setTabState on it to a non-remote page, it’ll do a remoteness switch too late to actually have the state manifested in the content.

In fact, this line I’ve highlighted, where we only sometimes update the remoteness, that’s introducing all of the test failures, full stop. So I’m in a pretty critical section of code, and I think I need some help.

When the browser detects that it needs to switch remoteness from a URI load, it’ll call SessionStore.navigateAndRestore in order to preserve history.

1:20 PM < mconley > My idea is that if we restore a window with more than one tab, then the unrestored background tabs be made non-remote - deferring the remoteness switch until the tab is selected

1:20 PM < mconley > that way, background, unrestored tabs cannot crash

1:21 PM < mconley > and that seems to work

1:21 PM < mconley > ("cannot crash", when the content process crashes. Of course the main process can crash.)

1:21 PM < mconley > here's the test that is failing: https://dxr.mozilla.org/mozilla-central/source/browser/components/sessionstore/test/browser_463205.js

1:21 PM < mconley > well, one of the tests that are failing, but I think this illustrates the problem I've introduced nicely

1:22 PM < mconley > here's the chunk that's introduced the problem: https://reviewboard.mozilla.org/r/22503/diff/1#0.18

1:22 PM < mconley > where I don't set the remoteness if the browser isn't selected, or if we've not been asked to restore immediately

1:23 PM < Mossop > What in that test fails?

1:23 PM < mconley > the test fails with --e10s, because when it creates that first about:blank, it's in a non-remote background tab as I've set it up

1:24 PM < mconley > (the rest of this is my interpretation of what's happening) - then it starts to actually load about:blank which causes the tab to go remote, since about:blank can load in the content process

1:25 PM < mconley > then the test points the browser at a chrome URL, which causes a remoteness flip

1:25 PM davidb|afk → davidb

1:26 PM < mconley > promiseTabState calls setTabState, which is going to eventually restoreTab, but the tab is background so it doesn't update the remoteness, and the form history goes down to the remote about:blank browser

1:26 PM < mconley > but then the browser tries to load the chrome URI, decides it needs to flip remoteness

1:26 PM < Mossop > So. The chunk you point at creates a tab, but that test wouldn't ever go through that because it creates its own tab and just restores state to it

1:27 PM < Mossop > I think it is the last chunk that is the problem

1:27 PM < mconley > https://reviewboard.mozilla.org/r/22503/diff/1#0.18 <-- this last chunk, where I flip the remoteness in the conditional?

1:27 PM < Mossop > I would guess so

1:28 PM < mconley > yeah, getting rid of the conditional fixes the failing tests

1:28 PM < mconley > like, without a doubt, it's that change in logic that's introducing the problem

1:28 PM < Mossop > Let me think for a moment

1:28 PM < mconley > k

1:33 PM < Mossop > mconley: What is the pref for making tabs wait to load?

1:33 PM < mconley > Mossop: browser.sessionstore.restore_on_demand

1:34 PM < Mossop > Ok

1:35 PM < Mossop > mconley: So I think I can see how you might make this work but I don't know if it will break other things

1:35 PM < mconley > I guess that's kinda my fear too

1:35 PM ⇐ davidb quit (davidb@66.207.193.21) Connection closed

1:35 PM < mconley > like

1:35 PM < Mossop > It also might be a bunch of work

1:36 PM < mconley > I'm worried this is one of those assumption changes that will break things subtly

1:36 PM → davidb joined (davidb@66.207.193.21)

1:36 PM < Mossop > mconley: You need to move the browser remoteness switch down to here https://dxr.mozilla.org/mozilla-central/source/browser/components/sessionstore/SessionStore.jsm#3032 so it happens just before a tab is getting restored

1:36 PM < Mossop > But that also means moving the history restore down to there too

1:37 PM < Mossop > Which means background tabs won't have their history available until they are restored

1:37 PM < Mossop > Maybe that isn't a problem?

1:37 PM < Mossop > OR you could just re-restore the history after switching the remoteness I guess

1:37 PM < Mossop > Maybe that might work

1:37 PM < mconley > let me try some of this stuff

1:37 PM < mconley > thanks for the tips

1:38 PM < mconley > I'm starting to get more familiar with SessionStore, but parts are still quite new to me

1:40 PM < Mossop > mconley: Actually...

1:41 PM < mconley > mm?

1:41 PM < Mossop > mconley: Background tabs get their content restored here: https://dxr.mozilla.org/mozilla-central/source/browser/components/sessionstore/SessionStore.jsm#3101 so that is where you would need to do the remoteness shift and re-restore history

1:41 PM < Mossop > Or at least queued tabs I guess. Maybe background tabs get restored elsewhere. This is very confusing

1:44 PM < mconley > well

1:44 PM < mconley > there's like, two different kind of restorations it seems

1:44 PM < mconley > there's restoring the tabs

1:44 PM < mconley > and there's restoring the tab content

1:45 PM < felipe > i wonder if we could fix the root of the problem which is that unrestored tabs actually have about:blank loaded in the child. But I imagine there are tons of assumptions around that

1:46 PM < Mossop > mconley: Looks like restoreTab restores history then for the current tab restores content and other tabs go into a queue (which never runs if restore_on_demand is set). The queue restores tab content where I linked. Not sure where restore_on_demand tabs get restored

1:46 PM < mconley > Mossop: onTabSelect

1:47 PM < mconley > felipe: if you could have the unrestored tabs load something else, what would they load?

1:48 PM < felipe > mconley: I was wondering if there's a browser that can have "nothing" loaded.. or if we could make unrestored tabs not have a browser until they are loaded

1:49 PM < mconley > hmmm... that rings a bell. I think I'm actually Cc'd on a long-standing bug to delay browser instantiation on unrestored background tabs

1:49 PM — mconley looks for it

1:49 PM < mconley > bug 906076

1:51 PM < Mossop > I don't know if that would make this any easier, you'd still need to muck with session restore in the same way for that too

1:51 PM < mconley > true - and it looks like this patch needs quite a bit more hammering out anyways

1:53 PM < felipe > yeah.. nice to see that there's an attempt in progress in that bug! that looks like a much wider change

Okay… I think I see what I need to do.

I need to defer the flipping of the remoteness (and _all_ communication through the browser’s message manager) until we restore the tab content. Let’s try that.

GARRR. There’s a problem with that too.

SessionStore only shows us the tab titles on unrestored tabs after the history has been restored in each tab.

So we actually have to restore the history in each tab. DAMN IT.

Okay, let’s think about that...

What we really want to do, I guess, is to restore the history for all tabs ever, but make sure that they all stay non-remote.

Once the history has been updated, and we hear back that the history has been updated, can we flip the remoteness and then restore the content.

Let’s try that.

So what should the order of events be here?:

We restoreTabs in all windows
Each tab gets SessionStore:restoreHistory sent down to it so that we display the tab titles correctly? The selected browser gets restoreTabContent called on it.
restoreTabContent will update the remoteness on the selected browser as appropriate, and then send down the restoreTabContent message?

And how does this relate to navigateAndRestore?

What exactly is navigateAndRestore doing, and can I abstract some of that work out into doing whatever it is that I need restoreTabContent to do now?

Question: Why do we wait until we get a restoreHistoryComplete message back from the child before updating the tab title? If there’s no good reason, maybe we can avoid sending the SessionStore:restoreHistory down to unrestored tabs until we restoreTabContent. Is that true?

If so, what would that look like?

Move setting the tab title into restoreTab so that it occurs immediately
Move the restoration of history so that it occurs just before restoring content? Or combine these two?

Man… this is hard. I’d need to consult with ttaubert, I think.

While I’m waiting to talk to ttaubert, let’s keep thinking about this.

It seems silly to send down the history to the tabs that’ll just convert to remote on selection.

What if we revert the change so that unrestored tabs are still remote…but when they crash, instead of sending them to about:tabcrashed, we send them to about:blank, and then restore each of them?

I want to fully understand the test failure in browser_463205.js.

Logging messages:

Restore a tab (but not restore its content)

Update the remoteness from remote to non-remote for the chrome URI

Restore the tab content

A remote browser is opened at about:blank
We set tab state on that remote browser and say that it’s supposed to be pointed at a chrome URI, and we send down form data along with it
The tab state is received by the remote browser, and it attempts to navigate to the chrome URI, and queues up the job of setting the form data once the page has loaded.
The content process realizes that it cannot view the chrome URI and sends a message to the parent saying to flip the remoteness
The parent asks the child for all of its history. This occurs before the page has loaded and the form data filled out? That’s why the form data is empty.
The parent receives the data including the empty form data, and then does the remoteness flip, and sends down the new history data.

I believe the race is: between steps 3-5, the content process is holding onto the form data in some intermediary variable but will not have that information sent back up to the parent in 5 before the page has finished loading. So the form history data is lost.

Actually, now that I think about it, I don’t think it’s much of a race, since the content process has no chance of filling in the form history stuff because the chrome URI cannot possibly be loaded in the content process, so it’s not possible to read the form in the content to persist history.

There is NO RACE HERE. At least, that’s my current hypothesis.

We could potentially fix this by having content-SessionStore fish out the aborted form data that we were going to restore with out from the content process.

Let’s look at these other failing tests to see if they’re hitting the same problem of attempting to grab formdata from a unrestored URI that cannot be loaded in the content process.

SimpleTest.waitForFocus/waitForFocusInner/focusedOrLoaded/<@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:735:59

1935 INFO TEST-UNEXPECTED-FAIL | browser/components/sessionstore/test/browser_467409-backslashplosion.js | Uncaught exception - at chrome://mochitests/content/browser/browser/components/sessionstore/test/browser_467409-backslashplosion.js:60 - TypeError: formdata is undefined

Stack trace:

checkState@chrome://mochitests/content/browser/browser/components/sessionstore/test/browser_467409-backslashplosion.js:60:3

Handler.prototype.process@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:934:23

this.PromiseWalker.walkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:813:7

Promise*this.PromiseWalker.scheduleWalkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:744:11

this.PromiseWalker.schedulePromise@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:776:7

this.PromiseWalker.completePromise@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:711:7

listener@chrome://mochitests/content/browser/browser/components/sessionstore/test/head.js:500:7

ssi_sendTabRestoredNotification@resource:///modules/sessionstore/SessionStore.jsm:3708:5

receiveMessage@resource:///modules/sessionstore/SessionStore.jsm:806:9

Tester_execTest@chrome://mochikit/content/browser-test.js:754:9

Tester.prototype.nextTest</<@chrome://mochikit/content/browser-test.js:676:7

SimpleTest.waitForFocus/waitForFocusInner/focusedOrLoaded/<@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:735:59

Tester_execTest@chrome://mochikit/content/browser-test.js:754:9

Tester.prototype.nextTest</<@chrome://mochikit/content/browser-test.js:676:7

SimpleTest.waitForFocus/waitForFocusInner/focusedOrLoaded/<@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:735:59

1936 INFO TEST-UNEXPECTED-FAIL | browser/components/sessionstore/test/browser_467409-backslashplosion.js | Found an unexpected tab at the end of test run: about:sessionrestore -

1937 INFO TEST-UNEXPECTED-FAIL | browser/components/sessionstore/test/browser_615394-SSWindowState_events.js | undefined assertion name - Got 2, expected 1

Stack trace:

chrome://mochikit/content/browser-test.js:test_is:940

chrome://mochitests/content/browser/browser/components/sessionstore/test/browser_615394-SSWindowState_events.js:onSSTabRestored:102

resource:///modules/sessionstore/SessionStore.jsm:ssi_sendTabRestoredNotification:3708

resource:///modules/sessionstore/SessionStore.jsm:receiveMessage:806

null:null:0

1938 INFO TEST-UNEXPECTED-FAIL | browser/components/sessionstore/test/browser_615394-SSWindowState_events.js | undefined assertion name - Got 2, expected 1

Stack trace:

chrome://mochikit/content/browser-test.js:test_is:940

chrome://mochitests/content/browser/browser/components/sessionstore/test/browser_615394-SSWindowState_events.js:onSSTabRestored:103

resource:///modules/sessionstore/SessionStore.jsm:ssi_sendTabRestoredNotification:3708

resource:///modules/sessionstore/SessionStore.jsm:receiveMessage:806

null:null:0

1939 INFO TEST-UNEXPECTED-FAIL | browser/components/sessionstore/test/browser_615394-SSWindowState_events.js | undefined assertion name - Got 2, expected 1

Stack trace:

chrome://mochikit/content/browser-test.js:test_is:940

chrome://mochitests/content/browser/browser/components/sessionstore/test/browser_615394-SSWindowState_events.js:onSSTabRestored:231

resource:///modules/sessionstore/SessionStore.jsm:ssi_sendTabRestoredNotification:3708

resource:///modules/sessionstore/SessionStore.jsm:receiveMessage:806

null:null:0

1940 INFO TEST-UNEXPECTED-FAIL | browser/components/sessionstore/test/browser_615394-SSWindowState_events.js | undefined assertion name - Got 2, expected 1

Stack trace:

chrome://mochikit/content/browser-test.js:test_is:940

chrome://mochitests/content/browser/browser/components/sessionstore/test/browser_615394-SSWindowState_events.js:onSSTabRestored:232

resource:///modules/sessionstore/SessionStore.jsm:ssi_sendTabRestoredNotification:3708

resource:///modules/sessionstore/SessionStore.jsm:receiveMessage:806

null:null:0

1941 INFO TEST-UNEXPECTED-FAIL | browser/components/sessionstore/test/browser_615394-SSWindowState_events.js | [test_setBrowserState] window3 busy event count correct - Got 4, expected 1

Stack trace:

chrome://mochikit/content/browser-test.js:test_is:940

chrome://mochitests/content/browser/browser/components/sessionstore/test/browser_615394-SSWindowState_events.js:test_setBrowserState/<:292

chrome://mochikit/content/browser-test.js:testScope/test_executeSoon/<.run:966

null:null:0

1942 INFO TEST-UNEXPECTED-FAIL | browser/components/sessionstore/test/browser_615394-SSWindowState_events.js | [test_setBrowserState] window3 ready event count correct - Got 4, expected 1

Stack trace:

chrome://mochikit/content/browser-test.js:test_is:940

chrome://mochitests/content/browser/browser/components/sessionstore/test/browser_615394-SSWindowState_events.js:test_setBrowserState/<:294

chrome://mochikit/content/browser-test.js:testScope/test_executeSoon/<.run:966

null:null:0

1943 INFO TEST-UNEXPECTED-FAIL | browser/components/sessionstore/test/browser_615394-SSWindowState_events.js | [test_setBrowserState] window900 busy event count correct - Got 4, expected 1

Stack trace:

chrome://mochikit/content/browser-test.js:test_is:940

chrome://mochitests/content/browser/browser/components/sessionstore/test/browser_615394-SSWindowState_events.js:test_setBrowserState/<:292

chrome://mochikit/content/browser-test.js:testScope/test_executeSoon/<.run:966

null:null:0

1944 INFO TEST-UNEXPECTED-FAIL | browser/components/sessionstore/test/browser_615394-SSWindowState_events.js | [test_setBrowserState] window900 ready event count correct - Got 4, expected 1

Stack trace:

chrome://mochikit/content/browser-test.js:test_is:940

chrome://mochitests/content/browser/browser/components/sessionstore/test/browser_615394-SSWindowState_events.js:test_setBrowserState/<:294

chrome://mochikit/content/browser-test.js:testScope/test_executeSoon/<.run:966

null:null:0

1945 INFO TEST-UNEXPECTED-FAIL | browser/components/sessionstore/test/browser_615394-SSWindowState_events.js | undefined assertion name - Got 4, expected 1

Stack trace:

chrome://mochikit/content/browser-test.js:test_is:940

chrome://mochitests/content/browser/browser/components/sessionstore/test/browser_615394-SSWindowState_events.js:onSSTabRestored:353

resource:///modules/sessionstore/SessionStore.jsm:ssi_sendTabRestoredNotification:3708

resource:///modules/sessionstore/SessionStore.jsm:receiveMessage:806

null:null:0

1946 INFO TEST-UNEXPECTED-FAIL | browser/components/sessionstore/test/browser_615394-SSWindowState_events.js | undefined assertion name - Got 4, expected 1

Stack trace:

chrome://mochikit/content/browser-test.js:test_is:940

chrome://mochitests/content/browser/browser/components/sessionstore/test/browser_615394-SSWindowState_events.js:onSSTabRestored:354

resource:///modules/sessionstore/SessionStore.jsm:ssi_sendTabRestoredNotification:3708

resource:///modules/sessionstore/SessionStore.jsm:receiveMessage:806

null:null:0

These failures are because we’re increasing the amount of times that we are restoring tabs - we restore them once to load them the first time, and restore them again in order to flip their remoteness properly. Sometimes we seem to fire the ready / busy events 4 times instead of 1. I don’t know why that is.

1947 INFO TEST-UNEXPECTED-FAIL | browser/components/sessionstore/test/browser_aboutSessionRestore.js | Test timed out -

1948 INFO TEST-UNEXPECTED-FAIL | browser/components/sessionstore/test/browser_aboutSessionRestore.js | Found a tab after previous test timed out: about:sessionrestore -

1949 INFO TEST-UNEXPECTED-FAIL | browser/components/sessionstore/test/browser_broadcast.js | A promise chain failed to handle a rejection: - at chrome://mochikit/content/browser-test.js:761 - TypeError: this.SimpleTest.isExpectingUncaughtException is not a function

Stack trace:

Tester_execTest/<@chrome://mochikit/content/browser-test.js:761:34

Tester_execTest@chrome://mochikit/content/browser-test.js:754:9

Tester.prototype.nextTest</<@chrome://mochikit/content/browser-test.js:676:7

SimpleTest.waitForFocus/waitForFocusInner/focusedOrLoaded/<@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:735:59

1950 INFO TEST-UNEXPECTED-FAIL | browser/components/sessionstore/test/browser_broadcast.js | Test timed out -

1951 INFO TEST-UNEXPECTED-FAIL | browser/components/sessionstore/test/browser_formdata_format.js | This test contains no passes, no fails and no todos. Maybe it threw a silent exception? Make sure you use waitForExplicitFinish() if you need it. -

SUITE-END | took 206s

MacBook-Pro-3:mozilla-central mikeconley$ ./mach mochitest browser/components/sessionstore/test/browser_463205.js

In an ideal world, all of the backgrounds browsers that are unrestored would be non-remote so that they can’t crash. The remoteness would be flipped on demand. When the parent asks the child to flush its state back up to the parent, the child should be able to remember any form history for unloaded pages because the page couldn’t be loaded? What?

content-SessionStore.js when loading the document can determine that the document being loaded is going to cause a remoteness switch. In that event, can it stuff the form data it was going to restore into the FormDataListener?

Add a way for things to register for onPendingRemotenessSwitch or something onto the ContentRestore instance, and then if before the load it’s clear that we’re going to be flipping remoteness, to signal all of those listeners with the data we were going to restore the content with...

Attack plan to propose to Mossop or ttaubert or both:

In content-SessionStore.js, notice when we’re going to flip remoteness on a page load during content restore and queue up messages for formdata, scroll, etc for stuff we were never able to restore before the flip. That should fix browser_463205.js, browser_467409-backslashplosion.js and browser_formdata_format.js
Update the SSWindowState events test to account for the fact that we will restore background tabs in e10s mode.

All background browsers are about:blank in the parent process. We restore each of them so that they show their favicons and tab titles right away.

The user clicks on one to load on demand.

We should then notice that this browser has the wrong remoteness setting. So we then flush the history back up, merge it with the fields that couldn’t possibly have been set (like formdata, etc), and then blast that down to the newly remote browser before loading content.

IDEA:

restoreTabContent is the choke point, and where all of the action, I think needs to happen. Basically, I think what we can do is determine if the tab content is going to necessitate a remoteness switch. If so, we should then flip the remoteness, re-send the history out of the TabStateCache, send a new epoch, send the load arguments… and THEN send the restoreTabContent message.

So this works, but for some reason, now when remoteness is flipped from navigateAndRestore, I get the following error:

Full message: TypeError: frameLoader.tabParent is null

Full stack: set_docShellIsActive@chrome://global/content/bindings/remote-browser.xml:229:13

updateBrowserRemoteness@chrome://browser/content/tabbrowser.xml:1552:1

updateBrowserRemotenessByURL@chrome://browser/content/tabbrowser.xml:1605:1

SessionStoreInternal.restoreTabContent@resource:///modules/sessionstore/SessionStore.jsm:3056:9

restoreTab@resource:///modules/sessionstore/SessionStore.jsm:3022:7

navigateAndRestore/<@resource:///modules/sessionstore/SessionStore.jsm:2299:7

Handler.prototype.process@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:934:23

this.PromiseWalker.walkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:813:7

Promise*this.PromiseWalker.scheduleWalkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:744:11

this.PromiseWalker.schedulePromise@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:776:7

this.PromiseWalker.completePromise@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:711:7

resolve@resource:///modules/sessionstore/TabStateFlusher.jsm:96:5

resolve@resource:///modules/sessionstore/TabStateFlusher.jsm:34:5

receiveMessage@resource:///modules/sessionstore/SessionStore.jsm:692:11

Why?

Hrm… everything is behaving how I’d expect. Like the browser is reattached to the DOM… why is this not working?

AHH. Okay.

The problem is that when we have a non-remote browser and have nodefaultsrc set on it, flipping the remoteness on it when it hasn’t yet loaded a document will fail spectacularly. Like, I think it’s like:

A new about:blank tab is loaded with nodefaultsrc, because then we load about:newtab in it.

navigateAndRestore causes us to do a remoteness switch on it.

Now we have a non-remote browser with nodefaultsrc on it. And then we click on a tile.

And that causes navigateAndRestore to occur again, and this causes a remoteness switch, and we explode.

And this bug wasn’t introduced by the remoteness switch, interestingly… it must be something else.

Let me make sure I have this down right - because I think I got the steps wrong before.

When the user clicks on the New Tab button, we eventually call loadOneTab, passing in about:newtab. This calls addTab, which notices that about:newtab cannot be actually opened in the content process, so the initial browser that is created is non-remote.

That newly created browser then loads up some content, easy peasy. No need to restore it or anything.

Then, when the user clicks on a link in the about:newtab page, the DocShell checks to see if it can be loaded in another process - which it can! This sends a signal to the parent to do a remoteness switch, which puts us into navigateAndRestore.

Here’s the stack frame in the GOOD case when we set the TabParent:

* thread #1: tid = 0x14b5c4, 0x0000000103bc7ed3 XUL`nsFrameLoader::TryRemoteBrowser(this=0x000000011fe93ae0) + 1603 at nsFrameLoader.cpp:2266, queue = 'com.apple.main-thread', stop reason = breakpoint 7.1

* frame #0: 0x0000000103bc7ed3 XUL`nsFrameLoader::TryRemoteBrowser(this=0x000000011fe93ae0) + 1603 at nsFrameLoader.cpp:2266

frame #1: 0x0000000103bc82a6 XUL`nsFrameLoader::ShowRemoteFrame(this=0x000000011fe93ae0, size=0x00007fff5fbeee38, aFrame=0x000000012c58b020) + 134 at nsFrameLoader.cpp:785

frame #2: 0x0000000103bcb04a XUL`nsFrameLoader::Show(this=0x000000011fe93ae0, marginWidth=-1, marginHeight=-1, scrollbarPrefX=1, scrollbarPrefY=1, frame=0x000000012c58b020) + 202 at nsFrameLoader.cpp:656

frame #3: 0x0000000106445345 XUL`nsSubDocumentFrame::ShowViewer(this=0x000000012c58b020) + 245 at nsSubDocumentFrame.cpp:175

frame #4: 0x0000000106472c1c XUL`AsyncFrameInit::Run(this=0x000000011ff207c0) + 76 at nsSubDocumentFrame.cpp:86

frame #5: 0x0000000103906ad6 XUL`nsContentUtils::RemoveScriptBlocker() + 502 at nsContentUtils.cpp:5194

frame #6: 0x0000000103b88020 XUL`nsDocument::EndUpdate(this=0x000000012c02ba00, aUpdateType=1) + 176 at nsDocument.cpp:4937

frame #7: 0x0000000105c3e795 XUL`mozilla::dom::XULDocument::EndUpdate(this=0x000000012c02ba00, aUpdateType=1) + 37 at XULDocument.cpp:3185

frame #8: 0x000000010391d6fc XUL`mozAutoDocUpdate::~mozAutoDocUpdate(this=0x00007fff5fbef160) + 76 at mozAutoDocUpdate.h:40

frame #9: 0x0000000103919e95 XUL`mozAutoDocUpdate::~mozAutoDocUpdate(this=0x00007fff5fbef160) + 21 at mozAutoDocUpdate.h:38

frame #10: 0x0000000103c3260c XUL`nsINode::ReplaceOrInsertBefore(this=0x000000012f070560, aReplace=false, aNewChild=0x000000012f06fca0, aRefChild=0x0000000000000000, aError=0x00007fff5fbef5d8) + 5356 at nsINode.cpp:2303

and in the bad case?

Ah, well, we definitely don’t get there. Interesting. Why not?

We never get as far as nsFrameLoader::Show for some reason… why not?

The browser that we’re flipping remoteness on is maybe in a weird state or something?

Here’s where we init the nsSubDocumentFrame in the good case:

* thread #1: tid = 0x14f11c, 0x0000000106444b3f XUL`nsSubDocumentFrame::Init(this=0x000000012dcd0a10, aContent=0x000000012018cea0, aParent=0x00000001350ed538, aPrevInFlow=0x0000000000000000) + 31 at nsSubDocumentFrame.cpp:106, queue = 'com.apple.main-thread', stop reason = breakpoint 8.1

* frame #0: 0x0000000106444b3f XUL`nsSubDocumentFrame::Init(this=0x000000012dcd0a10, aContent=0x000000012018cea0, aParent=0x00000001350ed538, aPrevInFlow=0x0000000000000000) + 31 at nsSubDocumentFrame.cpp:106

frame #1: 0x00000001062023a0 XUL`nsCSSFrameConstructor::InitAndRestoreFrame(this=0x000000012a65fa40, aState=0x00007fff5fbf67e8, aContent=0x000000012018cea0, aParentFrame=0x00000001350ed538, aNewFrame=0x000000012dcd0a10, aAllowCounters=true) + 256 at nsCSSFrameConstructor.cpp:4685

frame #2: 0x000000010620a596 XUL`nsCSSFrameConstructor::ConstructFrameFromItemInternal(this=0x000000012a65fa40, aItem=0x000000012018f380, aState=0x00007fff5fbf67e8, aParentFrame=0x00000001350ed538, aFrameItems=0x00007fff5fbf4ae0) + 2630 at nsCSSFrameConstructor.cpp:3697

frame #3: 0x000000010620eed4 XUL`nsCSSFrameConstructor::ConstructFramesFromItem(this=0x000000012a65fa40, aState=0x00007fff5fbf67e8, aIter=0x00007fff5fbf4520, aParentFrame=0x00000001350ed538, aFrameItems=0x00007fff5fbf4ae0) + 564 at nsCSSFrameConstructor.cpp:5868

frame #4: 0x0000000106243d15 XUL`nsCSSFrameConstructor::ConstructFramesFromItemList(this=0x000000012a65fa40, aState=0x00007fff5fbf67e8, aItems=0x00007fff5fbf4760, aParentFrame=0x00000001350ed538, aFrameItems=0x00007fff5fbf4ae0) + 293 at nsCSSFrameConstructor.cpp:10189

frame #5: 0x0000000106202d97 XUL`nsCSSFrameConstructor::ProcessChildren(this=0x000000012a65fa40, aState=0x00007fff5fbf67e8, aContent=0x000000012018e160, aStyleContext=0x000000012eb76a40, aFrame=0x00000001350ed538, aCanHaveGeneratedContent=true, aFrameItems=0x00007fff5fbf4ae0, aAllowBlockStyles=false, aPendingBinding=0x0000000000000000, aPossiblyLeafFrame=0x00000001350ed538) + 2375 at nsCSSFrameConstructor.cpp:10389

frame #6: 0x000000010620acc5 XUL`nsCSSFrameConstructor::ConstructFrameFromItemInternal(this=0x000000012a65fa40, aItem=0x000000012018f100, aState=0x00007fff5fbf67e8, aParentFrame=0x00000001350ede88, aFrameItems=0x00007fff5fbf52f0) + 4469 at nsCSSFrameConstructor.cpp:3808

frame #7: 0x000000010620eed4 XUL`nsCSSFrameConstructor::ConstructFramesFromItem(this=0x000000012a65fa40, aState=0x00007fff5fbf67e8, aIter=0x00007fff5fbf4d30, aParentFrame=0x00000001350ede88, aFrameItems=0x00007fff5fbf52f0) + 564 at nsCSSFrameConstructor.cpp:5868

frame #8: 0x0000000106243d15 XUL`nsCSSFrameConstructor::ConstructFramesFromItemList(this=0x000000012a65fa40, aState=0x00007fff5fbf67e8, aItems=0x00007fff5fbf4f70, aParentFrame=0x00000001350ede88, aFrameItems=0x00007fff5fbf52f0) + 293 at nsCSSFrameConstructor.cpp:10189

frame #9: 0x0000000106202d97 XUL`nsCSSFrameConstructor::ProcessChildren(this=0x000000012a65fa40, aState=0x00007fff5fbf67e8, aContent=0x000000012018e200, aStyleContext=0x000000012ea64588, aFrame=0x00000001350ede88, aCanHaveGeneratedContent=true, aFrameItems=0x00007fff5fbf52f0, aAllowBlockStyles=false, aPendingBinding=0x0000000000000000, aPossiblyLeafFrame=0x00000001350ede88) + 2375 at nsCSSFrameConstructor.cpp:10389

frame #10: 0x000000010620acc5 XUL`nsCSSFrameConstructor::ConstructFrameFromItemInternal(this=0x000000012a65fa40, aItem=0x000000012018f060, aState=0x00007fff5fbf67e8, aParentFrame=0x0000000135599ec0, aFrameItems=0x00007fff5fbf5b00) + 4469 at nsCSSFrameConstructor.cpp:3808

frame #11: 0x000000010620eed4 XUL`nsCSSFrameConstructor::ConstructFramesFromItem(this=0x000000012a65fa40, aState=0x00007fff5fbf67e8, aIter=0x00007fff5fbf5540, aParentFrame=0x0000000135599ec0, aFrameItems=0x00007fff5fbf5b00) + 564 at nsCSSFrameConstructor.cpp:5868

frame #12: 0x0000000106243d15 XUL`nsCSSFrameConstructor::ConstructFramesFromItemList(this=0x000000012a65fa40, aState=0x00007fff5fbf67e8, aItems=0x00007fff5fbf5780, aParentFrame=0x0000000135599ec0, aFrameItems=0x00007fff5fbf5b00) + 293 at nsCSSFrameConstructor.cpp:10189

frame #13: 0x0000000106202d97 XUL`nsCSSFrameConstructor::ProcessChildren(this=0x000000012a65fa40, aState=0x00007fff5fbf67e8, aContent=0x000000012018e340, aStyleContext=0x000000012ea3a3c8, aFrame=0x0000000135599ec0, aCanHaveGeneratedContent=true, aFrameItems=0x00007fff5fbf5b00, aAllowBlockStyles=false, aPendingBinding=0x0000000000000000, aPossiblyLeafFrame=0x0000000135599ec0) + 2375 at nsCSSFrameConstructor.cpp:10389

frame #14: 0x000000010620acc5 XUL`nsCSSFrameConstructor::ConstructFrameFromItemInternal(this=0x000000012a65fa40, aItem=0x000000012018efc0, aState=0x00007fff5fbf67e8, aParentFrame=0x00000001350edbc8, aFrameItems=0x00007fff5fbf6310) + 4469 at nsCSSFrameConstructor.cpp:3808

frame #15: 0x000000010620eed4 XUL`nsCSSFrameConstructor::ConstructFramesFromItem(this=0x000000012a65fa40, aState=0x00007fff5fbf67e8, aIter=0x00007fff5fbf5d50, aParentFrame=0x00000001350edbc8, aFrameItems=0x00007fff5fbf6310) + 564 at nsCSSFrameConstructor.cpp:5868

frame #16: 0x0000000106243d15 XUL`nsCSSFrameConstructor::ConstructFramesFromItemList(this=0x000000012a65fa40, aState=0x00007fff5fbf67e8, aItems=0x00007fff5fbf5f90, aParentFrame=0x00000001350edbc8, aFrameItems=0x00007fff5fbf6310) + 293 at nsCSSFrameConstructor.cpp:10189

frame #17: 0x0000000106202d97 XUL`nsCSSFrameConstructor::ProcessChildren(this=0x000000012a65fa40, aState=0x00007fff5fbf67e8, aContent=0x000000012018e660, aStyleContext=0x000000012ec372d8, aFrame=0x00000001350edbc8, aCanHaveGeneratedContent=true, aFrameItems=0x00007fff5fbf6310, aAllowBlockStyles=false, aPendingBinding=0x000000011d9f68c0, aPossiblyLeafFrame=0x00000001350edbc8) + 2375 at nsCSSFrameConstructor.cpp:10389

frame #18: 0x000000010620acc5 XUL`nsCSSFrameConstructor::ConstructFrameFromItemInternal(this=0x000000012a65fa40, aItem=0x000000012018ee80, aState=0x00007fff5fbf67e8, aParentFrame=0x000000012ecc8150, aFrameItems=0x00007fff5fbf6690) + 4469 at nsCSSFrameConstructor.cpp:3808

frame #19: 0x000000010620eed4 XUL`nsCSSFrameConstructor::ConstructFramesFromItem(this=0x000000012a65fa40, aState=0x00007fff5fbf67e8, aIter=0x00007fff5fbf6560, aParentFrame=0x000000012ecc8150, aFrameItems=0x00007fff5fbf6690) + 564 at nsCSSFrameConstructor.cpp:5868

frame #20: 0x0000000106243d15 XUL`nsCSSFrameConstructor::ConstructFramesFromItemList(this=0x000000012a65fa40, aState=0x00007fff5fbf67e8, aItems=0x00007fff5fbf66b0, aParentFrame=0x000000012ecc8150, aFrameItems=0x00007fff5fbf6690) + 293 at nsCSSFrameConstructor.cpp:10189

frame #21: 0x00000001062153d3 XUL`nsCSSFrameConstructor::ContentAppended(this=0x000000012a65fa40, aContainer=0x000000012ec4c040, aFirstNewContent=0x000000012018e660, aAllowLazyConstruction=true) + 3251 at nsCSSFrameConstructor.cpp:7189

frame #22: 0x00000001062bf181 XUL`PresShell::ContentAppended(this=0x000000012b9ea980, aDocument=0x000000012ba83a00, aContainer=0x000000012ec4c040, aFirstNewContent=0x000000012018e660, aNewIndexInContainer=13) + 417 at nsPresShell.cpp:4308

AH HAH. OH MY GOD I’VE FIGURED IT OUT.

The browser has the “pending” attribute applied to it when we attempt to flip the remoteness. You cannot flip the remoteness of a browser that is not being displayed!

HOORAY! It works! WOOOOOOOOOOO

Except now I’ve got 4 new test failures. But they’re different, and everything else passes! So, forward progress!

22 INFO TEST-UNEXPECTED-FAIL | browser/components/sessionstore/test/browser_replace_load.js | correct URL loaded - Got about:mozilla, expected about:mozilla#fooobar

Stack trace:

chrome://mochikit/content/browser-test.js:test_is:940

chrome://mochitests/content/browser/browser/components/sessionstore/test/browser_replace_load.js:testSwitchToTab<:41

@chrome://mochitests/content/browser/browser/components/sessionstore/test/browser_replace_load.js:16:9

TaskImpl_run@resource://gre/modules/Task.jsm:314:40

TaskImpl@resource://gre/modules/Task.jsm:275:3

createAsyncFunction/asyncFunction@resource://gre/modules/Task.jsm:249:14

Task_spawn@resource://gre/modules/Task.jsm:164:12

TaskImpl_handleResultValue@resource://gre/modules/Task.jsm:381:1

TaskImpl_run@resource://gre/modules/Task.jsm:322:13

TaskImpl@resource://gre/modules/Task.jsm:275:3

createAsyncFunction/asyncFunction@resource://gre/modules/Task.jsm:249:14

Task_spawn@resource://gre/modules/Task.jsm:164:12

Tester_execTest@chrome://mochikit/content/browser-test.js:754:9

Tester.prototype.nextTest</<@chrome://mochikit/content/browser-test.js:676:7

SimpleTest.waitForFocus/waitForFocusInner/focusedOrLoaded/<@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:735:59

23 INFO TEST-UNEXPECTED-FAIL | browser/components/sessionstore/test/browser_replace_load.js | three history entries - Got 2, expected 3

Stack trace:

chrome://mochikit/content/browser-test.js:test_is:940

chrome://mochitests/content/browser/browser/components/sessionstore/test/browser_replace_load.js:testSwitchToTab<:50

@chrome://mochitests/content/browser/browser/components/sessionstore/test/browser_replace_load.js:16:9

TaskImpl_run@resource://gre/modules/Task.jsm:314:40

TaskImpl@resource://gre/modules/Task.jsm:275:3

createAsyncFunction/asyncFunction@resource://gre/modules/Task.jsm:249:14

Task_spawn@resource://gre/modules/Task.jsm:164:12

TaskImpl_handleResultValue@resource://gre/modules/Task.jsm:381:1

TaskImpl_run@resource://gre/modules/Task.jsm:322:13

TaskImpl@resource://gre/modules/Task.jsm:275:3

createAsyncFunction/asyncFunction@resource://gre/modules/Task.jsm:249:14

Task_spawn@resource://gre/modules/Task.jsm:164:12

Tester_execTest@chrome://mochikit/content/browser-test.js:754:9

Tester.prototype.nextTest</<@chrome://mochikit/content/browser-test.js:676:7

SimpleTest.waitForFocus/waitForFocusInner/focusedOrLoaded/<@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:735:59

24 INFO TEST-UNEXPECTED-FAIL | browser/components/sessionstore/test/browser_replace_load.js | correct URL loaded - Got about:mozilla, expected about:mozilla?foo=bar

Stack trace:

chrome://mochikit/content/browser-test.js:test_is:940

chrome://mochitests/content/browser/browser/components/sessionstore/test/browser_replace_load.js:testSwitchToTab<:41

@chrome://mochitests/content/browser/browser/components/sessionstore/test/browser_replace_load.js:17:9

TaskImpl_run@resource://gre/modules/Task.jsm:314:40

Handler.prototype.process@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:934:23

this.PromiseWalker.walkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:813:7

Promise*this.PromiseWalker.scheduleWalkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:744:11

this.PromiseWalker.schedulePromise@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:776:7

this.PromiseWalker.completePromise@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:711:7

receiveMessage@resource://testing-common/ContentTask.jsm:94:7

25 INFO TEST-UNEXPECTED-FAIL | browser/components/sessionstore/test/browser_replace_load.js | three history entries - Got 2, expected 3

Stack trace:

chrome://mochikit/content/browser-test.js:test_is:940

chrome://mochitests/content/browser/browser/components/sessionstore/test/browser_replace_load.js:testSwitchToTab<:50

@chrome://mochitests/content/browser/browser/components/sessionstore/test/browser_replace_load.js:17:9

TaskImpl_run@resource://gre/modules/Task.jsm:314:40

Handler.prototype.process@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:934:23

this.PromiseWalker.walkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:813:7

Promise*this.PromiseWalker.scheduleWalkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:744:11

this.PromiseWalker.schedulePromise@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:776:7

this.PromiseWalker.completePromise@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:711:7

receiveMessage@resource://testing-common/ContentTask.jsm:94:7

Create remote about:blank browser.

setTabState causes a restoreTab at about:mozilla

We send state down to the content process

We attempt to switch to the tab to about:mozilla#fooobar

But then the browser doesn’t have the right URL (expected: about:mozilla#fooobar, and got about:mozilla).

Is it a race? Doesn’t look like it - looks like honest to goodness full-time breakage.

This test was added for bug 1100223 . Let’s take a look at the fix in that bug that this test tests for.

Ahhhhhhh I see.

In this case, a loadURI is sent to the browser, and then the selectedIndex is flipped.

This is a problem.

If an unrestored tab has loadURI called on it, I’m guessing it’ll do the work of asking for the restore. However, when we switch tabs, we might do a remoteness switch.

How does this behave with non-e10s? Let’s use the debugger and look at restoreTabContent...

Ah… in the non-e10s case, restoreTabContent is never called. In the e10s case, it is I guess because we’re doing navigateAndRestore from the initial about:blank?

Ohhhhh this is interesting. We’re entering restoreTabContent because it looks to us as if the browser needs restoring. And… yet, the tab’s restore state is… not defined. So how did we get here? Interesting!

Now that’s an honest-to-goodness mystery. How the hell are we getting there?

LET’S FIND OUT!!!!!

I’m going to make __SS_restoreState a getter / setter on browsers that dump stacks. That might illuminate what the heck is going on here.

Wait… why doesn’t restoreTabContent get called for the non-e10s case? I think that’s where the real mystery is. I’m certain restoreTab is… right?

Ah… it IS some kind of race. Something to do with TabRestoreQueue?

Ok, I can solve this by flushing the TabStateFlusher before restoring the content….

AAAAND NOW MORE TEST FAILURES. DAMN IT.

1976 INFO TEST-UNEXPECTED-FAIL | browser/components/sessionstore/test/browser_586068-cascade.js | load 1 - # tabs that need to be restored - Got 0, expected 3

Stack trace:

chrome://mochikit/content/browser-test.js:test_is:940

chrome://mochitests/content/browser/browser/components/sessionstore/test/browser_586068-cascade.js:test/promiseRestoringTabs</<:37

chrome://mochitests/content/browser/browser/components/sessionstore/test/head.js:gProgressListener.onRestored:368

chrome://mochitests/content/browser/browser/components/sessionstore/test/head.js:gProgressListener.observe:362

resource:///modules/sessionstore/SessionStore.jsm:receiveMessage:801

1977 INFO TEST-UNEXPECTED-FAIL | browser/components/sessionstore/test/browser_586068-cascade.js | load 1 - # tabs that are restoring - Got 6, expected 3

Stack trace:

chrome://mochikit/content/browser-test.js:test_is:940

chrome://mochitests/content/browser/browser/components/sessionstore/test/browser_586068-cascade.js:test/promiseRestoringTabs</<:38

chrome://mochitests/content/browser/browser/components/sessionstore/test/head.js:gProgressListener.onRestored:368

chrome://mochitests/content/browser/browser/components/sessionstore/test/head.js:gProgressListener.observe:362

resource:///modules/sessionstore/SessionStore.jsm:receiveMessage:801

1978 INFO TEST-UNEXPECTED-FAIL | browser/components/sessionstore/test/browser_586068-cascade.js | load 2 - # tabs that need to be restored - Got 0, expected 2

Stack trace:

chrome://mochikit/content/browser-test.js:test_is:940

chrome://mochitests/content/browser/browser/components/sessionstore/test/browser_586068-cascade.js:test/promiseRestoringTabs</<:37

chrome://mochitests/content/browser/browser/components/sessionstore/test/head.js:gProgressListener.onRestored:368

chrome://mochitests/content/browser/browser/components/sessionstore/test/head.js:gProgressListener.observe:362

resource:///modules/sessionstore/SessionStore.jsm:receiveMessage:801

1979 INFO TEST-UNEXPECTED-FAIL | browser/components/sessionstore/test/browser_586068-cascade.js | load 2 - # tabs that are restoring - Got 5, expected 3

Stack trace:

chrome://mochikit/content/browser-test.js:test_is:940

chrome://mochitests/content/browser/browser/components/sessionstore/test/browser_586068-cascade.js:test/promiseRestoringTabs</<:38

chrome://mochitests/content/browser/browser/components/sessionstore/test/head.js:gProgressListener.onRestored:368

chrome://mochitests/content/browser/browser/components/sessionstore/test/head.js:gProgressListener.observe:362

resource:///modules/sessionstore/SessionStore.jsm:receiveMessage:801

1980 INFO TEST-UNEXPECTED-FAIL | browser/components/sessionstore/test/browser_586068-cascade.js | load 3 - # tabs that need to be restored - Got 0, expected 1

Stack trace:

chrome://mochikit/content/browser-test.js:test_is:940

chrome://mochitests/content/browser/browser/components/sessionstore/test/browser_586068-cascade.js:test/promiseRestoringTabs</<:37

chrome://mochitests/content/browser/browser/components/sessionstore/test/head.js:gProgressListener.onRestored:368

chrome://mochitests/content/browser/browser/components/sessionstore/test/head.js:gProgressListener.observe:362

resource:///modules/sessionstore/SessionStore.jsm:receiveMessage:801

1981 INFO TEST-UNEXPECTED-FAIL | browser/components/sessionstore/test/browser_586068-cascade.js | load 3 - # tabs that are restoring - Got 4, expected 3

Stack trace:

chrome://mochikit/content/browser-test.js:test_is:940

chrome://mochitests/content/browser/browser/components/sessionstore/test/browser_586068-cascade.js:test/promiseRestoringTabs</<:38

chrome://mochitests/content/browser/browser/components/sessionstore/test/head.js:gProgressListener.onRestored:368

chrome://mochitests/content/browser/browser/components/sessionstore/test/head.js:gProgressListener.observe:362

resource:///modules/sessionstore/SessionStore.jsm:receiveMessage:801

1982 INFO TEST-UNEXPECTED-FAIL | browser/components/sessionstore/test/browser_595601-restore_hidden.js | restoring 3 tabs concurrently - Got 8, expected 3

Stack trace:

chrome://mochikit/content/browser-test.js:test_is:940

chrome://mochitests/content/browser/browser/components/sessionstore/test/browser_595601-restore_hidden.js:test_loadTabs/<:39

chrome://mochitests/content/browser/browser/components/sessionstore/test/browser_595601-restore_hidden.js:TabsProgressListener.onRestored:81

chrome://mochitests/content/browser/browser/components/sessionstore/test/browser_595601-restore_hidden.js:TabsProgressListener.observe:76

resource:///modules/sessionstore/SessionStore.jsm:receiveMessage:801

1983 INFO TEST-UNEXPECTED-FAIL | browser/components/sessionstore/test/browser_595601-restore_hidden.js | restoring max. 3 tabs concurrently -

Stack trace:

chrome://mochitests/content/browser/browser/components/sessionstore/test/browser_595601-restore_hidden.js:test_loadTabs/<:41

chrome://mochitests/content/browser/browser/components/sessionstore/test/browser_595601-restore_hidden.js:TabsProgressListener.onRestored:81

chrome://mochitests/content/browser/browser/components/sessionstore/test/browser_595601-restore_hidden.js:TabsProgressListener.observe:76

resource:///modules/sessionstore/SessionStore.jsm:receiveMessage:801

1984 INFO TEST-UNEXPECTED-FAIL | browser/components/sessionstore/test/browser_595601-restore_hidden.js | restoring max. 3 tabs concurrently -

Stack trace:

chrome://mochitests/content/browser/browser/components/sessionstore/test/browser_595601-restore_hidden.js:test_loadTabs/<:41

chrome://mochitests/content/browser/browser/components/sessionstore/test/browser_595601-restore_hidden.js:TabsProgressListener.onRestored:81

chrome://mochitests/content/browser/browser/components/sessionstore/test/browser_595601-restore_hidden.js:TabsProgressListener.observe:76

resource:///modules/sessionstore/SessionStore.jsm:receiveMessage:801

1985 INFO TEST-UNEXPECTED-FAIL | browser/components/sessionstore/test/browser_595601-restore_hidden.js | restoring max. 3 tabs concurrently -

Stack trace:

chrome://mochitests/content/browser/browser/components/sessionstore/test/browser_595601-restore_hidden.js:test_loadTabs/<:41

chrome://mochitests/content/browser/browser/components/sessionstore/test/browser_595601-restore_hidden.js:TabsProgressListener.onRestored:81

chrome://mochitests/content/browser/browser/components/sessionstore/test/browser_595601-restore_hidden.js:TabsProgressListener.observe:76

resource:///modules/sessionstore/SessionStore.jsm:receiveMessage:801

1986 INFO TEST-UNEXPECTED-FAIL | browser/components/sessionstore/test/browser_595601-restore_hidden.js | restoring max. 3 tabs concurrently -

Stack trace:

chrome://mochitests/content/browser/browser/components/sessionstore/test/browser_595601-restore_hidden.js:test_loadTabs/<:41

chrome://mochitests/content/browser/browser/components/sessionstore/test/browser_595601-restore_hidden.js:TabsProgressListener.onRestored:81

chrome://mochitests/content/browser/browser/components/sessionstore/test/browser_595601-restore_hidden.js:TabsProgressListener.observe:76

resource:///modules/sessionstore/SessionStore.jsm:receiveMessage:801

1987 INFO TEST-UNEXPECTED-FAIL | browser/components/sessionstore/test/browser_595601-restore_hidden.js | restoring 3 tabs concurrently - Got 4, expected 3

Stack trace:

chrome://mochikit/content/browser-test.js:test_is:940

chrome://mochitests/content/browser/browser/components/sessionstore/test/browser_595601-restore_hidden.js:test_loadTabs/<:39

chrome://mochitests/content/browser/browser/components/sessionstore/test/browser_595601-restore_hidden.js:TabsProgressListener.onRestored:81

chrome://mochitests/content/browser/browser/components/sessionstore/test/browser_595601-restore_hidden.js:TabsProgressListener.observe:76

resource:///modules/sessionstore/SessionStore.jsm:receiveMessage:801

1988 INFO TEST-UNEXPECTED-FAIL | browser/components/sessionstore/test/browser_636279.js | restoring 3 tabs concurrently - Got 4, expected 3

Stack trace:

chrome://mochikit/content/browser-test.js:test_is:940

chrome://mochitests/content/browser/browser/components/sessionstore/test/browser_636279.js:test/onReady/<:36

chrome://mochitests/content/browser/browser/components/sessionstore/test/browser_636279.js:TabsProgressListener.onRestored:98

chrome://mochitests/content/browser/browser/components/sessionstore/test/browser_636279.js:TabsProgressListener.observe:93

resource:///modules/sessionstore/SessionStore.jsm:receiveMessage:801

1989 INFO TEST-UNEXPECTED-FAIL | browser/components/sessionstore/test/browser_682507.js | second tab should have not 'pending' attribute -

Stack trace:

chrome://mochitests/content/browser/browser/components/sessionstore/test/browser_682507.js:test:12

chrome://mochikit/content/browser-test.js:Tester_execTest:783

chrome://mochikit/content/browser-test.js:Tester.prototype.nextTest</<:676

chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:SimpleTest.waitForFocus/waitForFocusInner/focusedOrLoaded/<:735

SUITE-END | took 108s

I’m now less confident that I want to use TabStateFlusher to ensure that the state has been properly set… let’s see here.

A remote browser is created pointed at about:blank.

setTabState is called, and via the message sent from restoreTab, the child realizes that it’s supposed to be loading in the a non-remote browser, so calls navigateAndRestore, passing the about:mozilla#fooobar URI argument with it.

navigateAndRestore flushes the TabStateFlusher, and we return to the event loop...

The tab is then selected?

Assumptions that I had wrong:

I thought the “select” events on e10s tabs would be fired only after the frame was ready. This is false. Therefore, the TabSelect events fire synchronously on tab selection .

THEORY:

navigateAndRestore is determined to be the right call in the parent process on tab switch. We kick off navigateAndRestore, which kicks off the async TabStateFlusher.flush operation.

THEN, we flip the tabs, and synchronously decide that the tab content needs to be restored, so we call into restoreTabContent.

restoreTabContent realizes that the tab needs to have its remoteness flipped, does it, and then resends down the history and then the command to restore content, minus the load arguments that had been sent to navigateAndRestore.

So that’s why we don’t load up about:mozilla#fooobar

So what are we going to do about it?

Hm… let me try adding a TAB_STATE_WILL_RESTORE state for __SS_restoreState, to guard against the TabSelect event handler trying to call restoreTabContent when we’ve got a Flush pending..

OH MY GOD IT WORKS. ALL TESTS PASS.

Now let’s try with no —e10s...

Pass! Let’s clean it up and post it for review!

AH GOD DAMN IT. I have to make some slight alterations.

We want to only show about:tabcrashed for selected tabs. What about the other tabs that had crashed properly? Well, great question...

First off, I guess we don’t care about multiple processes at this point, so I can avoid that case. Just assume a single content process.

(But what if we’re viewing a non-remote browser at the time of the crash?… I’ve needinfo’d phlsa).

Let’s ignore the above problem for a second. Let’s pretend we’re looking at a remote browser - the common case. What do we do?

Well, for the selected browser, we just do what we’ve always done.

If the browser that crashed is not selected, we should flip its remoteness to false, and then restore it so that it’s in the TAB_STATE_NEEDS_RESTORE state.

So who’s responsible for that? tabbrowser.xml or SessionStore?

What should SessionStore be doing? Hum… I guess it could take care of the unselected tab states.

And tabbrowser.xml takes care of the crashed tab state.

Well, fuck it, let’s try it.

The work that I did previously in this bug fixed it in a way that was not exactly up to specification. Should I completely blow away that work, or should I build on it?

I shouldn’t try to build on it just so that I don’t throw away work. I should make sure it’s the best solution.

One way of doing that is to pretend that I hadn’t written those patches, and think about how I would approach fixing the actual problem and see if the solution I had originally written is part of that.

So let’s do that. Let’s pretend I haven’t written a thing.

So now the background tabs are all remote again.

The simplest solution is, in tabbrowser.xml, when we see oop-browser-crashed event, for the tabs that are not selected, flip remoteness to false, and then immediately revive them. Otherwise, show about:tabcrashed.

Let’s see if that works.

Yay, it works!

But I think there are edge-cases with the crashed tab count and WeakSet.

_crashedBrowsers is to make sure we know who can revive.

_crashedTabCount is just so we know what kind of buttons to show.

Move responsibility of tracking how many browsers are in about:tabcrashed into TabCrashReporter.

“SessionStore:crashedTabRevived” was originally sent up from content-sessionStore because it was expected that every tab would be crashed, and we needed to know when each is revived (in case they browsed away)… I think we can just do that with an unload handler in aboutTabCrashed.js

2005 INFO TEST-UNEXPECTED-FAIL | browser/components/sessionstore/test/browser_async_flushes.js | Test timed out -

2006 INFO TEST-UNEXPECTED-FAIL | browser/components/sessionstore/test/browser_async_flushes.js | Found a tab after previous test timed out: data:text/html;charset=utf-8,<a%20href=%23>clickme</a> -

2007 INFO TEST-UNEXPECTED-FAIL | browser/components/sessionstore/test/browser_crashedTabs.js | Second tab should be crashed too. - Got , expected true

Stack trace:

chrome://mochikit/content/browser-test.js:test_is:940

chrome://mochitests/content/browser/browser/components/sessionstore/test/browser_crashedTabs.js:test_revive_tab_from_session_store:239

Handler.prototype.process@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:934:23

this.PromiseWalker.walkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:813:7

Promise*this.PromiseWalker.scheduleWalkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:744:11

this.PromiseWalker.schedulePromise@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:776:7

this.PromiseWalker.completePromise@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:711:7

resolve@resource:///modules/sessionstore/TabStateFlusher.jsm:96:5

resolve@resource:///modules/sessionstore/TabStateFlusher.jsm:34:5

receiveMessage@resource:///modules/sessionstore/SessionStore.jsm:686:11

2008 INFO TEST-UNEXPECTED-FAIL | browser/components/sessionstore/test/browser_crashedTabs.js | Second tab should still be crashed though. - Got , expected true

Stack trace:

chrome://mochikit/content/browser-test.js:test_is:940

chrome://mochitests/content/browser/browser/components/sessionstore/test/browser_crashedTabs.js:test_revive_tab_from_session_store:245