REGRESSION(r109480): Form state for iframe content is not restored
authortkent@chromium.org <tkent@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 28 Aug 2012 04:20:12 +0000 (04:20 +0000)
committertkent@chromium.org <tkent@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 28 Aug 2012 04:20:12 +0000 (04:20 +0000)
https://bugs.webkit.org/show_bug.cgi?id=90870

Reviewed by Jochen Eisinger.

Source/WebCore:

Since r109480, we have restored form state only for documents
loaded by FrameLoader::loadItem(). However we should restore form
state for documents in sub-frames of documents loaded by
FrameLoader::loadItem().

Test: fast/loader/form-state-restore-with-frames.html

* history/HistoryItem.cpp:
(WebCore::HistoryItem::isAncestorOf):
Added. A function to search descendants for the specified
HistoryItem. This is used by isAssociatedToRequestedHistoryItem().
* history/HistoryItem.h:
(HistoryItem): Declare isAncestorOf().
* loader/HistoryController.cpp:
(WebCore::HistoryController::saveDocumentState):
Don't save form state for detached document.
This is needed because saveDocumentState() is called twice; before
document detach and after document detach. We need to avoid the
latter call because formElementsState() for a detached document
produces an empty state.
(WebCore::isAssociatedToRequestedHistoryItem):
Added. This function checks the current HistoryItem is associated
to the HistoryItem specified to FrameLoader::loadItem().
(WebCore::HistoryController::restoreDocumentState):
Uses isAssociatedToRequestedHistoryItem().

LayoutTests:

* fast/loader/form-state-restore-with-frames.html: Added.
* fast/loader/form-state-restore-with-frames-expected.txt: Added.
* fast/loader/resources/form-state-restore-with-frames-1.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/fast/loader/form-state-restore-with-frames-expected.txt [new file with mode: 0644]
LayoutTests/fast/loader/form-state-restore-with-frames.html [new file with mode: 0644]
LayoutTests/fast/loader/resources/form-state-restore-with-frames-1.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/history/HistoryItem.cpp
Source/WebCore/history/HistoryItem.h
Source/WebCore/loader/HistoryController.cpp

index e29768c..b5d9ce4 100644 (file)
@@ -1,3 +1,14 @@
+2012-08-27  Kent Tamura  <tkent@chromium.org>
+
+        REGRESSION(r109480): Form state for iframe content is not restored
+        https://bugs.webkit.org/show_bug.cgi?id=90870
+
+        Reviewed by Jochen Eisinger.
+
+        * fast/loader/form-state-restore-with-frames.html: Added.
+        * fast/loader/form-state-restore-with-frames-expected.txt: Added.
+        * fast/loader/resources/form-state-restore-with-frames-1.html: Added.
+
 2012-08-27  Julien Chaffraix  <jchaffraix@webkit.org>
 
         Crash in RenderTable::removeCaption
diff --git a/LayoutTests/fast/loader/form-state-restore-with-frames-expected.txt b/LayoutTests/fast/loader/form-state-restore-with-frames-expected.txt
new file mode 100644 (file)
index 0000000..020a370
--- /dev/null
@@ -0,0 +1,7 @@
+PASS frame$($("frame1"), "input2").value is "value2"
+PASS frame$(frame$($("frame1"), "frame2"), "input3").value is "value3"
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/fast/loader/form-state-restore-with-frames.html b/LayoutTests/fast/loader/form-state-restore-with-frames.html
new file mode 100644 (file)
index 0000000..8d6703c
--- /dev/null
@@ -0,0 +1,41 @@
+<!DOCTYPE html>
+<head>
+<script src="../js/resources/js-test-pre.js"></script>
+<meta http-equiv="pragma" content="no-cache">
+<meta http-equiv="cache-control" content="no-cache">
+</head>
+<body onload="startTest()">
+<input name="name1" id="input1">
+<iframe id="frame1" src="resources/form-state-restore-with-frames-1.html">
+</iframe>
+<form id="form1" action="data:text/html,&lt;script>history.back();&lt;/script>">
+<input type="submit">
+</form>
+
+<script>
+function $(id) {
+    return document.getElementById(id);
+}
+function frame$(frame, id) {
+    return frame.contentDocument.getElementById(id);
+}
+
+function startTest() {
+    if ($('input1').value == 'visited') {
+        shouldBeEqualToString('frame$($("frame1"), "input2").value', 'value2');
+        shouldBeEqualToString('frame$(frame$($("frame1"), "frame2"), "input3").value', 'value3');
+        finishJSTest();
+    } else {
+        setTimeout(function() {
+            $('input1').value = 'visited';
+            frame$($('frame1'), 'input2').value = 'value2';
+            frame$(frame$($('frame1'), 'frame2'), 'input3').value = 'value3';
+            $('form1').submit();
+        }, 0);
+    }
+}
+
+jsTestIsAsync = true;
+</script>
+<script src="../js/resources/js-test-post.js"></script>
+</body>
diff --git a/LayoutTests/fast/loader/resources/form-state-restore-with-frames-1.html b/LayoutTests/fast/loader/resources/form-state-restore-with-frames-1.html
new file mode 100644 (file)
index 0000000..6d132c4
--- /dev/null
@@ -0,0 +1,4 @@
+<!DOCTYPE html>
+<input name="name2" id="input2">
+<iframe id="frame2" srcdoc="&lt;input name=name3 id=input3>">
+</iframe>
index 0bd5298..9db638b 100644 (file)
@@ -1,3 +1,36 @@
+2012-08-27  Kent Tamura  <tkent@chromium.org>
+
+        REGRESSION(r109480): Form state for iframe content is not restored
+        https://bugs.webkit.org/show_bug.cgi?id=90870
+
+        Reviewed by Jochen Eisinger.
+
+        Since r109480, we have restored form state only for documents
+        loaded by FrameLoader::loadItem(). However we should restore form
+        state for documents in sub-frames of documents loaded by
+        FrameLoader::loadItem().
+
+        Test: fast/loader/form-state-restore-with-frames.html
+
+        * history/HistoryItem.cpp:
+        (WebCore::HistoryItem::isAncestorOf):
+        Added. A function to search descendants for the specified
+        HistoryItem. This is used by isAssociatedToRequestedHistoryItem().
+        * history/HistoryItem.h:
+        (HistoryItem): Declare isAncestorOf().
+        * loader/HistoryController.cpp:
+        (WebCore::HistoryController::saveDocumentState):
+        Don't save form state for detached document.
+        This is needed because saveDocumentState() is called twice; before
+        document detach and after document detach. We need to avoid the
+        latter call because formElementsState() for a detached document
+        produces an empty state.
+        (WebCore::isAssociatedToRequestedHistoryItem):
+        Added. This function checks the current HistoryItem is associated
+        to the HistoryItem specified to FrameLoader::loadItem().
+        (WebCore::HistoryController::restoreDocumentState):
+        Uses isAssociatedToRequestedHistoryItem().
+
 2012-08-27  Ian Vollick  <vollick@chromium.org>
 
         [chromium] Should accelerate rotations of >= 180 degrees
index fe11eab..a8f374e 100644 (file)
@@ -532,6 +532,18 @@ void HistoryItem::clearChildren()
     m_children.clear();
 }
 
+bool HistoryItem::isAncestorOf(const HistoryItem* item) const
+{
+    for (size_t i = 0; i < m_children.size(); ++i) {
+        HistoryItem* child = m_children[i].get();
+        if (child == item)
+            return true;
+        if (child->isAncestorOf(item))
+            return true;
+    }
+    return false;
+}
+
 // We do same-document navigation if going to a different item and if either of the following is true:
 // - The other item corresponds to the same document (for history entries created via pushState or fragment changes).
 // - The other item corresponds to the same set of documents, including frames (for history entries created via regular navigation)
index ffe64fb..97d729b 100644 (file)
@@ -171,6 +171,7 @@ public:
     const HistoryItemVector& children() const;
     bool hasChildren() const;
     void clearChildren();
+    bool isAncestorOf(const HistoryItem*) const;
     
     bool shouldDoSameDocumentNavigationTo(HistoryItem* otherItem) const;
     bool hasSameFrames(HistoryItem* otherItem) const;
index 5e3ce84..eb58341 100644 (file)
@@ -163,7 +163,7 @@ void HistoryController::saveDocumentState()
     Document* document = m_frame->document();
     ASSERT(document);
     
-    if (item->isCurrentDocument(document)) {
+    if (item->isCurrentDocument(document) && document->attached()) {
         LOG(Loading, "WebCoreLoading %s: saving form state to %p", m_frame->tree()->uniqueName().string().utf8().data(), item);
         item->setDocumentState(document->formElementsState());
     }
@@ -179,6 +179,22 @@ void HistoryController::saveDocumentAndScrollState()
     }
 }
 
+static inline bool isAssociatedToRequestedHistoryItem(const HistoryItem* current, Frame* frame, const HistoryItem* requested)
+{
+    if (requested == current)
+        return true;
+    if (requested)
+        return false;
+    while ((frame = frame->tree()->parent())) {
+        requested = frame->loader()->requestedHistoryItem();
+        if (!requested)
+            continue;
+        if (requested->isAncestorOf(current))
+            return true;
+    }
+    return false;
+}
+
 void HistoryController::restoreDocumentState()
 {
     Document* doc = m_frame->document();
@@ -201,7 +217,7 @@ void HistoryController::restoreDocumentState()
     
     if (!itemToRestore)
         return;
-    if (m_frame->loader()->requestedHistoryItem() == m_currentItem.get() && !m_frame->loader()->documentLoader()->isClientRedirect()) {
+    if (isAssociatedToRequestedHistoryItem(itemToRestore, m_frame, m_frame->loader()->requestedHistoryItem()) && !m_frame->loader()->documentLoader()->isClientRedirect()) {
         LOG(Loading, "WebCoreLoading %s: restoring form state from %p", m_frame->tree()->uniqueName().string().utf8().data(), itemToRestore);
         doc->setStateForNewFormElements(itemToRestore->documentState());
     }