Bug 36152 - [chromium] Add support for history.pushState and history.replaceState
Summary: [chromium] Add support for history.pushState and history.replaceState
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Page Loading (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Darin Fisher (:fishd, Google)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-03-15 21:04 PDT by Darin Fisher (:fishd, Google)
Modified: 2010-03-16 11:05 PDT (History)
4 users (show)

See Also:


Attachments
v1 patch (20.74 KB, patch)
2010-03-15 21:27 PDT, Darin Fisher (:fishd, Google)
abarth: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Darin Fisher (:fishd, Google) 2010-03-15 21:04:39 PDT
[chromium] Add support for history.pushState and history.replaceState

This involves making the changes required to support these methods in the
Chromium port.
Comment 1 Darin Fisher (:fishd, Google) 2010-03-15 21:27:00 PDT
Created attachment 50758 [details]
v1 patch
Comment 2 Darin Fisher (:fishd, Google) 2010-03-15 21:30:22 PDT
With this patch, all stateobject layout tests pass except for:

  fast/loader/stateobjects/pushstate-object-types.html = TEXT
  fast/loader/stateobjects/state-api-on-detached-frame-crash.html = TEXT

The first fails due to differences in the way we serialize Date and RegExp objects.

The second fails due to an exception thrown--not a crash, which the test was written to detect--while trying to access window.history.replaceState on a detached window (from an iframe that has been removed).
Comment 3 Adam Barth 2010-03-16 00:12:09 PDT
Comment on attachment 50758 [details]
v1 patch

Ok.  I'm sad about backForwardNavigationScheme, but it looks like that's already there...

 590     bool isHashChange =
 591         !m_webFrame->frame()->loader()->history()->currentItem()->stateObject();

No need to line break in WebKit-land.
Comment 4 Darin Fisher (:fishd, Google) 2010-03-16 10:13:06 PDT
Thanks Adam.  Yes, backForwardNavigationScheme is the suck.  This change makes it suck a bit less since it will be seen by much less code; however, I really need to kill it somehow.  It should be pretty easy to do :)
Comment 5 Darin Fisher (:fishd, Google) 2010-03-16 10:25:15 PDT
>  590     bool isHashChange =
>  591        
> !m_webFrame->frame()->loader()->history()->currentItem()->stateObject();
> 
> No need to line break in WebKit-land.

Right, but line breaking like this is optional, no?  There are a number of similar line breaks in the same function and in this file.  I prefer to leave the line breaks for readability.
Comment 6 Darin Fisher (:fishd, Google) 2010-03-16 10:27:25 PDT
Landed as http://trac.webkit.org/changeset/56070
Comment 7 Adam Barth 2010-03-16 11:05:13 PDT
> Right, but line breaking like this is optional, no?

Yes, it's optional.