Ian Gilman [:iangilman] Reporter
	Description • 14 years ago

This should cover setWindowState, as well as other restore triggers. This is necessary for Panorama to properly handle session restore. We need this sooner than later, as it blocks a number of Panorama bugs.

	Paul O'Shannessy [:zpao] (not bugmail, email directly) Assignee
	Comment 1 • 14 years ago

So we really want to make it clear when we're adding/replacing state.

This can be done via restoreLastSession, setBrowserState, setWindowState, setTabState, duplicateTab, undoCloseTab, undoCloseWindow.

Of those, restoreLastSession & setBrowserState will send sessionstore-browser-state-restored when finished.

setWindowState will fire SSTabRestoring on each tab it's restoring. setTabState, undoCloseTab, duplicateTab should also be firing a single SSTabRestoring.

undoCloseWindow will open a new window so doesn't actually matter to Panorama right now.

So what I'm thinking right now is that we fire an event on the window when we start modifying state then fire another event indicating that we're ready for that state to be inspected/modified. Something like SSWindowStateBusy and SSWindowStateReady?

That might be a bit too much of a wide angle approach, but I think it achieves what we want and is enough for Panorama to know not to just start doing things on TabOpen.

Ian, do you think something like that would be enough or would a more fine-grained system of events be better?

	Ian Gilman [:iangilman] Reporter
	Comment 2 • 14 years ago

Sounds great to me!

	Paul O'Shannessy [:zpao] (not bugmail, email directly) Assignee
	Comment 3 • 14 years ago

I haven't tested this at all (I think I can dispatch the event from the window?), but this is what I would do.

SSWindowStateBusy is fired before any tabs are opened/closed/modified in the window. SSWindowStateReady is fired at the end of restoreHistoryPrecursor, at which point all tabs are opened & get/setTabValue are ready for use (through the early access path). That should be late enough, even though technically we're still "busy". The alternative is to fire after restoreHistory (that might actually be better...)

Tests will come soon.

	Paul O'Shannessy [:zpao] (not bugmail, email directly) Assignee
	Comment 4 • 14 years ago

I traced through and (before testing) I think this is covered just via code paths - the following call _sendWindowStateBusy (with many calls coming via restoreWindow)

setTabState
duplicateTab
undoCloseTab
restoreWindow
  via restoreWindow
    via onLoad
      via _openWindowWithState
        via restoreLastSession
        via setBrowserState
        via undoCloseWindow
  via setBrowserState
  via setWindowState
  via restoreLastSession

	Ian Gilman [:iangilman] Reporter
	Comment 5 • 14 years ago

Nominating for blocking, as it blocks two blockers.

	Dietrich Ayala (:dietrich)
	Comment 6 • 14 years ago

Comment on attachment 494235 [details] [diff] [review]
Patch v0.1 (WIP)


>+    // At this point we're essentially ready for consumers to read/write data
>+    // via the sessionstore API so we'll send the SSWindowStateReady event.
>+    this._sendWindowStateReady(aWindow);

not actually ready if the consumer is trolling tab history, so as you mentioned in the comments, i'd be more conservative here and wait until restoreHistory has completed.

>+  _sendWindowStateBusy: function sss__sendWindowStateBusy(aWindow) {
>+    let event = aWindow.document.createEvent("Events");
>+    event.initEvent("SSWindowStateBusy", true, false);
>+    aWindow.dispatchEvent(event);
>+  },
>+
>+  _sendWindowStateReady: function sss__sendWindowStateReady(aWindow) {
>+    let event = aWindow.document.createEvent("Events");
>+    event.initEvent("SSWindowStateReady", true, false);
>+    aWindow.dispatchEvent(event);
>+  },

could compact this into a single sendWindowStateEvent(aWindow, aEvent)

	ithinc
	Comment 7 • 14 years ago

What about "SSWindowRestoring" and "SSWindowRestored" for the name?

	Paul O'Shannessy [:zpao] (not bugmail, email directly) Assignee
	Comment 8 • 14 years ago

(In reply to comment #7)
> What about "SSWindowRestoring" and "SSWindowRestored" for the name?

So the impression I get from those names is that it can only happen once. Once a window is "restored" we wouldn't get a "restoring" event from it again.

Since this can happen via several different paths (overwriting, adding state) I figured just a busy vs ready denotation would be best.

	Paul O'Shannessy [:zpao] (not bugmail, email directly) Assignee
	Comment 10 • 14 years ago

Pushed http://hg.mozilla.org/mozilla-central/rev/92104acff2b3

	u279076
	Comment 12 • 14 years ago

Is there a way I can verify this bug?

	Paul O'Shannessy [:zpao] (not bugmail, email directly) Assignee
	Comment 13 • 14 years ago

(In reply to comment #12)
> Is there a way I can verify this bug?

Tests pass &  panorama is using it.