More restore cleanups

RESOLVED FIXED in Firefox 28



Session Restore
4 years ago
4 years ago


(Reporter: billm, Assigned: billm)


Firefox 28
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)


(Whiteboard: [qa-])


(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]

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]

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]

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]

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

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

Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 28
Depends on: 942622


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