[SCM] WebKit Debian packaging branch, webkit-1.2, updated. upstream/1.1.90-6072-g9a69373
mitz at apple.com
mitz at apple.com
Wed Apr 7 23:20:16 UTC 2010
The following commit has been merged in the webkit-1.2 branch:
commit 9915160d43c817a54363dc322c10ffbd70e77bbf
Author: mitz at apple.com <mitz at apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Date: Tue Nov 3 19:07:48 2009 +0000
WebCore: https://bugs.webkit.org/show_bug.cgi?id=31070
Fix <rdar://problem/7194735> Crashes at RenderText::RenderText()
Fix <rdar://problem/6937089> Crashes at RenderWidget::destroy()
Reviewed by Anders Carlsson and Beth Dakin.
Tests: plugins/attach-during-destroy.html
plugins/destroy-reentry.html
These crashes were caused by plug-in code running during detach(),
causing re-entry into RenderWidget::destroy() in one case and a call
into attach() in the other. The fix is to prevent plug-in code from
being called at certain unsafe times (during attach(), detach(), and
recalcStyle()) by deferring changes to the widget hierarchy.
* dom/Document.cpp:
(WebCore::Document::recalcStyle): Suspend widget hierarchy updates
during style recalculation.
* dom/Element.cpp:
(WebCore::Element::attach): Suspend widget hierarchy updates during
attach().
(WebCore::Element::detach): Suspend widget hierarchy updates during
detach().
* rendering/RenderWidget.cpp:
(WebCore::widgetNewParentMap): Returns a static map of pending changes
to the widget hierarchy.
(WebCore::RenderWidget::suspendWidgetHierarchyUpdates): Increments the
suspend count.
(WebCore::RenderWidget::resumeWidgetHierarchyUpdates): Decrements the
suspend count. If the count is going to be zero, updates the widget
hierarchy by executing the pending changes stored in the map.
(WebCore::moveWidgetToParentSoon): Updates the widget hierarchy
immediately or makes or updates an entry in the map, depending on
whether updates are suspended.
(WebCore::RenderWidget::destroy): Removed earlier bandaid fix for
<rdar://problem/6937089>.
(WebCore::RenderWidget::setWidgetGeometry): Assert that widget updates
are not suspended, because this function updates the widget’s
bounds, which can result in arbitrary native and JavaScript code
execution. I think this assertion is true thanks to some deferred-
update mechanisms that have already been deployed in other places
in the code.
(WebCore::RenderWidget::setWidget): Call moveWidgetToParentSoon instead
of changing the widget hierarchy directly.
* rendering/RenderWidget.h: Declared suspendWidgetHierarchyUpdates()
and resumeWidgetHierarchyUpdates().
WebKitTools: https://bugs.webkit.org/show_bug.cgi?id=31070
Reviewed by Anders Carlsson and Beth Dakin.
Added an 'ondestroy' parameter to the test plug-in. When the plug-in is
destroyed, it executes the value of the 'ondestroy' parameter as a
script.
* DumpRenderTree/TestNetscapePlugIn.subproj/PluginObject.cpp:
(pluginAllocate): Initialize onDestroy.
* DumpRenderTree/TestNetscapePlugIn.subproj/PluginObject.h: Define
onDestroy.
* DumpRenderTree/TestNetscapePlugIn.subproj/main.cpp:
(NPP_New): Set onDestroy to the value of the 'ondestroy' parameter, if
specified.
(NPP_Destroy): Execute the value of 'ondestroy' as a script.
LayoutTests: https://bugs.webkit.org/show_bug.cgi?id=31070
Test for <rdar://problem/7194735> Crashes at RenderText::RenderText()
Test for <rdar://problem/6937089> Crashes at RenderWidget::destroy()
Reviewed by Anders Carlsson and Beth Dakin.
* plugins/attach-during-destroy-expected.txt: Added.
* plugins/attach-during-destroy.html: Added.
* plugins/destroy-reentry-expected.txt: Added.
* plugins/destroy-reentry.html: Added.
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@50470 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index 7d183c5..22b0100 100644
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,16 @@
+2009-11-03 Dan Bernstein <mitz at apple.com>
+
+ Reviewed by Anders Carlsson and Beth Dakin.
+
+ https://bugs.webkit.org/show_bug.cgi?id=31070
+ Test for <rdar://problem/7194735> Crashes at RenderText::RenderText()
+ Test for <rdar://problem/6937089> Crashes at RenderWidget::destroy()
+
+ * plugins/attach-during-destroy-expected.txt: Added.
+ * plugins/attach-during-destroy.html: Added.
+ * plugins/destroy-reentry-expected.txt: Added.
+ * plugins/destroy-reentry.html: Added.
+
2009-11-03 Evan Martin <evan at chromium.org>
Reviewed by Darin Adler.
diff --git a/LayoutTests/plugins/attach-during-destroy-expected.txt b/LayoutTests/plugins/attach-during-destroy-expected.txt
new file mode 100644
index 0000000..7fe8fdb
--- /dev/null
+++ b/LayoutTests/plugins/attach-during-destroy-expected.txt
@@ -0,0 +1,6 @@
+Test for rdar://problem/7194735 Crashes at RenderText::RenderText().
+
+This test should not crash or cause an assertion failure.
+
+
+
diff --git a/LayoutTests/plugins/attach-during-destroy.html b/LayoutTests/plugins/attach-during-destroy.html
new file mode 100644
index 0000000..9c42685
--- /dev/null
+++ b/LayoutTests/plugins/attach-during-destroy.html
@@ -0,0 +1,21 @@
+<p>
+ Test for <i><a href="rdar://problem/7194735">rdar://problem/7194735</a> Crashes at RenderText::RenderText()</i>.
+</p>
+<p>
+ This test should not crash or cause an assertion failure.
+</p>
+<embed type="application/x-webkit-test-netscape" ondestroy="destroyed()">
+<div id="target"></div>
+<script>
+ function destroyed()
+ {
+ var target = document.getElementById("target");
+ target.innerHTML = "text";
+ }
+
+ if (window.layoutTestController)
+ layoutTestController.dumpAsText();
+
+ document.body.offsetTop;
+ location.href = "data:text/html,Test for rdar://problem/7194735 PASSED.";
+</script>
diff --git a/LayoutTests/plugins/destroy-reentry-expected.txt b/LayoutTests/plugins/destroy-reentry-expected.txt
new file mode 100644
index 0000000..f0d1c45
--- /dev/null
+++ b/LayoutTests/plugins/destroy-reentry-expected.txt
@@ -0,0 +1,5 @@
+Test for rdar://problem/6937089 Crashes at RenderWidget::destroy().
+
+This test should not crash or cause an assertion failure.
+
+
diff --git a/LayoutTests/plugins/destroy-reentry.html b/LayoutTests/plugins/destroy-reentry.html
new file mode 100644
index 0000000..b4c10a9
--- /dev/null
+++ b/LayoutTests/plugins/destroy-reentry.html
@@ -0,0 +1,22 @@
+<p>
+ Test for <i><a href="rdar://problem/6937089">rdar://problem/6937089</a> Crashes at RenderWidget::destroy()</i>.
+</p>
+<p>
+ This test should not crash or cause an assertion failure.
+</p>
+<div id="target">
+ <embed type="application/x-webkit-test-netscape" ondestroy="destroyed()">
+</div>
+<script>
+ function destroyed()
+ {
+ var target = document.getElementById("target");
+ target.parentNode.removeChild(target);
+ }
+
+ if (window.layoutTestController)
+ layoutTestController.dumpAsText();
+
+ document.body.offsetTop;
+ location.href = "data:text/html,Test for rdar://problem/6937089 PASSED.";
+</script>
diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
index e7c2bd6..fef23a7 100644
--- a/WebCore/ChangeLog
+++ b/WebCore/ChangeLog
@@ -1,3 +1,54 @@
+2009-11-03 Dan Bernstein <mitz at apple.com>
+
+ Reviewed by Anders Carlsson and Beth Dakin.
+
+ https://bugs.webkit.org/show_bug.cgi?id=31070
+ Fix <rdar://problem/7194735> Crashes at RenderText::RenderText()
+ Fix <rdar://problem/6937089> Crashes at RenderWidget::destroy()
+
+ Tests: plugins/attach-during-destroy.html
+ plugins/destroy-reentry.html
+
+ These crashes were caused by plug-in code running during detach(),
+ causing re-entry into RenderWidget::destroy() in one case and a call
+ into attach() in the other. The fix is to prevent plug-in code from
+ being called at certain unsafe times (during attach(), detach(), and
+ recalcStyle()) by deferring changes to the widget hierarchy.
+
+ * dom/Document.cpp:
+ (WebCore::Document::recalcStyle): Suspend widget hierarchy updates
+ during style recalculation.
+
+ * dom/Element.cpp:
+ (WebCore::Element::attach): Suspend widget hierarchy updates during
+ attach().
+ (WebCore::Element::detach): Suspend widget hierarchy updates during
+ detach().
+
+ * rendering/RenderWidget.cpp:
+ (WebCore::widgetNewParentMap): Returns a static map of pending changes
+ to the widget hierarchy.
+ (WebCore::RenderWidget::suspendWidgetHierarchyUpdates): Increments the
+ suspend count.
+ (WebCore::RenderWidget::resumeWidgetHierarchyUpdates): Decrements the
+ suspend count. If the count is going to be zero, updates the widget
+ hierarchy by executing the pending changes stored in the map.
+ (WebCore::moveWidgetToParentSoon): Updates the widget hierarchy
+ immediately or makes or updates an entry in the map, depending on
+ whether updates are suspended.
+ (WebCore::RenderWidget::destroy): Removed earlier bandaid fix for
+ <rdar://problem/6937089>.
+ (WebCore::RenderWidget::setWidgetGeometry): Assert that widget updates
+ are not suspended, because this function updates the widget’s
+ bounds, which can result in arbitrary native and JavaScript code
+ execution. I think this assertion is true thanks to some deferred-
+ update mechanisms that have already been deployed in other places
+ in the code.
+ (WebCore::RenderWidget::setWidget): Call moveWidgetToParentSoon instead
+ of changing the widget hierarchy directly.
+ * rendering/RenderWidget.h: Declared suspendWidgetHierarchyUpdates()
+ and resumeWidgetHierarchyUpdates().
+
2009-11-03 Pavel Feldman <pfeldman at chromium.org>
Reviewed by Timothy Hatcher.
diff --git a/WebCore/dom/Document.cpp b/WebCore/dom/Document.cpp
index 489ac39..1f534a5 100644
--- a/WebCore/dom/Document.cpp
+++ b/WebCore/dom/Document.cpp
@@ -1238,6 +1238,7 @@ void Document::recalcStyle(StyleChange change)
m_inStyleRecalc = true;
suspendPostAttachCallbacks();
+ RenderWidget::suspendWidgetHierarchyUpdates();
if (view())
view()->pauseScheduledEvents();
@@ -1300,6 +1301,7 @@ bail_out:
if (view())
view()->resumeScheduledEvents();
+ RenderWidget::resumeWidgetHierarchyUpdates();
resumePostAttachCallbacks();
m_inStyleRecalc = false;
diff --git a/WebCore/dom/Element.cpp b/WebCore/dom/Element.cpp
index 9edde25..2e7c44f 100644
--- a/WebCore/dom/Element.cpp
+++ b/WebCore/dom/Element.cpp
@@ -47,6 +47,7 @@
#include "NodeRenderStyle.h"
#include "Page.h"
#include "RenderView.h"
+#include "RenderWidget.h"
#include "TextIterator.h"
#include "XMLNames.h"
@@ -727,6 +728,7 @@ void Element::removedFromDocument()
void Element::attach()
{
suspendPostAttachCallbacks();
+ RenderWidget::suspendWidgetHierarchyUpdates();
createRendererIfNeeded();
ContainerNode::attach();
@@ -739,15 +741,20 @@ void Element::attach()
}
}
+ RenderWidget::resumeWidgetHierarchyUpdates();
resumePostAttachCallbacks();
}
void Element::detach()
{
+ RenderWidget::suspendWidgetHierarchyUpdates();
+
cancelFocusAppearanceUpdate();
if (hasRareData())
rareData()->resetComputedStyle();
ContainerNode::detach();
+
+ RenderWidget::resumeWidgetHierarchyUpdates();
}
bool Element::pseudoStyleCacheIsInvalid(const RenderStyle* currentStyle, RenderStyle* newStyle)
diff --git a/WebCore/rendering/RenderWidget.cpp b/WebCore/rendering/RenderWidget.cpp
index 6cdfcea..0363ba3 100644
--- a/WebCore/rendering/RenderWidget.cpp
+++ b/WebCore/rendering/RenderWidget.cpp
@@ -40,6 +40,55 @@ static HashMap<const Widget*, RenderWidget*>& widgetRendererMap()
return *staticWidgetRendererMap;
}
+static size_t widgetHierarchyUpdateSuspendCount;
+
+typedef HashMap<RefPtr<Widget>, FrameView*> WidgetToParentMap;
+
+static WidgetToParentMap& widgetNewParentMap()
+{
+ DEFINE_STATIC_LOCAL(WidgetToParentMap, map, ());
+ return map;
+}
+
+void RenderWidget::suspendWidgetHierarchyUpdates()
+{
+ widgetHierarchyUpdateSuspendCount++;
+}
+
+void RenderWidget::resumeWidgetHierarchyUpdates()
+{
+ ASSERT(widgetHierarchyUpdateSuspendCount);
+ if (widgetHierarchyUpdateSuspendCount == 1) {
+ WidgetToParentMap map = widgetNewParentMap();
+ widgetNewParentMap().clear();
+ WidgetToParentMap::iterator end = map.end();
+ for (WidgetToParentMap::iterator it = map.begin(); it != end; ++it) {
+ Widget* child = it->first.get();
+ ScrollView* currentParent = child->parent();
+ FrameView* newParent = it->second;
+ if (newParent != currentParent) {
+ if (currentParent)
+ currentParent->removeChild(child);
+ if (newParent)
+ newParent->addChild(child);
+ }
+ }
+ }
+ widgetHierarchyUpdateSuspendCount--;
+}
+
+static void moveWidgetToParentSoon(Widget* child, FrameView* parent)
+{
+ if (!widgetHierarchyUpdateSuspendCount) {
+ if (parent)
+ parent->addChild(child);
+ else
+ child->removeFromParent();
+ return;
+ }
+ widgetNewParentMap().set(child, parent);
+}
+
RenderWidget::RenderWidget(Node* node)
: RenderReplaced(node)
, m_widget(0)
@@ -60,14 +109,6 @@ void RenderWidget::destroy()
// both RenderBox::destroy() and RenderObject::destroy().
// Fix originally made for <rdar://problem/4228818>.
- // <rdar://problem/6937089> suggests that node() can be null by the time we call renderArena()
- // in the end of this function. One way this might happen is if this function was invoked twice
- // in a row, so bail out and turn a crash into an assertion failure in debug builds and a leak
- // in release builds.
- ASSERT(node());
- if (!node())
- return;
-
animation()->cancelAnimations(this);
if (RenderView* v = view())
@@ -94,14 +135,6 @@ void RenderWidget::destroy()
destroyLayer();
}
- // <rdar://problem/6937089> suggests that node() can be null here. One way this might happen is
- // if this function was re-entered (and therefore the null check at the beginning did not fail),
- // so bail out and turn a crash into an assertion failure in debug builds and a leak in release
- // builds.
- ASSERT(node());
- if (!node())
- return;
-
// Grab the arena from node()->document()->renderArena() before clearing the node pointer.
// Clear the node before deref-ing, as this may be deleted when deref is called.
RenderArena* arena = renderArena();
@@ -117,6 +150,7 @@ RenderWidget::~RenderWidget()
bool RenderWidget::setWidgetGeometry(const IntRect& frame)
{
+ ASSERT(!widgetHierarchyUpdateSuspendCount);
if (!node() || m_widget->frameRect() == frame)
return false;
@@ -132,7 +166,7 @@ void RenderWidget::setWidget(PassRefPtr<Widget> widget)
return;
if (m_widget) {
- m_widget->removeFromParent();
+ moveWidgetToParentSoon(m_widget.get(), 0);
widgetRendererMap().remove(m_widget.get());
clearWidget();
}
@@ -150,7 +184,7 @@ void RenderWidget::setWidget(PassRefPtr<Widget> widget)
else
m_widget->show();
}
- m_frameView->addChild(m_widget.get());
+ moveWidgetToParentSoon(m_widget.get(), m_frameView);
}
}
diff --git a/WebCore/rendering/RenderWidget.h b/WebCore/rendering/RenderWidget.h
index 5bd59a8..6cad04a 100644
--- a/WebCore/rendering/RenderWidget.h
+++ b/WebCore/rendering/RenderWidget.h
@@ -42,6 +42,9 @@ public:
void showSubstituteImage(PassRefPtr<Image>);
+ static void suspendWidgetHierarchyUpdates();
+ static void resumeWidgetHierarchyUpdates();
+
protected:
RenderWidget(Node*);
diff --git a/WebKitTools/ChangeLog b/WebKitTools/ChangeLog
index 15b39e9..e2f1e6b 100644
--- a/WebKitTools/ChangeLog
+++ b/WebKitTools/ChangeLog
@@ -1,3 +1,22 @@
+2009-11-03 Dan Bernstein <mitz at apple.com>
+
+ Reviewed by Anders Carlsson and Beth Dakin.
+
+ https://bugs.webkit.org/show_bug.cgi?id=31070
+
+ Added an 'ondestroy' parameter to the test plug-in. When the plug-in is
+ destroyed, it executes the value of the 'ondestroy' parameter as a
+ script.
+
+ * DumpRenderTree/TestNetscapePlugIn.subproj/PluginObject.cpp:
+ (pluginAllocate): Initialize onDestroy.
+ * DumpRenderTree/TestNetscapePlugIn.subproj/PluginObject.h: Define
+ onDestroy.
+ * DumpRenderTree/TestNetscapePlugIn.subproj/main.cpp:
+ (NPP_New): Set onDestroy to the value of the 'ondestroy' parameter, if
+ specified.
+ (NPP_Destroy): Execute the value of 'ondestroy' as a script.
+
2009-11-02 Joanmarie Diggs <joanmarie.diggs at gmail.com>
Reviewed by Xan Lopez.
diff --git a/WebKitTools/DumpRenderTree/TestNetscapePlugIn.subproj/PluginObject.cpp b/WebKitTools/DumpRenderTree/TestNetscapePlugIn.subproj/PluginObject.cpp
index 14280ba..b98a175 100644
--- a/WebKitTools/DumpRenderTree/TestNetscapePlugIn.subproj/PluginObject.cpp
+++ b/WebKitTools/DumpRenderTree/TestNetscapePlugIn.subproj/PluginObject.cpp
@@ -721,6 +721,7 @@ static NPObject *pluginAllocate(NPP npp, NPClass *theClass)
newInstance->eventLogging = FALSE;
newInstance->onStreamLoad = 0;
newInstance->onStreamDestroy = 0;
+ newInstance->onDestroy = 0;
newInstance->onURLNotify = 0;
newInstance->logDestroy = FALSE;
newInstance->logSetWindow = FALSE;
diff --git a/WebKitTools/DumpRenderTree/TestNetscapePlugIn.subproj/PluginObject.h b/WebKitTools/DumpRenderTree/TestNetscapePlugIn.subproj/PluginObject.h
index 7437d04..b34d24a 100644
--- a/WebKitTools/DumpRenderTree/TestNetscapePlugIn.subproj/PluginObject.h
+++ b/WebKitTools/DumpRenderTree/TestNetscapePlugIn.subproj/PluginObject.h
@@ -40,6 +40,7 @@ typedef struct {
NPStream* stream;
char* onStreamLoad;
char* onStreamDestroy;
+ char* onDestroy;
char* onURLNotify;
char* firstUrl;
char* firstHeaders;
diff --git a/WebKitTools/DumpRenderTree/TestNetscapePlugIn.subproj/main.cpp b/WebKitTools/DumpRenderTree/TestNetscapePlugIn.subproj/main.cpp
index 125d2e8..005e92a 100644
--- a/WebKitTools/DumpRenderTree/TestNetscapePlugIn.subproj/main.cpp
+++ b/WebKitTools/DumpRenderTree/TestNetscapePlugIn.subproj/main.cpp
@@ -105,6 +105,8 @@ NPError NPP_New(NPMIMEType pluginType, NPP instance, uint16 mode, int16 argc, ch
pluginLog(instance, "src: %s", argv[i]);
} else if (strcasecmp(argn[i], "cleardocumentduringnew") == 0)
executeScript(obj, "document.body.innerHTML = ''");
+ else if (!strcasecmp(argn[i], "ondestroy"))
+ obj->onDestroy = strdup(argv[i]);
}
#ifndef NP_NO_CARBON
@@ -140,6 +142,11 @@ NPError NPP_Destroy(NPP instance, NPSavedData **save)
{
PluginObject* obj = static_cast<PluginObject*>(instance->pdata);
if (obj) {
+ if (obj->onDestroy) {
+ executeScript(obj, obj->onDestroy);
+ free(obj->onDestroy);
+ }
+
if (obj->onStreamLoad)
free(obj->onStreamLoad);
--
WebKit Debian packaging
More information about the Pkg-webkit-commits
mailing list