Bug 1059036 - [e10s] Undo Close Tab would not load tab contents.

handyman is working on this one, and feedback?'d me on a patch… but his patch undoes the async stuff we did in bug 1009628.

STR:

This is a slightly more formal set of steps. It is a 100% repro on my Macbook Pro:

* Open an e10s window

* Browse to any site

* Open a new tab (Command-t)

* Close the old tab (press the X)

* In the History menu, select the tab you just closed.

Expected result:

The new tab you opened is closed and the old tab is reopened to the correctly rendered page.

Actual result:

The new tab you opened is closed and the old tab is reopened to a white screen.

Let me see if I can drill in and see what the problem is.

I suspect that there's tricky stuff going on in either browser.js or SessionStore that opens tabs and switches their order, and we're not handling these fast operations correctly with our async stuff.

Let me see if I can get an exact read on what kind of selectedIndex sets get called, since that triggers the async stuff.

Ah hah, so we're just straight up setting the selectedIndex incorrectly…

Setting selectedIndex to 1: set_selectedIndex@chrome://browser/content/tabbrowser.xml:5265:61

set_selectedPanel@chrome://global/content/bindings/tabbox.xml:671:13

set_selectedIndex@chrome://global/content/bindings/tabbox.xml:399:15

set_selectedItem@chrome://global/content/bindings/tabbox.xml:431:34

set_selectedTab@chrome://global/content/bindings/tabbox.xml:110:15

set_selectedTab@chrome://browser/content/tabbrowser.xml:2530:11

SessionStoreInternal.restoreTabs@resource:///modules/sessionstore/SessionStore.jsm:2421:7

ssi_undoCloseTab@resource:///modules/sessionstore/SessionStore.jsm:1626:1

ss_undoCloseTab@resource:///modules/sessionstore/SessionStore.jsm:198:12

undoCloseTab@chrome://browser/content/browser.js:15831:11

oncommand@chrome://browser/content/browser.xul:1:1

Hunh… I wonder if this is related to the nsDeckFrame stuff I did. It looks like SessionStore is doing everything right - it's the stuff in browser.js - specifically this:

function undoCloseTab(aIndex) {

// wallpaper patch to prevent an unnecessary blank tab (bug 343895)

var blankTabToRemove = null;

if (gBrowser.tabs.length == 1 && isTabEmpty(gBrowser.selectedTab))

blankTabToRemove = gBrowser.selectedTab;

var tab = null;

if (SessionStore.getClosedTabCount(window) > (aIndex || 0)) {

TabView.prepareUndoCloseTab(blankTabToRemove);

tab = SessionStore.undoCloseTab(window, aIndex || 0);

TabView.afterUndoCloseTab();

if (blankTabToRemove)

gBrowser.removeTab(blankTabToRemove);

}

return tab;

}

that's causing things to go wrong.

Theory 1

blankTabToRemove is set to the newtab tab. We restore the old tab at index 1. Asyncification occurs - we have not yet finalized the tab switch to index 1.

Immediately, we remove the previous tab. The nsDeckFrame code fires a runnable that causes the index to decrement by 1 to 0.

Async event finishes up with MozAfterRemotePaint, and we finalize the tab switch at index 1, which is now pointing at blank stuff.

Let's see if I can prove this now. I'll do some dump-ing at the following points:

1) Dump the selectedIndex that we want to set the tabpanels to

2) Dump the runnable setting the index to 0 (can't actually printf in the runnable, but can printf in the index change listener)

3) Dump the finalization of the tab switch

Here goes.

No, this theory is bunk. The index is being set at 1 right from the start.

What's interesting is that this doesn't effect non-e10s windows, just e10s windows. In non-e10s windows, the nsDeckFrame just does its decrement thing.

Actually, what's really interesting is that it doesn't look like selectedIndex gets touched at all with non-e10s. What the hell?

Question

Why is selectedIndex called from e10s windows, and not non-e10s windows?

Oh wait. This might be wrong. selectedIndex is probably being set, but it's using the non-remote one. Of course. I'd forgotten that non-remote browser windows don't have tabbrowser-tabpanels.

So, in the non-e10s window, we get the selectedIndex being set to 1, and then the deck frame index changes to 0.

Setting selectedIndex to 1: set_selectedIndex@chrome://browser/content/tabbrowser.xml:5265:61

set_selectedPanel@chrome://global/content/bindings/tabbox.xml:672:13

set_selectedIndex@chrome://global/content/bindings/tabbox.xml:399:15

set_selectedItem@chrome://global/content/bindings/tabbox.xml:431:34

set_selectedTab@chrome://global/content/bindings/tabbox.xml:110:15

set_selectedTab@chrome://browser/content/tabbrowser.xml:2530:11

SessionStoreInternal.restoreTabs@resource:///modules/sessionstore/SessionStore.jsm:2421:7

ssi_undoCloseTab@resource:///modules/sessionstore/SessionStore.jsm:1626:1

ss_undoCloseTab@resource:///modules/sessionstore/SessionStore.jsm:198:12

undoCloseTab@chrome://browser/content/browser.js:15831:11

oncommand@chrome://browser/content/browser.xul:1:1

Setting selectedIndex to 1

Finally setting selectedIndex to 1

And the nsDeckFrame never does its thing…

non-e10s:

Setting selectedIndex to 1: set_selectedIndex@chrome://global/content/bindings/tabbox.xml:644:61