More restore cleanups

RESOLVED FIXED in Firefox 28

Status

()

Firefox
Session Restore
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: billm, Assigned: billm)

Tracking

Trunk
Firefox 28
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [qa-])

Attachments

(3 attachments)

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.
Created attachment 8334970 [details] [diff] [review]
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.
Attachment #8334970 - Flags: review?(ttaubert)
Created attachment 8334976 [details] [diff] [review]
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.
Attachment #8334976 - Flags: review?(ttaubert)
Created attachment 8334982 [details] [diff] [review]
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.
Attachment #8334982 - Flags: review?(ttaubert)
Attachment #8334970 - Flags: review?(ttaubert) → review+
Attachment #8334976 - Flags: review?(ttaubert) → review+
Attachment #8334982 - Flags: review?(ttaubert) → review+
Thanks! Always great to clean up some code :)
OS: Linux → All
Hardware: x86_64 → All
Version: unspecified → Trunk
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();
> +      }

});
https://hg.mozilla.org/mozilla-central/rev/ade5c649dd27
https://hg.mozilla.org/mozilla-central/rev/c0b94392c6aa
https://hg.mozilla.org/mozilla-central/rev/9bc9c5739a0a
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 28
Depends on: 942622

Updated

4 years ago
Whiteboard: [qa-]
You need to log in before you can comment on or make changes to this bug.