Commit 2b1d0661 authored by andersca's avatar andersca

JavaScriptCore:

        Reviewed by Darin.

        <rdar://problem/5103077> 
        Crash at _NPN_ReleaseObject when quitting page at http://eshop.macsales.com/shop/ModBook
        
        <rdar://problem/5183692>
        http://bugs.webkit.org/show_bug.cgi?id=13547
        REGRESSION: Crash in _NPN_ReleaseObject when closing Safari on nba.com (13547)
        
        <rdar://problem/5261499>
        CrashTracer: [USER] 75 crashes in Safari at com.apple.JavaScriptCore: KJS::Bindings::CInstance::~CInstance + 40
        
        Have the root object track all live instances of RuntimeObjectImp. When invalidating 
        the root object, also invalidate all live runtime objects by zeroing out their instance ivar.
        This prevents instances from outliving their plug-ins which lead to crashes.
        
        * bindings/c/c_utility.cpp:
        (KJS::Bindings::convertValueToNPVariant):
        * bindings/jni/jni_jsobject.cpp:
        (JavaJSObject::convertValueToJObject):
        * bindings/jni/jni_utility.cpp:
        (KJS::Bindings::convertValueToJValue):
        * bindings/objc/objc_runtime.mm:
        (ObjcFallbackObjectImp::callAsFunction):
        * bindings/runtime_array.cpp:
        (RuntimeArray::RuntimeArray):
        * bindings/runtime_array.h:
        (KJS::RuntimeArray::getConcreteArray):
        * bindings/runtime_method.cpp:
        (RuntimeMethod::callAsFunction):
        * bindings/runtime_method.h:
        * bindings/runtime_object.cpp:
        (RuntimeObjectImp::RuntimeObjectImp):
        (RuntimeObjectImp::~RuntimeObjectImp):
        (RuntimeObjectImp::invalidate):
        (RuntimeObjectImp::fallbackObjectGetter):
        (RuntimeObjectImp::fieldGetter):
        (RuntimeObjectImp::methodGetter):
        (RuntimeObjectImp::getOwnPropertySlot):
        (RuntimeObjectImp::put):
        (RuntimeObjectImp::canPut):
        (RuntimeObjectImp::defaultValue):
        (RuntimeObjectImp::implementsCall):
        (RuntimeObjectImp::callAsFunction):
        (RuntimeObjectImp::getPropertyNames):
        (RuntimeObjectImp::throwInvalidAccessError):
        * bindings/runtime_object.h:
        * bindings/runtime_root.cpp:
        (KJS::Bindings::RootObject::invalidate):
        (KJS::Bindings::RootObject::addRuntimeObject):
        (KJS::Bindings::RootObject::removeRuntimeObject):
        * bindings/runtime_root.h:

LayoutTests:

        Reviewed by Darin.

        Add test that manipulates plug-in script objects after the plug-in has been destroyed.
        
        * plugins/netscape-destroy-plugin-script-objects-expected.txt: Added.
        * plugins/netscape-destroy-plugin-script-objects.html: Added.



git-svn-id: http://svn.webkit.org/repository/webkit/trunk@23538 268f45cc-cd09-0410-ab3c-d52691b4dbfc
parent 04411e84
2007-06-14 Anders Carlsson <andersca@apple.com>
Reviewed by Darin.
<rdar://problem/5103077>
Crash at _NPN_ReleaseObject when quitting page at http://eshop.macsales.com/shop/ModBook
<rdar://problem/5183692>
http://bugs.webkit.org/show_bug.cgi?id=13547
REGRESSION: Crash in _NPN_ReleaseObject when closing Safari on nba.com (13547)
<rdar://problem/5261499>
CrashTracer: [USER] 75 crashes in Safari at com.apple.JavaScriptCore: KJS::Bindings::CInstance::~CInstance + 40
Have the root object track all live instances of RuntimeObjectImp. When invalidating
the root object, also invalidate all live runtime objects by zeroing out their instance ivar.
This prevents instances from outliving their plug-ins which lead to crashes.
* bindings/c/c_utility.cpp:
(KJS::Bindings::convertValueToNPVariant):
* bindings/jni/jni_jsobject.cpp:
(JavaJSObject::convertValueToJObject):
* bindings/jni/jni_utility.cpp:
(KJS::Bindings::convertValueToJValue):
* bindings/objc/objc_runtime.mm:
(ObjcFallbackObjectImp::callAsFunction):
* bindings/runtime_array.cpp:
(RuntimeArray::RuntimeArray):
* bindings/runtime_array.h:
(KJS::RuntimeArray::getConcreteArray):
* bindings/runtime_method.cpp:
(RuntimeMethod::callAsFunction):
* bindings/runtime_method.h:
* bindings/runtime_object.cpp:
(RuntimeObjectImp::RuntimeObjectImp):
(RuntimeObjectImp::~RuntimeObjectImp):
(RuntimeObjectImp::invalidate):
(RuntimeObjectImp::fallbackObjectGetter):
(RuntimeObjectImp::fieldGetter):
(RuntimeObjectImp::methodGetter):
(RuntimeObjectImp::getOwnPropertySlot):
(RuntimeObjectImp::put):
(RuntimeObjectImp::canPut):
(RuntimeObjectImp::defaultValue):
(RuntimeObjectImp::implementsCall):
(RuntimeObjectImp::callAsFunction):
(RuntimeObjectImp::getPropertyNames):
(RuntimeObjectImp::throwInvalidAccessError):
* bindings/runtime_object.h:
* bindings/runtime_root.cpp:
(KJS::Bindings::RootObject::invalidate):
(KJS::Bindings::RootObject::addRuntimeObject):
(KJS::Bindings::RootObject::removeRuntimeObject):
* bindings/runtime_root.h:
2007-06-14 Anders Carlsson <andersca@apple.com>
Reviewed by Mitz.
......
......@@ -116,9 +116,11 @@ void convertValueToNPVariant(ExecState *exec, JSValue *value, NPVariant *result)
if (object->classInfo() == &RuntimeObjectImp::info) {
RuntimeObjectImp* imp = static_cast<RuntimeObjectImp *>(value);
CInstance* instance = static_cast<CInstance*>(imp->getInternalInstance());
NPObject* obj = instance->getObject();
_NPN_RetainObject(obj);
OBJECT_TO_NPVARIANT(obj, *result);
if (instance) {
NPObject* obj = instance->getObject();
_NPN_RetainObject(obj);
OBJECT_TO_NPVARIANT(obj, *result);
}
} else {
Interpreter* originInterpreter = exec->dynamicInterpreter();
RootObject* originRootObject = findRootObject(originInterpreter);
......
......@@ -397,6 +397,9 @@ jobject JavaJSObject::convertValueToJObject (JSValue *value) const
if (imp->classInfo() && strcmp(imp->classInfo()->className, "RuntimeObject") == 0) {
RuntimeObjectImp *runtimeImp = static_cast<RuntimeObjectImp*>(value);
JavaInstance *runtimeInstance = static_cast<JavaInstance *>(runtimeImp->getInternalInstance());
if (!runtimeInstance)
return 0;
return runtimeInstance->javaInstance();
}
else {
......
......@@ -888,7 +888,8 @@ jvalue convertValueToJValue (ExecState *exec, JSValue *value, JNIType _JNIType,
if (objectImp->classInfo() == &RuntimeObjectImp::info) {
RuntimeObjectImp *imp = static_cast<RuntimeObjectImp *>(value);
JavaInstance *instance = static_cast<JavaInstance*>(imp->getInternalInstance());
result.l = instance->javaInstance();
if (instance)
result.l = instance->javaInstance();
}
else if (objectImp->classInfo() == &RuntimeArray::info) {
// Input is a JavaScript Array that was originally created from a Java Array
......
......@@ -251,6 +251,9 @@ JSValue* ObjcFallbackObjectImp::callAsFunction(ExecState* exec, JSObject* thisOb
RuntimeObjectImp* imp = static_cast<RuntimeObjectImp*>(thisObj);
Instance* instance = imp->getInternalInstance();
if (!instance)
return RuntimeObjectImp::throwInvalidAccessError(exec);
instance->begin();
ObjcInstance* objcInstance = static_cast<ObjcInstance*>(instance);
......
......@@ -32,14 +32,8 @@ const ClassInfo RuntimeArray::info = {"RuntimeArray", &ArrayInstance::info, 0, 0
RuntimeArray::RuntimeArray(ExecState *exec, Bindings::Array *a)
: JSObject(exec->lexicalInterpreter()->builtinArrayPrototype())
, _array(a)
{
// Always takes ownership of concrete array.
_array = a;
}
RuntimeArray::~RuntimeArray()
{
delete _array;
}
JSValue *RuntimeArray::lengthGetter(ExecState*, JSObject*, const Identifier&, const PropertySlot& slot)
......
......@@ -26,6 +26,8 @@
#ifndef RUNTIME_ARRAY_H_
#define RUNTIME_ARRAY_H_
#include <wtf/OwnPtr.h>
#include "array_instance.h"
#include "runtime.h"
......@@ -34,7 +36,6 @@ namespace KJS {
class RuntimeArray : public JSObject {
public:
RuntimeArray(ExecState *exec, Bindings::Array *i);
~RuntimeArray();
virtual bool getOwnPropertySlot(ExecState *, const Identifier&, PropertySlot&);
virtual bool getOwnPropertySlot(ExecState *, unsigned, PropertySlot&);
......@@ -48,7 +49,7 @@ public:
unsigned getLength() const { return getConcreteArray()->getLength(); }
Bindings::Array *getConcreteArray() const { return _array; }
Bindings::Array *getConcreteArray() const { return _array.get(); }
static const ClassInfo info;
......@@ -56,7 +57,7 @@ private:
static JSValue *lengthGetter(ExecState *, JSObject *, const Identifier&, const PropertySlot&);
static JSValue *indexGetter(ExecState *, JSObject *, const Identifier&, const PropertySlot&);
Bindings::Array *_array;
OwnPtr<Bindings::Array> _array;
};
} // namespace KJS
......
......@@ -39,10 +39,6 @@ RuntimeMethod::RuntimeMethod(ExecState *exec, const Identifier &ident, Bindings:
{
}
RuntimeMethod::~RuntimeMethod()
{
}
JSValue *RuntimeMethod::lengthGetter(ExecState*, JSObject*, const Identifier&, const PropertySlot& slot)
{
RuntimeMethod *thisObj = static_cast<RuntimeMethod *>(slot.slotBase());
......@@ -87,6 +83,9 @@ JSValue *RuntimeMethod::callAsFunction(ExecState *exec, JSObject *thisObj, const
return throwError(exec, TypeError);
Instance *instance = imp->getInternalInstance();
if (!instance)
return RuntimeObjectImp::throwInvalidAccessError(exec);
instance->begin();
JSValue *aValue = instance->invokeMethod(exec, *_methodList, args);
instance->end();
......
......@@ -38,8 +38,6 @@ class RuntimeMethod : public InternalFunctionImp
public:
RuntimeMethod(ExecState *exec, const Identifier &n, Bindings::MethodList &methodList);
virtual ~RuntimeMethod();
virtual bool getOwnPropertySlot(ExecState *, const Identifier&, PropertySlot&);
virtual JSValue *callAsFunction(ExecState *exec, JSObject *thisObj, const List &args);
......
......@@ -29,6 +29,7 @@
#include "error_object.h"
#include "operations.h"
#include "runtime_method.h"
#include "runtime_root.h"
using namespace KJS;
using namespace Bindings;
......@@ -38,6 +39,19 @@ const ClassInfo RuntimeObjectImp::info = {"RuntimeObject", 0, 0, 0};
RuntimeObjectImp::RuntimeObjectImp(Bindings::Instance *i)
: instance(i)
{
instance->rootObject()->addRuntimeObject(this);
}
RuntimeObjectImp::~RuntimeObjectImp()
{
if (instance)
instance->rootObject()->removeRuntimeObject(this);
}
void RuntimeObjectImp::invalidate()
{
ASSERT(instance);
instance = 0;
}
JSValue *RuntimeObjectImp::fallbackObjectGetter(ExecState* exec, JSObject*, const Identifier& propertyName, const PropertySlot& slot)
......@@ -45,6 +59,9 @@ JSValue *RuntimeObjectImp::fallbackObjectGetter(ExecState* exec, JSObject*, cons
RuntimeObjectImp *thisObj = static_cast<RuntimeObjectImp *>(slot.slotBase());
Bindings::Instance *instance = thisObj->instance.get();
if (!instance)
return throwInvalidAccessError(exec);
instance->begin();
Class *aClass = instance->getClass();
......@@ -56,10 +73,13 @@ JSValue *RuntimeObjectImp::fallbackObjectGetter(ExecState* exec, JSObject*, cons
}
JSValue *RuntimeObjectImp::fieldGetter(ExecState* exec, JSObject*, const Identifier& propertyName, const PropertySlot& slot)
{
{
RuntimeObjectImp *thisObj = static_cast<RuntimeObjectImp *>(slot.slotBase());
Bindings::Instance *instance = thisObj->instance.get();
if (!instance)
return throwInvalidAccessError(exec);
instance->begin();
Class *aClass = instance->getClass();
......@@ -76,6 +96,9 @@ JSValue *RuntimeObjectImp::methodGetter(ExecState* exec, JSObject*, const Identi
RuntimeObjectImp *thisObj = static_cast<RuntimeObjectImp *>(slot.slotBase());
Bindings::Instance *instance = thisObj->instance.get();
if (!instance)
return throwInvalidAccessError(exec);
instance->begin();
Class *aClass = instance->getClass();
......@@ -89,6 +112,11 @@ JSValue *RuntimeObjectImp::methodGetter(ExecState* exec, JSObject*, const Identi
bool RuntimeObjectImp::getOwnPropertySlot(ExecState *exec, const Identifier& propertyName, PropertySlot& slot)
{
if (!instance) {
throwInvalidAccessError(exec);
return false;
}
instance->begin();
Class *aClass = instance->getClass();
......@@ -128,36 +156,37 @@ bool RuntimeObjectImp::getOwnPropertySlot(ExecState *exec, const Identifier& pro
void RuntimeObjectImp::put(ExecState* exec, const Identifier& propertyName, JSValue* value, int)
{
if (!instance) {
throwInvalidAccessError(exec);
return;
}
instance->begin();
// Set the value of the property.
Field *aField = instance->getClass()->fieldNamed(propertyName, instance.get());
if (aField) {
getInternalInstance()->setValueOfField(exec, aField, value);
}
else {
if (getInternalInstance()->supportsSetValueOfUndefinedField()){
getInternalInstance()->setValueOfUndefinedField(exec, propertyName, value);
}
}
if (aField)
instance->setValueOfField(exec, aField, value);
else if (instance->supportsSetValueOfUndefinedField())
instance->setValueOfUndefinedField(exec, propertyName, value);
instance->end();
}
bool RuntimeObjectImp::canPut(ExecState*, const Identifier& propertyName) const
bool RuntimeObjectImp::canPut(ExecState* exec, const Identifier& propertyName) const
{
bool result = false;
if (!instance) {
throwInvalidAccessError(exec);
return false;
}
instance->begin();
Field *aField = instance->getClass()->fieldNamed(propertyName, instance.get());
instance->end();
if (aField)
return true;
return result;
return aField;
}
bool RuntimeObjectImp::deleteProperty(ExecState*, const Identifier&)
......@@ -166,13 +195,16 @@ bool RuntimeObjectImp::deleteProperty(ExecState*, const Identifier&)
return false;
}
JSValue *RuntimeObjectImp::defaultValue(ExecState*, JSType hint) const
JSValue *RuntimeObjectImp::defaultValue(ExecState* exec, JSType hint) const
{
if (!instance)
return throwInvalidAccessError(exec);
JSValue *result;
instance->begin();
result = getInternalInstance()->defaultValue(hint);
result = instance->defaultValue(hint);
instance->end();
......@@ -181,11 +213,17 @@ JSValue *RuntimeObjectImp::defaultValue(ExecState*, JSType hint) const
bool RuntimeObjectImp::implementsCall() const
{
return getInternalInstance()->implementsCall();
if (!instance)
return false;
return instance->implementsCall();
}
JSValue *RuntimeObjectImp::callAsFunction(ExecState* exec, JSObject*, const List& args)
{
if (!instance)
return throwInvalidAccessError(exec);
instance->begin();
JSValue *aValue = instance->invokeDefaultMethod(exec, args);
......@@ -197,8 +235,17 @@ JSValue *RuntimeObjectImp::callAsFunction(ExecState* exec, JSObject*, const List
void RuntimeObjectImp::getPropertyNames(ExecState* exec, PropertyNameArray& propertyNames)
{
if (!instance) {
throwInvalidAccessError(exec);
return;
}
instance->begin();
instance->getPropertyNames(exec, propertyNames);
instance->end();
}
JSObject* RuntimeObjectImp::throwInvalidAccessError(ExecState* exec)
{
return throwError(exec, ReferenceError, "Trying to access object from destroyed plug-in.");
}
......@@ -36,7 +36,8 @@ namespace KJS {
class RuntimeObjectImp : public JSObject {
public:
RuntimeObjectImp(Bindings::Instance *i);
virtual ~RuntimeObjectImp();
const ClassInfo *classInfo() const { return &info; }
virtual bool getOwnPropertySlot(ExecState *, const Identifier&, PropertySlot&);
......@@ -48,8 +49,11 @@ public:
virtual JSValue *callAsFunction(ExecState *exec, JSObject *thisObj, const List &args);
virtual void getPropertyNames(ExecState*, PropertyNameArray&);
void invalidate();
Bindings::Instance *getInternalInstance() const { return instance.get(); }
static JSObject* throwInvalidAccessError(ExecState*);
static const ClassInfo info;
private:
......
......@@ -23,9 +23,12 @@
* OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
*/
#include "config.h"
#include "runtime_root.h"
#include "object.h"
#include "runtime_root.h"
#include "runtime.h"
#include "runtime_object.h"
#include <wtf/HashCountedSet.h>
#include <wtf/HashSet.h>
......@@ -218,6 +221,14 @@ void RootObject::invalidate()
if (!m_isValid)
return;
{
HashSet<RuntimeObjectImp*>::iterator end = m_runtimeObjects.end();
for (HashSet<RuntimeObjectImp*>::iterator it = m_runtimeObjects.begin(); it != end; ++it)
(*it)->invalidate();
m_runtimeObjects.clear();
}
m_isValid = false;
m_nativeHandle = 0;
......@@ -276,4 +287,20 @@ Interpreter* RootObject::interpreter() const
return m_interpreter.get();
}
void RootObject::addRuntimeObject(RuntimeObjectImp* object)
{
ASSERT(m_isValid);
ASSERT(!m_runtimeObjects.contains(object));
m_runtimeObjects.add(object);
}
void RootObject::removeRuntimeObject(RuntimeObjectImp* object)
{
ASSERT(m_isValid);
ASSERT(m_runtimeObjects.contains(object));
m_runtimeObjects.remove(object);
}
} } // namespace KJS::Bindings
......@@ -32,10 +32,13 @@
#endif
#include "protect.h"
#include <wtf/HashSet.h>
#include <wtf/Noncopyable.h>
namespace KJS {
class RuntimeObjectImp;
namespace Bindings {
class RootObject;
......@@ -82,6 +85,8 @@ public:
static void dispatchToJavaScriptThread(JSObjectCallContext *context);
#endif
void addRuntimeObject(RuntimeObjectImp*);
void removeRuntimeObject(RuntimeObjectImp*);
private:
RootObject(const void* nativeHandle, PassRefPtr<Interpreter> interpreter);
~RootObject();
......@@ -93,6 +98,8 @@ private:
RefPtr<Interpreter> m_interpreter;
ProtectCountSet m_protectCountSet;
HashSet<RuntimeObjectImp*> m_runtimeObjects;
#if PLATFORM(MAC)
static CreateRootObjectFunction _createRootObject;
static CFRunLoopRef _runLoop;
......
2007-06-14 Anders Carlsson <andersca@apple.com>
Reviewed by Darin.
Add test that manipulates plug-in script objects after the plug-in has been destroyed.
* plugins/netscape-destroy-plugin-script-objects-expected.txt: Added.
* plugins/netscape-destroy-plugin-script-objects.html: Added.
2007-06-14 George Staikos <staikos@kde.org>
Reviewed by Maciej.
PLUGIN: NPP_Destroy
This tests that accessing plug-in script objects from a destroyed plugin cause throws the right exceptions.
SUCCESS
<html>
<script>
function runTest()
{
if (window.layoutTestController)
layoutTestController.dumpAsText();
var successCount = 0;
var plugin = document.getElementById("testPlugin");
plugin.logDestroy = true;
var testObject = plugin.testObject;
plugin.parentNode.removeChild(plugin);
try {
testObject.property;
} catch (e) {
if (e instanceof ReferenceError)
successCount++;
}
try {
testObject.property = 'hello';
} catch (e) {
if (e instanceof ReferenceError)
successCount++;
}
if (successCount == 2)
document.getElementById('result').innerHTML = 'SUCCESS';
}
</script>
<body onload="runTest();">
This tests that accessing plug-in script objects from a destroyed plugin cause throws the right exceptions.
<div id="result">FAILURE</div>
<embed id="testPlugin" type="application/x-webkit-test-netscape" width="200" height="200"></embed>
</body>
</html>
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment