940777 - More restore cleanups

Status: RESOLVED FIXED

(Firefox :: Session Restore, defect)

Description

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

This is a follow-up to bug 930269. It has some of the meatier changes that I was holding off on posting until getting it all green.

Comment 1

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Attachment: fixup-index-arith

This patch ensures that tabData.index is a reasonable value in restoreTabs. That way we can rely on it being correct throughout the rest of the restoration process.

Comment 2

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Attachment: simplify-about-blank

We have a special path to load about:blank if there are no history entries to restore. For some reason this load happens much earlier in the restore process than normal loads do. We also fire the SSTabRestored listener much earlier in this case because didStartLoad ends up as false.

I've moved it so that the about:blank load happens at the normal time. This breaks a test that assumed that the load would still be in progress when SSTabRestored fires, so I just fixed the test.

Comment 3

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Attachment: move-restored-event

I'm trying to transition the tests so that they listen for SSTabRestored rather than load events. (Eventually the load event will fire in the child and it will send a message to the parent, which will then fire SSTabRestored.) Right now, we fire SSTabRestored before browser.__SS_data and some other fields have been cleaned up. That can break some tests which perform other session store API calls from within the SSTabRestored handler.

This patch makes sure that session restoration is completely done before triggering SSTabRestored. Note that the deletion of browser.__SS_data in restoreDocument is safe because restoreDocument is only called from the "load" event, and that handler calls onTabLoad right after, which already deletes that field.

Comment 4

[Tim Taubert [:ttaubert] (inactive)]

Thanks! Always great to clean up some code :)

Comment 5

[Tim Taubert [:ttaubert] (inactive)]

Comment on attachment 8334976 [details] [diff] [review]
simplify-about-blank

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

::: browser/components/sessionstore/test/browser_522545.js
@@ +210,5 @@
> +           "sessionstore got correct userTypedValue");
> +        is(newState.windows[0].tabs[0].userTypedClear, 0,
> +           "sessionstore got correct userTypedClear");
> +        runNextTest();
> +      }

});

Comment 6

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/ade5c649dd27
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/c0b94392c6aa
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/9bc9c5739a0a

Comment 7

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/ade5c649dd27
https://hg.mozilla.org/mozilla-central/rev/c0b94392c6aa
https://hg.mozilla.org/mozilla-central/rev/9bc9c5739a0a