HTMLEmbedObject should match HTMLObjectElement by stopping any load when it is remove...
authoreric@webkit.org <eric@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 26 Jan 2012 00:00:03 +0000 (00:00 +0000)
committereric@webkit.org <eric@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 26 Jan 2012 00:00:03 +0000 (00:00 +0000)
https://bugs.webkit.org/show_bug.cgi?id=74360

Reviewed by Adam Barth.

Source/WebCore:

Neither of these !renderer() checks is strictly necessary since requestObject()
makes the same check.  However requestObject() asserts before it makes that
check, so it makes sense to add the check to HTMLEmebedObject instead of
removing the check from HTMLObjectElement.
I also moved the protect RefPtr to before the beforeload dispatch since
beforeload can remove the whole DOM element (as this test does) and
thus I beleive the renderer() check could be checking free'd memory.

Updated fast/dom/beforeload/remove-flash-in-beforeload-listener.html to test
<embed> as well as <object>.

* html/HTMLEmbedElement.cpp:
(WebCore::HTMLEmbedElement::updateWidget):
* html/HTMLObjectElement.cpp:
(WebCore::HTMLObjectElement::updateWidget):

LayoutTests:

Test <embed> as well as <object> in this test.

* fast/dom/beforeload/remove-flash-in-beforeload-listener.html:

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

LayoutTests/ChangeLog
LayoutTests/fast/dom/beforeload/remove-flash-in-beforeload-listener.html
Source/WebCore/ChangeLog
Source/WebCore/html/HTMLEmbedElement.cpp
Source/WebCore/html/HTMLObjectElement.cpp

index 5c2bc01..2f6debd 100644 (file)
@@ -1,3 +1,14 @@
+2012-01-25  Eric Seidel  <eric@webkit.org>
+
+        HTMLEmbedObject should match HTMLObjectElement by stopping any load when it is removed from beforeload
+        https://bugs.webkit.org/show_bug.cgi?id=74360
+
+        Reviewed by Adam Barth.
+
+        Test <embed> as well as <object> in this test.
+
+        * fast/dom/beforeload/remove-flash-in-beforeload-listener.html:
+
 2012-01-25  Noel Gordon  <noel.gordon@gmail.com>
 
         [Chromium] Layout Test fast/canvas/webgl/invalid-passed-params.html is failing
index 1c8c021..7fd0f56 100644 (file)
@@ -20,6 +20,7 @@
 <body>
     <p>This page tests that you can correctly remove a flash object in a beforeload listener without causing a crash.</p>
     <object type="application/x-shockwave-flash" data="../../../plugins/resources/simple_blank.swf"></object>
+    <embed type="application/x-shockwave-flash" src="../../../plugins/resources/simple_blank.swf"></object>
     <div id="console"></div>
     <script>
         function checkObjectRemoval()
@@ -45,4 +46,3 @@
     </script>
 </body>
 </html>
-
index f54a6c5..9065227 100644 (file)
@@ -1,3 +1,26 @@
+2012-01-25  Eric Seidel  <eric@webkit.org>
+
+        HTMLEmbedObject should match HTMLObjectElement by stopping any load when it is removed from beforeload
+        https://bugs.webkit.org/show_bug.cgi?id=74360
+
+        Reviewed by Adam Barth.
+
+        Neither of these !renderer() checks is strictly necessary since requestObject()
+        makes the same check.  However requestObject() asserts before it makes that
+        check, so it makes sense to add the check to HTMLEmebedObject instead of
+        removing the check from HTMLObjectElement.
+        I also moved the protect RefPtr to before the beforeload dispatch since
+        beforeload can remove the whole DOM element (as this test does) and
+        thus I beleive the renderer() check could be checking free'd memory.
+
+        Updated fast/dom/beforeload/remove-flash-in-beforeload-listener.html to test
+        <embed> as well as <object>.
+
+        * html/HTMLEmbedElement.cpp:
+        (WebCore::HTMLEmbedElement::updateWidget):
+        * html/HTMLObjectElement.cpp:
+        (WebCore::HTMLObjectElement::updateWidget):
+
 2012-01-25  Kent Tamura  <tkent@chromium.org>
 
         Move focus/blur handling code of HTMLInputElement to InputType
index 6b95ab6..08c15e9 100644 (file)
@@ -154,6 +154,7 @@ void HTMLEmbedElement::updateWidget(PluginCreationOption pluginCreationOption)
     Vector<String> paramValues;
     parametersForPlugin(paramNames, paramValues);
 
+    RefPtr<HTMLEmbedElement> protect(this); // Loading the plugin might remove us from the document.
     bool beforeLoadAllowedLoad = guardedDispatchBeforeLoadEvent(m_url);
     if (!beforeLoadAllowedLoad) {
         if (document()->isPluginDocument()) {
@@ -164,8 +165,9 @@ void HTMLEmbedElement::updateWidget(PluginCreationOption pluginCreationOption)
         }
         return;
     }
+    if (!renderer()) // Do not load the plugin if beforeload removed this element or its renderer.
+        return;
 
-    RefPtr<HTMLEmbedElement> protect(this); // Loading the plugin might remove us from the document.
     SubframeLoader* loader = document()->frame()->loader()->subframeLoader();
     // FIXME: beforeLoad could have detached the renderer!  Just like in the <object> case above.
     loader->requestObject(this, m_url, getAttribute(nameAttr), m_serviceType, paramNames, paramValues);
index 3344761..925275f 100644 (file)
@@ -289,18 +289,13 @@ void HTMLObjectElement::updateWidget(PluginCreationOption pluginCreationOption)
     if (pluginCreationOption == CreateOnlyNonNetscapePlugins && wouldLoadAsNetscapePlugin(url, serviceType))
         return;
 
+    RefPtr<HTMLObjectElement> protect(this); // beforeload and plugin loading can make arbitrary DOM mutations.
     bool beforeLoadAllowedLoad = guardedDispatchBeforeLoadEvent(url);
-    // beforeload events can modify the DOM, potentially causing
-    // RenderWidget::destroy() to be called.  Ensure we haven't been
-    // destroyed before continuing.
-    // FIXME: Should this render fallback content?
-    if (!renderer())
+    if (!renderer()) // Do not load the plugin if beforeload removed this element or its renderer.
         return;
 
-    RefPtr<HTMLObjectElement> protect(this); // Loading the plugin might remove us from the document.
     SubframeLoader* loader = document()->frame()->loader()->subframeLoader();
     bool success = beforeLoadAllowedLoad && hasValidClassId() && loader->requestObject(this, url, getAttribute(nameAttr), serviceType, paramNames, paramValues);
-
     if (!success && fallbackContent)
         renderFallbackContent();
 }