[webkit-changes] [127221] branches/chromium/1229

Thu, 30 Aug 2012 18:14:12 -0700

Title: [127221] branches/chromium/1229
Revision
127221
Author
[email protected]
Date
2012-08-30 18:14:53 -0700 (Thu, 30 Aug 2012)

Log Message

Merge 126839 - REGRESSION(r109480): Form state for iframe content is not restored
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.


[email protected]
Review URL: https://chromiumcodereview.appspot.com/10918017

Modified Paths

Added Paths

Diff

Copied: branches/chromium/1229/LayoutTests/fast/loader/form-state-restore-with-frames-expected.txt (from rev 126839, trunk/LayoutTests/fast/loader/form-state-restore-with-frames-expected.txt) (0 => 127221)


--- branches/chromium/1229/LayoutTests/fast/loader/form-state-restore-with-frames-expected.txt	                        (rev 0)
+++ branches/chromium/1229/LayoutTests/fast/loader/form-state-restore-with-frames-expected.txt	2012-08-31 01:14:53 UTC (rev 127221)
@@ -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
+ 
+

Copied: branches/chromium/1229/LayoutTests/fast/loader/form-state-restore-with-frames.html (from rev 126839, trunk/LayoutTests/fast/loader/form-state-restore-with-frames.html) (0 => 127221)


--- branches/chromium/1229/LayoutTests/fast/loader/form-state-restore-with-frames.html	                        (rev 0)
+++ branches/chromium/1229/LayoutTests/fast/loader/form-state-restore-with-frames.html	2012-08-31 01:14:53 UTC (rev 127221)
@@ -0,0 +1,41 @@
+<!DOCTYPE html>
+<head>
+<script src=""
+<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=""
+</iframe>
+<form id="form1" action=""
+<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=""
+</body>

Copied: branches/chromium/1229/LayoutTests/fast/loader/resources/form-state-restore-with-frames-1.html (from rev 126839, trunk/LayoutTests/fast/loader/resources/form-state-restore-with-frames-1.html) (0 => 127221)


--- branches/chromium/1229/LayoutTests/fast/loader/resources/form-state-restore-with-frames-1.html	                        (rev 0)
+++ branches/chromium/1229/LayoutTests/fast/loader/resources/form-state-restore-with-frames-1.html	2012-08-31 01:14:53 UTC (rev 127221)
@@ -0,0 +1,4 @@
+<!DOCTYPE html>
+<input name="name2" id="input2">
+<iframe id="frame2" srcdoc="&lt;input name=name3 id=input3>">
+</iframe>

Modified: branches/chromium/1229/Source/WebCore/history/HistoryItem.cpp (127220 => 127221)


--- branches/chromium/1229/Source/WebCore/history/HistoryItem.cpp	2012-08-31 01:13:50 UTC (rev 127220)
+++ branches/chromium/1229/Source/WebCore/history/HistoryItem.cpp	2012-08-31 01:14:53 UTC (rev 127221)
@@ -532,6 +532,18 @@
     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)

Modified: branches/chromium/1229/Source/WebCore/history/HistoryItem.h (127220 => 127221)


--- branches/chromium/1229/Source/WebCore/history/HistoryItem.h	2012-08-31 01:13:50 UTC (rev 127220)
+++ branches/chromium/1229/Source/WebCore/history/HistoryItem.h	2012-08-31 01:14:53 UTC (rev 127221)
@@ -171,6 +171,7 @@
     const HistoryItemVector& children() const;
     bool hasChildren() const;
     void clearChildren();
+    bool isAncestorOf(const HistoryItem*) const;
     
     bool shouldDoSameDocumentNavigationTo(HistoryItem* otherItem) const;
     bool hasSameFrames(HistoryItem* otherItem) const;

Modified: branches/chromium/1229/Source/WebCore/loader/HistoryController.cpp (127220 => 127221)


--- branches/chromium/1229/Source/WebCore/loader/HistoryController.cpp	2012-08-31 01:13:50 UTC (rev 127220)
+++ branches/chromium/1229/Source/WebCore/loader/HistoryController.cpp	2012-08-31 01:14:53 UTC (rev 127221)
@@ -163,7 +163,7 @@
     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 @@
     }
 }
 
+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 @@
     
     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());
     }

_______________________________________________
webkit-changes mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to