[Chromium] Crash after magic iframe transfer for Pepper/NaCl plugins.
authordimich@chromium.org <dimich@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 19 Sep 2011 21:05:28 +0000 (21:05 +0000)
committerdimich@chromium.org <dimich@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 19 Sep 2011 21:05:28 +0000 (21:05 +0000)
https://bugs.webkit.org/show_bug.cgi?id=68267
Make adoptNode() to not enable live iframe transfer when the iframe's subtree contains plugins.

Reviewed by Adam Barth.

Source/WebCore:

Test: fast/frames/iframe-reparenting-embed-elements.html

* dom/Document.cpp:
(WebCore::Document::adoptNode):
* html/HTMLFrameElementBase.cpp:
(WebCore::hasPluginElements):
(WebCore::HTMLFrameElementBase::canRemainAliveOnRemovalFromTree):
* html/HTMLFrameElementBase.h:

LayoutTests:

* fast/frames/iframe-reparenting-embed-elements-expected.txt: Added.
* fast/frames/iframe-reparenting-embed-elements.html: Added.
* fast/frames/resources/iframe-reparenting-embed-frame1.html: Added.
* fast/frames/resources/iframe-reparenting-embed-iframe.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/fast/frames/iframe-reparenting-embed-elements-expected.txt [new file with mode: 0644]
LayoutTests/fast/frames/iframe-reparenting-embed-elements.html [new file with mode: 0644]
LayoutTests/fast/frames/resources/iframe-reparenting-embed-frame1.html [new file with mode: 0644]
LayoutTests/fast/frames/resources/iframe-reparenting-embed-iframe.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/dom/Document.cpp
Source/WebCore/html/HTMLFrameElementBase.cpp
Source/WebCore/html/HTMLFrameElementBase.h

index 233771e..dbaf66d 100644 (file)
@@ -1,3 +1,16 @@
+2011-09-19  Dmitry Titov  <dimich@chromium.org>
+
+        [Chromium] Crash after magic iframe transfer for Pepper/NaCl plugins.
+        https://bugs.webkit.org/show_bug.cgi?id=68267
+        Make adoptNode() to not enable live iframe transfer when the iframe's subtree contains plugins.
+
+        Reviewed by Adam Barth.
+
+        * fast/frames/iframe-reparenting-embed-elements-expected.txt: Added.
+        * fast/frames/iframe-reparenting-embed-elements.html: Added.
+        * fast/frames/resources/iframe-reparenting-embed-frame1.html: Added.
+        * fast/frames/resources/iframe-reparenting-embed-iframe.html: Added.
+
 2011-09-19  Abhishek Arya  <inferno@chromium.org>
 
         Issues with merging ruby bases.
diff --git a/LayoutTests/fast/frames/iframe-reparenting-embed-elements-expected.txt b/LayoutTests/fast/frames/iframe-reparenting-embed-elements-expected.txt
new file mode 100644 (file)
index 0000000..185bd65
--- /dev/null
@@ -0,0 +1,12 @@
+This test moves an iframe between two documents 3 times: without plugins, with <embed> tag and then with <object> tag.
+
+Only the attempt without plugin elements should succeed. The presence of plugin elements should prevent the document.adoptNode() method from triggering live transfer - in which case the iframe will be reloaded.
+
+Test succeeds if there are 'PASS' messages below and no 'FAIL' messages.
+
+PASS: Test without plugins
+PASS: Test with <embed>
+PASS: Test with <object>
+Test Finished.
+
diff --git a/LayoutTests/fast/frames/iframe-reparenting-embed-elements.html b/LayoutTests/fast/frames/iframe-reparenting-embed-elements.html
new file mode 100644 (file)
index 0000000..3837fb6
--- /dev/null
@@ -0,0 +1,69 @@
+<html>
+<script>
+function log(message) {
+  document.getElementById("log").innerText += message + "\n";
+}
+
+function verifyResult(message, actualToken, expectedToReload) {
+  var success = (expectedToReload != (actualToken == "modified"));
+  log((success ? "PASS" : "FAIL") + ": " + message);
+}
+
+function transferIframe(testStepDescription, expectedToReload, nextTest)
+{
+  var iframe = frame1.contentDocument.getElementsByTagName("iframe")[0];
+  if (iframe.contentWindow.token != "loaded")
+    log("FAIL: invalid initial state of test iframe");
+
+  iframe.contentWindow.token = "modified";
+
+  frame1.contentDocument.adoptNode(iframe);
+  frame2.contentDocument.body.appendChild(iframe);
+  verifyResult(testStepDescription, iframe.contentWindow.token, expectedToReload);
+
+  frame1.onload = nextTest;
+  frame1.contentWindow.location.reload();
+}
+
+function finish()
+{
+  log("Test Finished.");
+  if (window.layoutTestController)
+      layoutTestController.notifyDone();
+}
+
+function test() {
+  if (window.layoutTestController) {
+    layoutTestController.dumpAsText();
+    layoutTestController.waitUntilDone();
+  }
+
+  frame1 = document.getElementById("frame1");
+  frame2 = document.getElementById("frame2");
+
+  transferIframe("Test without plugins", false, test1);
+}
+
+function test1() {
+  var iframe = frame1.contentDocument.getElementsByTagName("iframe")[0];
+  iframe.contentDocument.body.appendChild(iframe.contentDocument.createElement("embed"));
+  transferIframe("Test with <embed>", true, test2);
+}
+
+function test2() {
+  var iframe = frame1.contentDocument.getElementsByTagName("iframe")[0];
+  iframe.contentDocument.body.appendChild(iframe.contentDocument.createElement("object"));
+  transferIframe("Test with <object>", true, finish);
+}
+
+</script>
+<body onload=test()>
+<p>This test moves an iframe between two documents 3 times: without plugins, with &lt;embed&gt; tag and then with &lt;object&gt; tag.</p>
+<p>Only the attempt without plugin elements should succeed. The presence of plugin elements should prevent the document.adoptNode() method from 
+triggering live transfer - in which case the iframe will be reloaded.</p>
+<p>Test succeeds if there are 'PASS' messages below and no 'FAIL' messages.</p>
+<iframe id=frame1 src="resources/iframe-reparenting-embed-frame1.html"></iframe>
+<iframe id=frame2 src="resources/iframe-reparenting-frame2.html"></iframe>
+<pre id=log></pre>
+</body>
+</html>
diff --git a/LayoutTests/fast/frames/resources/iframe-reparenting-embed-frame1.html b/LayoutTests/fast/frames/resources/iframe-reparenting-embed-frame1.html
new file mode 100644 (file)
index 0000000..97e0ab3
--- /dev/null
@@ -0,0 +1,2 @@
+<body>
+<iframe src=iframe-reparenting-embed-iframe.html></iframe>
diff --git a/LayoutTests/fast/frames/resources/iframe-reparenting-embed-iframe.html b/LayoutTests/fast/frames/resources/iframe-reparenting-embed-iframe.html
new file mode 100644 (file)
index 0000000..2a1c9ce
--- /dev/null
@@ -0,0 +1,10 @@
+<script>
+token = "loading";
+
+function init()
+{
+    token = "loaded";
+}
+
+</script>
+<body onload=init()>
index 8101729..87bae2a 100644 (file)
@@ -1,3 +1,20 @@
+2011-09-19  Dmitry Titov  <dimich@chromium.org>
+
+        [Chromium] Crash after magic iframe transfer for Pepper/NaCl plugins.
+        https://bugs.webkit.org/show_bug.cgi?id=68267
+        Make adoptNode() to not enable live iframe transfer when the iframe's subtree contains plugins.
+
+        Reviewed by Adam Barth.
+
+        Test: fast/frames/iframe-reparenting-embed-elements.html
+
+        * dom/Document.cpp:
+        (WebCore::Document::adoptNode):
+        * html/HTMLFrameElementBase.cpp:
+        (WebCore::hasPluginElements):
+        (WebCore::HTMLFrameElementBase::canRemainAliveOnRemovalFromTree):
+        * html/HTMLFrameElementBase.h:
+
 2011-09-19  Abhishek Arya  <inferno@chromium.org>
 
         Issues with merging ruby bases.
index 1943fae..fe4cd6d 100644 (file)
@@ -944,7 +944,7 @@ PassRefPtr<Node> Document::adoptNode(PassRefPtr<Node> source, ExceptionCode& ec)
                 ec = HIERARCHY_REQUEST_ERR;
                 return 0;
             }
-            iframe->setRemainsAliveOnRemovalFromTree(attached() && source->attached());
+            iframe->setRemainsAliveOnRemovalFromTree(attached() && source->attached() && iframe->canRemainAliveOnRemovalFromTree());
         }
 
         if (source->parentNode())
index 234ec02..2526acc 100644 (file)
@@ -44,6 +44,33 @@ namespace WebCore {
 
 using namespace HTMLNames;
 
+// Helper to check if the Frame's document contains elements that can instantiate plugins.
+// Does a recursive check for nested Frames too.
+static bool hasPluginElements(Frame* frame)
+{
+    if (!frame)
+        return false;
+
+    // Search for a plugin element in this document.
+    Document* document = frame->document();
+    for (Node* node = document->firstChild(); node; node = node->traverseNextNode(document)) {
+        if (!node->isElementNode())
+            continue;
+
+        Element* element = static_cast<Element*>(node);
+        if (element->hasLocalName(embedTag) || element->hasLocalName(objectTag))
+            return true;
+    }
+
+    // Do the same for the nested frames.
+    for (Frame* child = frame->tree()->firstChild(); child; child = child->tree()->nextSibling()) {
+        if (hasPluginElements(child))
+            return true;
+    }
+
+    return false;
+}
+
 HTMLFrameElementBase::HTMLFrameElementBase(const QualifiedName& tagName, Document* document)
     : HTMLFrameOwnerElement(tagName, document)
     , m_scrolling(ScrollbarAuto)
@@ -251,8 +278,18 @@ int HTMLFrameElementBase::height()
     return renderBox()->height();
 }
 
+// Some types of content can restrict the ability to move the iframes between pages.
+// For example, the plugin infrastructure of an embedder may associate the plugin instances
+// with the top-level Frame for tracking various resources and failure to transfer those
+// resources correctly may lead to crashes and other ill effects (https://bugs.webkit.org/show_bug.cgi?id=68267)
+bool HTMLFrameElementBase::canRemainAliveOnRemovalFromTree()
+{
+    return !hasPluginElements(contentFrame());
+}
+
 void HTMLFrameElementBase::setRemainsAliveOnRemovalFromTree(bool value)
 {
+    ASSERT(!value || canRemainAliveOnRemovalFromTree());
     m_remainsAliveOnRemovalFromTree = value;
 
     // There is a possibility that JS will do document.adoptNode() on this element but will not insert it into the tree.
index a0f57cc..0f587c9 100644 (file)
@@ -42,6 +42,7 @@ public:
     int width();
     int height();
 
+    bool canRemainAliveOnRemovalFromTree();
     void setRemainsAliveOnRemovalFromTree(bool);
 #if ENABLE(FULLSCREEN_API)
     virtual bool allowFullScreen() const;