[SCM] WebKit Debian packaging branch, debian/experimental, updated. upstream/1.3.3-9427-gc2be6fc
ap at apple.com
ap at apple.com
Wed Dec 22 15:31:18 UTC 2010
The following commit has been merged in the debian/experimental branch:
commit 7a2d4f42100cf32dba72c46fd0f69c122aaa06f5
Author: ap at apple.com <ap at apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Date: Fri Nov 5 17:02:24 2010 +0000
Reviewed by Darin Adler.
https://bugs.webkit.org/show_bug.cgi?id=49008
<rdar://problem/7906226> Frequent crashes on mail.yahoo.co.jp
WebCore:
Instance::m_runtimeObject used to be zeroed out by RuntimeObject destructor. But the
destructor may not be called immediately - GC first marks an object as dead, and only
destroys it when its cell is overwritten. So, m_runtimeObject would keep pointing to a dead
object.
Functions in JSPluginElementFunctions.cpp put the RuntimeObject on stack for later use,
but if it's already dead, it can be overwritten before use.
The solution is of course to use WeakGCPtr, which returns 0 for dead objects.
* bridge/jsc/BridgeJSC.cpp:
(JSC::Bindings::Instance::Instance):
(JSC::Bindings::Instance::~Instance):
(JSC::Bindings::Instance::createRuntimeObject):
(JSC::Bindings::Instance::willDestroyRuntimeObject):
(JSC::Bindings::Instance::willInvalidateRuntimeObject):
* bridge/jsc/BridgeJSC.h:
* bridge/runtime_object.cpp:
(JSC::Bindings::RuntimeObject::~RuntimeObject):
(JSC::Bindings::RuntimeObject::invalidate):
WebKit:
Callers of NetscapePluginInstanceProxy::waitForReply() are not prepared to be deleted during
the call, unless it returns 0. There are two reasons for NetscapePluginInstanceProxy to be
deleted during wait:
- plugin crashed;
- plugin was stopped (e.g. due to a DOM modification performed by another reply that came in
while waiting).
We didn't recognize the latter.
* Plugins/Hosted/NetscapePluginHostProxy.mm:
(WebKit::PluginDestroyDeferrer::~PluginDestroyDeferrer):
* Plugins/Hosted/NetscapePluginInstanceProxy.h:
(WebKit::NetscapePluginInstanceProxy::waitForReply):
* Plugins/Hosted/NetscapePluginInstanceProxy.mm:
(WebKit::NetscapePluginInstanceProxy::didCallPluginFunction):
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@71426 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
index 7be64f3..3579a56 100644
--- a/WebCore/ChangeLog
+++ b/WebCore/ChangeLog
@@ -1,3 +1,31 @@
+2010-11-04 Alexey Proskuryakov <ap at apple.com>
+
+ Reviewed by Darin Adler.
+
+ https://bugs.webkit.org/show_bug.cgi?id=49008
+ <rdar://problem/7906226> Frequent crashes on mail.yahoo.co.jp
+
+ Instance::m_runtimeObject used to be zeroed out by RuntimeObject destructor. But the
+ destructor may not be called immediately - GC first marks an object as dead, and only
+ destroys it when its cell is overwritten. So, m_runtimeObject would keep pointing to a dead
+ object.
+
+ Functions in JSPluginElementFunctions.cpp put the RuntimeObject on stack for later use,
+ but if it's already dead, it can be overwritten before use.
+
+ The solution is of course to use WeakGCPtr, which returns 0 for dead objects.
+
+ * bridge/jsc/BridgeJSC.cpp:
+ (JSC::Bindings::Instance::Instance):
+ (JSC::Bindings::Instance::~Instance):
+ (JSC::Bindings::Instance::createRuntimeObject):
+ (JSC::Bindings::Instance::willDestroyRuntimeObject):
+ (JSC::Bindings::Instance::willInvalidateRuntimeObject):
+ * bridge/jsc/BridgeJSC.h:
+ * bridge/runtime_object.cpp:
+ (JSC::Bindings::RuntimeObject::~RuntimeObject):
+ (JSC::Bindings::RuntimeObject::invalidate):
+
2010-11-05 Chris Marrin <cmarrin at apple.com>
Reviewed by Simon Fraser.
diff --git a/WebCore/bridge/jsc/BridgeJSC.cpp b/WebCore/bridge/jsc/BridgeJSC.cpp
index 9b3af25..49ffb2a 100644
--- a/WebCore/bridge/jsc/BridgeJSC.cpp
+++ b/WebCore/bridge/jsc/BridgeJSC.cpp
@@ -51,7 +51,6 @@ Array::~Array()
Instance::Instance(PassRefPtr<RootObject> rootObject)
: m_rootObject(rootObject)
- , m_runtimeObject(0)
{
ASSERT(m_rootObject);
}
@@ -59,6 +58,7 @@ Instance::Instance(PassRefPtr<RootObject> rootObject)
Instance::~Instance()
{
ASSERT(!m_runtimeObject);
+ ASSERT(!m_runtimeObject.hasDeadObject());
}
static KJSDidExecuteFunctionPtr s_didExecuteFunction;
@@ -87,12 +87,14 @@ JSObject* Instance::createRuntimeObject(ExecState* exec)
{
ASSERT(m_rootObject);
ASSERT(m_rootObject->isValid());
- if (m_runtimeObject)
- return m_runtimeObject;
+ if (RuntimeObject* existingObject = m_runtimeObject.get())
+ return existingObject;
+
JSLock lock(SilenceAssertionsOnly);
- m_runtimeObject = newRuntimeObject(exec);
- m_rootObject->addRuntimeObject(m_runtimeObject);
- return m_runtimeObject;
+ RuntimeObject* newObject = newRuntimeObject(exec);
+ m_runtimeObject = newObject;
+ m_rootObject->addRuntimeObject(newObject);
+ return newObject;
}
RuntimeObject* Instance::newRuntimeObject(ExecState* exec)
@@ -101,19 +103,18 @@ RuntimeObject* Instance::newRuntimeObject(ExecState* exec)
return new (exec)RuntimeObject(exec, exec->lexicalGlobalObject(), this);
}
-void Instance::willDestroyRuntimeObject()
+void Instance::willDestroyRuntimeObject(RuntimeObject* object)
{
ASSERT(m_rootObject);
ASSERT(m_rootObject->isValid());
- ASSERT(m_runtimeObject);
- m_rootObject->removeRuntimeObject(m_runtimeObject);
- m_runtimeObject = 0;
+ m_rootObject->removeRuntimeObject(object);
+ m_runtimeObject.clear(object);
}
-void Instance::willInvalidateRuntimeObject()
+void Instance::willInvalidateRuntimeObject(RuntimeObject* object)
{
- ASSERT(m_runtimeObject);
- m_runtimeObject = 0;
+ ASSERT(object);
+ m_runtimeObject.clear(object);
}
RootObject* Instance::rootObject() const
diff --git a/WebCore/bridge/jsc/BridgeJSC.h b/WebCore/bridge/jsc/BridgeJSC.h
index 9cc9140..96974d9 100644
--- a/WebCore/bridge/jsc/BridgeJSC.h
+++ b/WebCore/bridge/jsc/BridgeJSC.h
@@ -85,8 +85,8 @@ public:
virtual Class* getClass() const = 0;
JSObject* createRuntimeObject(ExecState*);
- void willInvalidateRuntimeObject();
- void willDestroyRuntimeObject();
+ void willInvalidateRuntimeObject(RuntimeObject*);
+ void willDestroyRuntimeObject(RuntimeObject*);
// Returns false if the value was not set successfully.
virtual bool setValueOfUndefinedField(ExecState*, const Identifier&, JSValue) { return false; }
@@ -122,7 +122,7 @@ protected:
RefPtr<RootObject> m_rootObject;
private:
- RuntimeObject* m_runtimeObject;
+ WeakGCPtr<RuntimeObject> m_runtimeObject;
};
class Array : public Noncopyable {
diff --git a/WebCore/bridge/runtime_object.cpp b/WebCore/bridge/runtime_object.cpp
index 9c5d7b6..368f7b0 100644
--- a/WebCore/bridge/runtime_object.cpp
+++ b/WebCore/bridge/runtime_object.cpp
@@ -55,14 +55,14 @@ RuntimeObject::RuntimeObject(ExecState*, JSGlobalObject* globalObject, NonNullPa
RuntimeObject::~RuntimeObject()
{
if (m_instance)
- m_instance->willDestroyRuntimeObject();
+ m_instance->willDestroyRuntimeObject(this);
}
void RuntimeObject::invalidate()
{
ASSERT(m_instance);
if (m_instance)
- m_instance->willInvalidateRuntimeObject();
+ m_instance->willInvalidateRuntimeObject(this);
m_instance = 0;
}
diff --git a/WebKit/mac/ChangeLog b/WebKit/mac/ChangeLog
index 40f6f0c..73903b3 100644
--- a/WebKit/mac/ChangeLog
+++ b/WebKit/mac/ChangeLog
@@ -1,3 +1,26 @@
+2010-11-04 Alexey Proskuryakov <ap at apple.com>
+
+ Reviewed by Darin Adler.
+
+ https://bugs.webkit.org/show_bug.cgi?id=49008
+ <rdar://problem/7906226> Frequent crashes on mail.yahoo.co.jp
+
+ Callers of NetscapePluginInstanceProxy::waitForReply() are not prepared to be deleted during
+ the call, unless it returns 0. There are two reasons for NetscapePluginInstanceProxy to be
+ deleted during wait:
+ - plugin crashed;
+ - plugin was stopped (e.g. due to a DOM modification performed by another reply that came in
+ while waiting).
+
+ We didn't recognize the latter.
+
+ * Plugins/Hosted/NetscapePluginHostProxy.mm:
+ (WebKit::PluginDestroyDeferrer::~PluginDestroyDeferrer):
+ * Plugins/Hosted/NetscapePluginInstanceProxy.h:
+ (WebKit::NetscapePluginInstanceProxy::waitForReply):
+ * Plugins/Hosted/NetscapePluginInstanceProxy.mm:
+ (WebKit::NetscapePluginInstanceProxy::didCallPluginFunction):
+
2010-11-05 Chris Marrin <cmarrin at apple.com>
Reviewed by Simon Fraser.
diff --git a/WebKit/mac/Plugins/Hosted/NetscapePluginHostProxy.mm b/WebKit/mac/Plugins/Hosted/NetscapePluginHostProxy.mm
index 067b8bb..4506f03 100644
--- a/WebKit/mac/Plugins/Hosted/NetscapePluginHostProxy.mm
+++ b/WebKit/mac/Plugins/Hosted/NetscapePluginHostProxy.mm
@@ -74,7 +74,8 @@ public:
~PluginDestroyDeferrer()
{
- m_proxy->didCallPluginFunction();
+ bool stopped;
+ m_proxy->didCallPluginFunction(stopped);
}
private:
diff --git a/WebKit/mac/Plugins/Hosted/NetscapePluginInstanceProxy.h b/WebKit/mac/Plugins/Hosted/NetscapePluginInstanceProxy.h
index 3081120..f784ade 100644
--- a/WebKit/mac/Plugins/Hosted/NetscapePluginInstanceProxy.h
+++ b/WebKit/mac/Plugins/Hosted/NetscapePluginInstanceProxy.h
@@ -153,7 +153,7 @@ public:
void invalidate();
void willCallPluginFunction();
- void didCallPluginFunction();
+ void didCallPluginFunction(bool& stopped);
bool shouldStop();
uint32_t nextRequestID();
@@ -262,8 +262,14 @@ public:
ASSERT(reply->m_type == T::ReplyType);
m_waitingForReply = false;
-
- didCallPluginFunction();
+
+ bool stopped = false;
+ didCallPluginFunction(stopped);
+ if (stopped) {
+ // The instance proxy may have been deleted from didCallPluginFunction(), so a null reply needs to be returned.
+ delete static_cast<T*>(reply);
+ return std::auto_ptr<T>();
+ }
return std::auto_ptr<T>(static_cast<T*>(reply));
}
diff --git a/WebKit/mac/Plugins/Hosted/NetscapePluginInstanceProxy.mm b/WebKit/mac/Plugins/Hosted/NetscapePluginInstanceProxy.mm
index a2d4a96..ecaa0d6 100644
--- a/WebKit/mac/Plugins/Hosted/NetscapePluginInstanceProxy.mm
+++ b/WebKit/mac/Plugins/Hosted/NetscapePluginInstanceProxy.mm
@@ -1455,7 +1455,7 @@ void NetscapePluginInstanceProxy::willCallPluginFunction()
m_pluginFunctionCallDepth++;
}
-void NetscapePluginInstanceProxy::didCallPluginFunction()
+void NetscapePluginInstanceProxy::didCallPluginFunction(bool& stopped)
{
ASSERT(m_pluginFunctionCallDepth > 0);
m_pluginFunctionCallDepth--;
@@ -1465,6 +1465,7 @@ void NetscapePluginInstanceProxy::didCallPluginFunction()
if (!m_pluginFunctionCallDepth && m_shouldStopSoon) {
m_shouldStopSoon = false;
[m_pluginView stop];
+ stopped = true;
}
}
--
WebKit Debian packaging
More information about the Pkg-webkit-commits
mailing list