From 1f8035c3dd11bc7a725364df4743d88fb4c7d84e Mon Sep 17 00:00:00 2001 From: "eric@webkit.org" Date: Thu, 26 Jan 2012 00:00:03 +0000 Subject: [PATCH] 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. 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 as well as . * html/HTMLEmbedElement.cpp: (WebCore::HTMLEmbedElement::updateWidget): * html/HTMLObjectElement.cpp: (WebCore::HTMLObjectElement::updateWidget): LayoutTests: Test as well as 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 | 11 +++++++++++ .../remove-flash-in-beforeload-listener.html | 2 +- Source/WebCore/ChangeLog | 23 ++++++++++++++++++++++ Source/WebCore/html/HTMLEmbedElement.cpp | 4 +++- Source/WebCore/html/HTMLObjectElement.cpp | 9 ++------- 5 files changed, 40 insertions(+), 9 deletions(-) diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog index 5c2bc01..2f6debd 100644 --- a/LayoutTests/ChangeLog +++ b/LayoutTests/ChangeLog @@ -1,3 +1,14 @@ +2012-01-25 Eric Seidel + + 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 as well as in this test. + + * fast/dom/beforeload/remove-flash-in-beforeload-listener.html: + 2012-01-25 Noel Gordon [Chromium] Layout Test fast/canvas/webgl/invalid-passed-params.html is failing diff --git a/LayoutTests/fast/dom/beforeload/remove-flash-in-beforeload-listener.html b/LayoutTests/fast/dom/beforeload/remove-flash-in-beforeload-listener.html index 1c8c021..7fd0f56 100644 --- a/LayoutTests/fast/dom/beforeload/remove-flash-in-beforeload-listener.html +++ b/LayoutTests/fast/dom/beforeload/remove-flash-in-beforeload-listener.html @@ -20,6 +20,7 @@

This page tests that you can correctly remove a flash object in a beforeload listener without causing a crash.

+
- diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog index f54a6c5..9065227 100644 --- a/Source/WebCore/ChangeLog +++ b/Source/WebCore/ChangeLog @@ -1,3 +1,26 @@ +2012-01-25 Eric Seidel + + 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 + as well as . + + * html/HTMLEmbedElement.cpp: + (WebCore::HTMLEmbedElement::updateWidget): + * html/HTMLObjectElement.cpp: + (WebCore::HTMLObjectElement::updateWidget): + 2012-01-25 Kent Tamura Move focus/blur handling code of HTMLInputElement to InputType diff --git a/Source/WebCore/html/HTMLEmbedElement.cpp b/Source/WebCore/html/HTMLEmbedElement.cpp index 6b95ab6..08c15e9 100644 --- a/Source/WebCore/html/HTMLEmbedElement.cpp +++ b/Source/WebCore/html/HTMLEmbedElement.cpp @@ -154,6 +154,7 @@ void HTMLEmbedElement::updateWidget(PluginCreationOption pluginCreationOption) Vector paramValues; parametersForPlugin(paramNames, paramValues); + RefPtr 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 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 case above. loader->requestObject(this, m_url, getAttribute(nameAttr), m_serviceType, paramNames, paramValues); diff --git a/Source/WebCore/html/HTMLObjectElement.cpp b/Source/WebCore/html/HTMLObjectElement.cpp index 3344761..925275f 100644 --- a/Source/WebCore/html/HTMLObjectElement.cpp +++ b/Source/WebCore/html/HTMLObjectElement.cpp @@ -289,18 +289,13 @@ void HTMLObjectElement::updateWidget(PluginCreationOption pluginCreationOption) if (pluginCreationOption == CreateOnlyNonNetscapePlugins && wouldLoadAsNetscapePlugin(url, serviceType)) return; + RefPtr 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 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(); } -- 2.7.4