634435 - Add a property to document to expose current state

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

A big problem with the pushState API which we're adding in FF4 is that during page loading, you don't have access to the current state until the page finishes loading and the "load" event is fired.

Consider the following set of actions:

1. User loads page
2. User interacts with page such that page calls pushState with a state object
3. The session-history entry created in step 2 is reloaded. This can happen in
   a number of ways, for example:
   * User presses reload
   * User navigates far enough away from the page that the page is no longer
     bf-cached. Then navigates back to session-history entry in step 2.
   * User exits (or crashes) browser and then restores previous session once
     the browser is restarted.

In step 3, the page won't have access to the state until the page is fully loaded. This will likely result the page rendering first the step-1 state, and then at the end snapping to the step-2 state.

We should fix this by always exposing the current state through a DOM property somewhere.

Comment 1

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Attachment: Patch to fix

Comment 2

[Justin Lebar (not reading bugmail)]

Comment on attachment 513172 [details] [diff] [review]
Patch to fix

>-  nsString mPendingStateObject;
>+  nsString mCurrentStateObject;
>   [...]
>+  nsCOMPtr<nsIVariant> mCurrentStateObjectCached;

Maybe move mCurrentStateObjectCached under mCurrentStateObject and add a
comment indicating mCurrentStateObjectCached is a deserialized copy of mCurrentStateObject, and that you should modify mCurrentStateObject by calling SetDocCurrentStateObject so that the cached copy doesn't become stale?

>+NS_IMETHODIMP
>+nsDocument::GetMozCurrentStateObject(nsIVariant** aState)
>+{
>+  // Get the document's pending state object -- it contains the data we're
>+  // going to send along with the popstate event.  The object is serialized as
>+  // JSON.

This comment is now out-of-date.  We should indicate that we use this method
both for backing document.mozGetCurrentStateObject and for popStateEvent.state, because that's not obvious.

r=me with those changes.

Comment 3

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

I didn't want to move around the members too much in order to keep binary compat.

I will update the comment though.

Comment 4

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 513172 [details] [diff] [review]
Patch to fix

a=beltzner

Comment 5

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Checked in. Thanks for the quick review!

http://hg.mozilla.org/mozilla-central/rev/06e5c81524e9

Comment 6

[Nickolay_Ponomarev]

dev-doc-needed: document.mozCurrentStateObject

Comment 7

[Justin Lebar (not reading bugmail)]

This will be backed out by bug 635844.  Removing dev-doc-needed.