2010-06-02 Darin Fisher <darin@chromium.org>
authoreric@webkit.org <eric@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 3 Jun 2010 05:38:18 +0000 (05:38 +0000)
committereric@webkit.org <eric@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 3 Jun 2010 05:38:18 +0000 (05:38 +0000)
        Reviewed by Brady Eidson.

        location.href and outgoing referrer not updated properly by
        pushState/replaceState
        https://bugs.webkit.org/show_bug.cgi?id=40027

        * fast/loader/stateobjects/document-destroyed-navigate-back-with-fragment-scroll.html:
        Updated this test to account for location being modified by replaceState.

        * fast/loader/stateobjects/pushstate-updates-location-expected.txt: Added.
        * fast/loader/stateobjects/pushstate-updates-location.html: Added.
        * fast/loader/stateobjects/replacestate-updates-location-expected.txt: Added.
        * fast/loader/stateobjects/replacestate-updates-location.html: Added.
        * http/tests/navigation/pushstate-updates-referrer-expected.txt: Added.
        * http/tests/navigation/pushstate-updates-referrer.html: Added.
        * http/tests/navigation/replacestate-updates-referrer-expected.txt: Added.
        * http/tests/navigation/replacestate-updates-referrer.html: Added.
        * http/tests/navigation/resources/check-referrer.html: Added.
2010-06-02  Darin Fisher  <darin@chromium.org>

        Reviewed by Brady Eidson.

        location.href and outgoing referrer not updated properly by
        pushState/replaceState
        https://bugs.webkit.org/show_bug.cgi?id=40027

        Tests: fast/loader/stateobjects/pushstate-updates-location.html
               fast/loader/stateobjects/replacestate-updates-location.html
               http/tests/navigation/pushstate-updates-referrer.html
               http/tests/navigation/replacestate-updates-referrer.html

        * dom/Document.cpp:
        (WebCore::Document::updateURLForPushOrReplaceState):
        Update the FrameLoader's notion of the current URL as well!

        * loader/FrameLoader.cpp:
        (WebCore::FrameLoader::loadInSameDocument):
        Use the 'url' parameter instead of m_URL since m_URL might have
        changed during the handling of the PopState event.  Eventually,
        this will become irrelevant since the PopState event should be
        dispatched asynchronously, but just in case we patch HashChange
        to be asynchronous before PopState, this change would be needed.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@60608 268f45cc-cd09-0410-ab3c-d52691b4dbfc

14 files changed:
LayoutTests/ChangeLog
LayoutTests/fast/loader/stateobjects/document-destroyed-navigate-back-with-fragment-scroll.html
LayoutTests/fast/loader/stateobjects/pushstate-updates-location-expected.txt [new file with mode: 0644]
LayoutTests/fast/loader/stateobjects/pushstate-updates-location.html [new file with mode: 0644]
LayoutTests/fast/loader/stateobjects/replacestate-updates-location-expected.txt [new file with mode: 0644]
LayoutTests/fast/loader/stateobjects/replacestate-updates-location.html [new file with mode: 0644]
LayoutTests/http/tests/navigation/pushstate-updates-referrer-expected.txt [new file with mode: 0644]
LayoutTests/http/tests/navigation/pushstate-updates-referrer.html [new file with mode: 0644]
LayoutTests/http/tests/navigation/replacestate-updates-referrer-expected.txt [new file with mode: 0644]
LayoutTests/http/tests/navigation/replacestate-updates-referrer.html [new file with mode: 0644]
LayoutTests/http/tests/navigation/resources/check-referrer.html [new file with mode: 0644]
WebCore/ChangeLog
WebCore/dom/Document.cpp
WebCore/loader/FrameLoader.cpp

index 11162f7..4013799 100644 (file)
@@ -1,3 +1,24 @@
+2010-06-02  Darin Fisher  <darin@chromium.org>
+
+        Reviewed by Brady Eidson.
+
+        location.href and outgoing referrer not updated properly by
+        pushState/replaceState
+        https://bugs.webkit.org/show_bug.cgi?id=40027
+
+        * fast/loader/stateobjects/document-destroyed-navigate-back-with-fragment-scroll.html:
+        Updated this test to account for location being modified by replaceState.
+
+        * fast/loader/stateobjects/pushstate-updates-location-expected.txt: Added.
+        * fast/loader/stateobjects/pushstate-updates-location.html: Added.
+        * fast/loader/stateobjects/replacestate-updates-location-expected.txt: Added.
+        * fast/loader/stateobjects/replacestate-updates-location.html: Added.
+        * http/tests/navigation/pushstate-updates-referrer-expected.txt: Added.
+        * http/tests/navigation/pushstate-updates-referrer.html: Added.
+        * http/tests/navigation/replacestate-updates-referrer-expected.txt: Added.
+        * http/tests/navigation/replacestate-updates-referrer.html: Added.
+        * http/tests/navigation/resources/check-referrer.html: Added.
+
 2010-06-02  Kent Tamura  <tkent@chromium.org>
 
         Unreviewed. Test expectation update.
index f29b3d1..775e86c 100644 (file)
@@ -46,20 +46,31 @@ function loaded()
 function statePopped()
 {
     alert("State popped - " + event.state + " (type " + typeof event.state + ")");
-    if (event.state == "FirstEntry") {
-        history.replaceState("FirstEntryWillLaterBeReactivated", null, "#FirstEntryWillLaterBeReactivated");
-        setTimeout("history.forward();", 0);
-    } else if (event.state == "SecondEntry") {
-        history.replaceState("SecondEntryWillLaterBeReactivated", null, "#SecondEntryWillLaterBeReactivated");
-        window.location = "resources/navigate-back.html";
-    } else if (event.state == "SecondEntryWillLaterBeReactivated")
-        history.back();
+
+    // FIXME: Once the popstate and hashchange events fire asynchronously, we
+    // can eliminate this setTimeout hack.  The hashchange event currently runs
+    // synchronously following the popstate event, but the calls to
+    // replaceState cause the location to change immediately.  That confuses
+    // our hashchange handler, which expects to see the "old" value of the
+    // location.
+
+    var state = event.state;
+    setTimeout(function() {
+        if (state == "FirstEntry") {
+            history.replaceState("FirstEntryWillLaterBeReactivated", null, "#FirstEntryWillLaterBeReactivated");
+            history.forward();
+        } else if (state == "SecondEntry") {
+            history.replaceState("SecondEntryWillLaterBeReactivated", null, "#SecondEntryWillLaterBeReactivated");
+            window.location = "resources/navigate-back.html";
+        } else if (state == "SecondEntryWillLaterBeReactivated")
+            history.back();
+    }, 0);
 }
 
 function hashChanged()
 {
     alert("hashChanged - Last path component of location is " + lastPathComponent());
-   if (window.location.hash == "#FirstEntryWillLaterBeReactivated") {
+    if (window.location.hash == "#FirstEntryWillLaterBeReactivated") {
         alert("Test complete");
         sessionStorage.clear();
         if (window.layoutTestController)
diff --git a/LayoutTests/fast/loader/stateobjects/pushstate-updates-location-expected.txt b/LayoutTests/fast/loader/stateobjects/pushstate-updates-location-expected.txt
new file mode 100644 (file)
index 0000000..7ef22e9
--- /dev/null
@@ -0,0 +1 @@
+PASS
diff --git a/LayoutTests/fast/loader/stateobjects/pushstate-updates-location.html b/LayoutTests/fast/loader/stateobjects/pushstate-updates-location.html
new file mode 100644 (file)
index 0000000..ef1a4dd
--- /dev/null
@@ -0,0 +1,14 @@
+<script>
+if (window.layoutTestController)
+  layoutTestController.dumpAsText();
+
+onload = function() {
+  history.pushState(null, null, "?foo");
+
+  if (location.search == "?foo") {
+    document.body.innerText = "PASS";
+  } else {
+    document.body.innerText = "FAIL: location.search is \'" + location.search + "\'";
+  }
+}
+</script>
diff --git a/LayoutTests/fast/loader/stateobjects/replacestate-updates-location-expected.txt b/LayoutTests/fast/loader/stateobjects/replacestate-updates-location-expected.txt
new file mode 100644 (file)
index 0000000..7ef22e9
--- /dev/null
@@ -0,0 +1 @@
+PASS
diff --git a/LayoutTests/fast/loader/stateobjects/replacestate-updates-location.html b/LayoutTests/fast/loader/stateobjects/replacestate-updates-location.html
new file mode 100644 (file)
index 0000000..b708dd6
--- /dev/null
@@ -0,0 +1,14 @@
+<script>
+if (window.layoutTestController)
+  layoutTestController.dumpAsText();
+
+onload = function() {
+  history.replaceState(null, null, "?foo");
+
+  if (location.search == "?foo") {
+    document.body.innerText = "PASS";
+  } else {
+    document.body.innerText = "FAIL: location.search is \'" + location.search + "\'";
+  }
+}
+</script>
diff --git a/LayoutTests/http/tests/navigation/pushstate-updates-referrer-expected.txt b/LayoutTests/http/tests/navigation/pushstate-updates-referrer-expected.txt
new file mode 100644 (file)
index 0000000..7ef22e9
--- /dev/null
@@ -0,0 +1 @@
+PASS
diff --git a/LayoutTests/http/tests/navigation/pushstate-updates-referrer.html b/LayoutTests/http/tests/navigation/pushstate-updates-referrer.html
new file mode 100644 (file)
index 0000000..f826e5a
--- /dev/null
@@ -0,0 +1,13 @@
+<script>
+if (window.layoutTestController) {
+  layoutTestController.dumpAsText();
+  layoutTestController.waitUntilDone();
+}
+
+onload = function() {
+  history.pushState(null, null, "/foo");
+
+  sessionStorage.expectedReferrer = location.href;
+  location = "/navigation/resources/check-referrer.html";
+}
+</script>
diff --git a/LayoutTests/http/tests/navigation/replacestate-updates-referrer-expected.txt b/LayoutTests/http/tests/navigation/replacestate-updates-referrer-expected.txt
new file mode 100644 (file)
index 0000000..7ef22e9
--- /dev/null
@@ -0,0 +1 @@
+PASS
diff --git a/LayoutTests/http/tests/navigation/replacestate-updates-referrer.html b/LayoutTests/http/tests/navigation/replacestate-updates-referrer.html
new file mode 100644 (file)
index 0000000..3f61afc
--- /dev/null
@@ -0,0 +1,13 @@
+<script>
+if (window.layoutTestController) {
+  layoutTestController.dumpAsText();
+  layoutTestController.waitUntilDone();
+}
+
+onload = function() {
+  history.replaceState(null, null, "/foo");
+
+  sessionStorage.expectedReferrer = location.href;
+  location = "/navigation/resources/check-referrer.html";
+}
+</script>
diff --git a/LayoutTests/http/tests/navigation/resources/check-referrer.html b/LayoutTests/http/tests/navigation/resources/check-referrer.html
new file mode 100644 (file)
index 0000000..bfb8bbb
--- /dev/null
@@ -0,0 +1,11 @@
+<script>
+onload = function() {
+  if (document.referrer == sessionStorage.expectedReferrer) {
+    document.body.innerText = "PASS";
+  } else {
+    document.body.innerText = "FAIL: document.referrer is \'" + document.referrer + "\'";
+  }
+  if (window.layoutTestController)
+    layoutTestController.notifyDone();
+}
+</script>
index b920ef7..95488bd 100644 (file)
@@ -1,3 +1,28 @@
+2010-06-02  Darin Fisher  <darin@chromium.org>
+
+        Reviewed by Brady Eidson.
+
+        location.href and outgoing referrer not updated properly by
+        pushState/replaceState
+        https://bugs.webkit.org/show_bug.cgi?id=40027
+
+        Tests: fast/loader/stateobjects/pushstate-updates-location.html
+               fast/loader/stateobjects/replacestate-updates-location.html
+               http/tests/navigation/pushstate-updates-referrer.html
+               http/tests/navigation/replacestate-updates-referrer.html
+
+        * dom/Document.cpp:
+        (WebCore::Document::updateURLForPushOrReplaceState):
+        Update the FrameLoader's notion of the current URL as well!
+
+        * loader/FrameLoader.cpp:
+        (WebCore::FrameLoader::loadInSameDocument):
+        Use the 'url' parameter instead of m_URL since m_URL might have
+        changed during the handling of the PopState event.  Eventually,
+        this will become irrelevant since the PopState event should be
+        dispatched asynchronously, but just in case we patch HashChange
+        to be asynchronous before PopState, this change would be needed.
+
 2010-06-02  Eric Seidel  <eric@webkit.org>
 
         Reviewed by Adam Barth.
index 5e4c5c6..4eddba4 100644 (file)
@@ -4656,7 +4656,9 @@ void Document::updateURLForPushOrReplaceState(const KURL& url)
     if (!f)
         return;
 
+    // FIXME: Eliminate this redundancy.
     setURL(url);
+    f->loader()->setURL(url);
     f->loader()->documentLoader()->replaceRequestURLForSameDocumentNavigation(url);
 }
 
index 43ddf3b..883abca 100644 (file)
@@ -1693,7 +1693,7 @@ void FrameLoader::loadInSameDocument(const KURL& url, SerializedScriptValue* sta
     }
     
     if (hashChange) {
-        m_frame->document()->enqueueHashchangeEvent(oldURL, m_URL);
+        m_frame->document()->enqueueHashchangeEvent(oldURL, url);
         m_client->dispatchDidChangeLocationWithinPage();
     }