Changeset 53672 in webkit

Timestamp:

Jan 21, 2010 8:22:02 PM (14 years ago)

Author:

beidson@apple.com

Message:

history.back() for same-document history traversals isn't synchronous as the specification states.
<rdar://problem/7535011> and ​https://bugs.webkit.org/show_bug.cgi?id=33538

Reviewed by Maciej Stachowiak.

JavaScriptCore:

• wtf/Platform.h: Add a "HISTORY_ALWAYS_ASYNC" enable and turn it on for Chromium.

WebCore:

In resolving ​https://bugs.webkit.org/show_bug.cgi?id=25570, all history.back()/forward()/go() navigations
were made asynchronous. That doesn't agree with the HTML5 spec and might have been overreaching for fixing
that particular bug. In working with the new history state APIs I noticed some things that should've been
possible were not possible because of this change.

The change in loading behavior is well covered via modifications to previous tests.

• loader/RedirectScheduler.cpp:

(WebCore::RedirectScheduler::scheduleHistoryNavigation): Determine beforehand if the traversal is

a same-document navigation. If it is, perform the load directly instead of scheduling it.

LayoutTests:

The change in loading behavior is well covered via modifications to these previous tests:

• fast/loader/stateobjects/document-destroyed-navigate-back-with-fragment-scroll.html:
• fast/loader/stateobjects/pushstate-with-fragment-urls-and-hashchange-expected.txt:
• fast/loader/stateobjects/pushstate-with-fragment-urls-and-hashchange.html:
• fast/loader/stateobjects/state-api-on-detached-frame-crash-expected.txt:
• fast/loader/stateobjects/state-api-on-detached-frame-crash.html:

Location:

trunk

Files:

• JavaScriptCore/ChangeLog (modified) (1 diff)
• JavaScriptCore/wtf/Platform.h (modified) (1 diff)
• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/loader/stateobjects/document-destroyed-navigate-back-with-fragment-scroll.html (modified) (2 diffs)
• LayoutTests/fast/loader/stateobjects/pushstate-with-fragment-urls-and-hashchange-expected.txt (modified) (1 diff)
• LayoutTests/fast/loader/stateobjects/pushstate-with-fragment-urls-and-hashchange.html (modified) (2 diffs)
• LayoutTests/fast/loader/stateobjects/state-api-on-detached-frame-crash-expected.txt (modified) (1 diff)
• LayoutTests/fast/loader/stateobjects/state-api-on-detached-frame-crash.html (modified) (1 diff)
• WebCore/ChangeLog (modified) (1 diff)
• WebCore/loader/RedirectScheduler.cpp (modified) (3 diffs)

trunk/JavaScriptCore/ChangeLog

@@ +1 @@
+2010-01-21  Brady Eidson  <beidson@apple.com>
+
+        Reviewed by Maciej Stachowiak.
+
+        history.back() for same-document history traversals isn't synchronous as the specification states.
+        <rdar://problem/7535011> and https://bugs.webkit.org/show_bug.cgi?id=33538
+
+        * wtf/Platform.h: Add a "HISTORY_ALWAYS_ASYNC" enable and turn it on for Chromium.
+
 2010-01-21  Geoffrey Garen  <ggaren@apple.com>
 

trunk/JavaScriptCore/wtf/Platform.h

@@ -459 +459 @@
 /* PLATFORM(SKIA) for Win/Linux, CG/CI for Mac */
 #if PLATFORM(CHROMIUM)
+#define ENABLE_HISTORY_ALWAYS_ASYNC 1
 #if OS(DARWIN)
 #define WTF_PLATFORM_CG 1

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2010-01-21  Brady Eidson  <beidson@apple.com>
+
+        Reviewed by Maciej Stachowiak.
+
+        history.back() for same-document history traversals isn't synchronous as the specification states.
+        <rdar://problem/7535011> and https://bugs.webkit.org/show_bug.cgi?id=33538
+
+        The change in loading behavior is well covered via modifications to these previous tests:
+
+        * fast/loader/stateobjects/document-destroyed-navigate-back-with-fragment-scroll.html:
+        * fast/loader/stateobjects/pushstate-with-fragment-urls-and-hashchange-expected.txt:
+        * fast/loader/stateobjects/pushstate-with-fragment-urls-and-hashchange.html:
+        * fast/loader/stateobjects/state-api-on-detached-frame-crash-expected.txt:
+        * fast/loader/stateobjects/state-api-on-detached-frame-crash.html:
+
 2010-01-21  Kent Tamura  <tkent@chromium.org>
 

trunk/LayoutTests/fast/loader/stateobjects/document-destroyed-navigate-back-with-fragment-scroll.html

@@ -25 +25 @@
 {
     alert("Last path component of location is " + lastPathComponent());
-    history.back();
+    setTimeout("history.back();", 0);
 }
 
@@ -49 +49 @@
     if (event.state == "FirstEntry") {
         history.replaceState("FirstEntryWillLaterBeReactivated", null, "#FirstEntryWillLaterBeReactivated");
-        history.forward();
+        setTimeout("history.forward();", 0);
     } else if (event.state == "SecondEntry") {
         history.replaceState("SecondEntryWillLaterBeReactivated", null, "#SecondEntryWillLaterBeReactivated");

trunk/LayoutTests/fast/loader/stateobjects/pushstate-with-fragment-urls-and-hashchange-expected.txt

@@ -3 +3 @@
 State popped with event null (type object) and last path component some-other.html?withsomeotherquery
 State popped with event null (type object) and last path component some-other.html?withsomeotherquery#
-Hash change fired
+Hash change fired and last path component is some-other.html?withsomeotherquery#
 State popped with event null (type object) and last path component some-other.html?withsomeotherquery
-Hash change fired
+Hash change fired and last path component is some-other.html?withsomeotherquery
 State popped with event null (type object) and last path component some-other.html?withsomeotherquery#somehash
-Hash change fired
+Hash change fired and last path component is some-other.html?withsomeotherquery#somehash
 State popped with event null (type object) and last path component some-other.html?withsomeotherquery#someotherhash
-Hash change fired
+Hash change fired and last path component is some-other.html?withsomeotherquery#someotherhash
 State popped with event null (type object) and last path component some-other.html?withquery#someotherhash
 State popped with event null (type object) and last path component some-other.html?withquery#
-Hash change fired
+Hash change fired and last path component is some-other.html?withquery#
 State popped with event null (type object) and last path component some-other.html?withquery#somehash
-Hash change fired
+Hash change fired and last path component is some-other.html?withquery#somehash
 State popped with event null (type object) and last path component some-other.html?withquery
-Hash change fired
+Hash change fired and last path component is some-other.html?withquery
 State popped with event null (type object) and last path component some-other.html?withquery#
-Hash change fired
+Hash change fired and last path component is some-other.html?withquery#
 State popped with event null (type object) and last path component some-other.html?withquery
-Hash change fired
+Hash change fired and last path component is some-other.html?withquery
 State popped with event null (type object) and last path component some-other.html
 State popped with event null (type object) and last path component pushstate-with-fragment-urls-and-hashchange.html#
 State popped with event null (type object) and last path component pushstate-with-fragment-urls-and-hashchange.html#
 State popped with event null (type object) and last path component pushstate-with-fragment-urls-and-hashchange.html#otherhash
-Hash change fired
+Hash change fired and last path component is pushstate-with-fragment-urls-and-hashchange.html#otherhash
 State popped with event null (type object) and last path component pushstate-with-fragment-urls-and-hashchange.html#hash
-Hash change fired
+Hash change fired and last path component is pushstate-with-fragment-urls-and-hashchange.html#hash
 State popped with event null (type object) and last path component pushstate-with-fragment-urls-and-hashchange.html#
-Hash change fired
+Hash change fired and last path component is pushstate-with-fragment-urls-and-hashchange.html#
 State popped with event null (type object) and last path component pushstate-with-fragment-urls-and-hashchange.html#
 State popped with event OriginalEntry (type string) and last path component pushstate-with-fragment-urls-and-hashchange.html

trunk/LayoutTests/fast/loader/stateobjects/pushstate-with-fragment-urls-and-hashchange.html

@@ -49 +49 @@
     log("State popped with event " + event.state + " (type " + typeof event.state + ") and last path component " + lastPathComponent());
     if (event.state != "OriginalEntry")
-        history.back();
+        setTimeout("history.back();", 0);
     else if (window.layoutTestController)
         layoutTestController.notifyDone();
@@ -56 +56 @@
 function hashChanged()
 {
-    log("Hash change fired");
+    log("Hash change fired and last path component is " + lastPathComponent());
 }
 

trunk/LayoutTests/fast/loader/stateobjects/state-api-on-detached-frame-crash-expected.txt

@@ -1 @@
-If this test doesn't crash, it passed.
+

trunk/LayoutTests/fast/loader/stateobjects/state-api-on-detached-frame-crash.html

@@ -9 +9 @@
     var ifr = frames[0];
     document.body.removeChild(document.getElementsByTagName("iframe")[0])
-    try {
-        ifr.history.replaceState("foo", "bar");
-    } catch(e) {
-        alert(e);
-    }
-    try {
-        ifr.history.pushState("fu", "barred");
-    } catch(e) {
-        alert(e);
-    }   
+    ifr.history.replaceState("foo", "bar");
+    ifr.history.pushState("fu", "barred");
 }
 
 </script>
 <body onload="runTest();">
-If this test doesn't crash, it passed.
 <iframe src="about:blank">
 </iframe>

trunk/WebCore/ChangeLog

@@ +1 @@
+2010-01-21  Brady Eidson  <beidson@apple.com>
+
+        Reviewed by Maciej Stachowiak.
+
+        history.back() for same-document history traversals isn't synchronous as the specification states.
+        <rdar://problem/7535011> and https://bugs.webkit.org/show_bug.cgi?id=33538
+
+        In resolving https://bugs.webkit.org/show_bug.cgi?id=25570, all history.back()/forward()/go() navigations
+        were made asynchronous. That doesn't agree with the HTML5 spec and might have been overreaching for fixing
+        that particular bug. In working with the new history state APIs I noticed some things that should've been
+        possible were not possible because of this change.
+
+        The change in loading behavior is well covered via modifications to previous tests.
+
+        * loader/RedirectScheduler.cpp:
+        (WebCore::RedirectScheduler::scheduleHistoryNavigation): Determine beforehand if the traversal is
+          a same-document navigation. If it is, perform the load directly instead of scheduling it.
+
 2010-01-21  Jakub Wieczorek  <faw217@gmail.com>
 

trunk/WebCore/loader/RedirectScheduler.cpp

@@ -33 +33 @@
 #include "RedirectScheduler.h"
 
+#include "BackForwardList.h"
 #include "DocumentLoader.h"
 #include "Event.h"
@@ -39 +40 @@
 #include "FrameLoadRequest.h"
 #include "FrameLoader.h"
+#include "HistoryItem.h"
 #include "HTMLFormElement.h"
 #include "HTMLFrameOwnerElement.h"
@@ -266 +268 @@
     // Invalid history navigations (such as history.forward() during a new load) have the side effect of cancelling any scheduled
     // redirects. We also avoid the possibility of cancelling the current load by avoiding the scheduled redirection altogether.
-    if (!m_frame->page()->canGoBackOrForward(steps)) { 
-        cancel(); 
-        return; 
-    } 
-
+    HistoryItem* specifiedEntry = m_frame->page()->backForwardList()->itemAtIndex(steps);
+    if (!specifiedEntry) {
+        cancel();
+        return;
+    }
+    
+#if !ENABLE(HISTORY_ALWAYS_ASYNC)
+    // If the specified entry and the current entry have the same document, this is either a state object traversal or a fragment 
+    // traversal (or both) and should be performed synchronously.
+    HistoryItem* currentEntry = m_frame->loader()->history()->currentItem();
+    if (currentEntry != specifiedEntry && specifiedEntry->document() && currentEntry->document() == specifiedEntry->document()) {
+        m_frame->loader()->history()->goToItem(specifiedEntry, FrameLoadTypeIndexedBackForward);
+        return;
+    }
+#endif
+    
+    // In all other cases, schedule the history traversal to occur asynchronously.
     schedule(new ScheduledRedirection(steps));
 }