JavaScriptCore:
authorggaren <ggaren@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 16 Feb 2006 06:34:49 +0000 (06:34 +0000)
committerggaren <ggaren@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 16 Feb 2006 06:34:49 +0000 (06:34 +0000)
        Reviewed by Maciej, Eric.

        - WebCore half of fix for <rdar://problem/4176077> CrashTracer: 6569
        crashes in DashboardClient at com.apple.JavaScriptCore:
        KJS::Bindings::ObjcFallbackObjectImp::type()

        WebCore and JavaScriptCore weren't sharing Instance objects very
        nicely. I made them use RefPtrs, and sent them to bed without dessert.

        * bindings/jni/jni_instance.cpp: Made _instance a RefPtr
        (JavaInstance::~JavaInstance):
        (JObjectWrapper::JObjectWrapper):
        * bindings/jni/jni_instance.h:
        (KJS::Bindings::JObjectWrapper::ref):
        (KJS::Bindings::JObjectWrapper::deref):
        * bindings/jni/jni_runtime.cpp: Made _array a RefPtr
        (JavaArray::~JavaArray):
        (JavaArray::JavaArray):
        * bindings/jni/jni_runtime.h:
        (KJS::Bindings::JavaArray::operator=):
        * bindings/objc/objc_runtime.h:
        - Prohibited copying because that would muss the ref count.
        - Prohibited construction without instance because an instance wrapper
        without an instance is almost certainly a bug.
        * bindings/objc/objc_runtime.mm:
        (ObjcFallbackObjectImp::ObjcFallbackObjectImp):
        * bindings/runtime.cpp:
        (KJS::Bindings::Instance::Instance):
        (KJS::Bindings::Instance::createBindingForLanguageInstance):
        (KJS::Bindings::Instance::createRuntimeObject):
        * bindings/runtime.h:
        (KJS::Bindings::Instance::ref):
        (KJS::Bindings::Instance::deref):
        * bindings/runtime_object.cpp:
        (RuntimeObjectImp::RuntimeObjectImp):
        (RuntimeObjectImp::fallbackObjectGetter):
        (RuntimeObjectImp::fieldGetter):
        (RuntimeObjectImp::methodGetter):
        (RuntimeObjectImp::getOwnPropertySlot):
        (RuntimeObjectImp::put):
        (RuntimeObjectImp::canPut):
        * bindings/runtime_object.h:
        - Removed ownsInstance data member because RefPtr takes care of
        instance lifetime now.
        - Prohibited copying because that would muss the ref count.
        - Prohibited construction without instance because an instance wrapper
        without an instance is almost certainly a bug.
        (KJS::RuntimeObjectImp::getInternalInstance):

LayoutTests:

        Reviewed by Eric.

        - Layout test for <rdar://problem/4176077> CrashTracer: 6569
        crashes in DashboardClient at com.apple.JavaScriptCore:
        KJS::Bindings::ObjcFallbackObjectImp::type()

        * plugins: Added.
        * plugins/undefined-property-crash-expected.txt: Added.
        * plugins/undefined-property-crash.html: Added.

WebCore:

        Reviewed by Maciej, Eric.

        - WebCore half of fix for <rdar://problem/4176077> CrashTracer: 6569
        crashes in DashboardClient at com.apple.JavaScriptCore:
        KJS::Bindings::ObjcFallbackObjectImp::type()

        WebCore and JavaScriptCore weren't sharing Instance objects very
        nicely. I made them use RefPtrs, and sent them to bed without dessert.

        * khtml/html/html_objectimpl.cpp:
        (WebCore::HTMLAppletElementImpl::HTMLAppletElementImpl): Made
        appletInstance a RefPtr
        (WebCore::HTMLAppletElementImpl::getAppletInstance):
        (WebCore::HTMLAppletElementImpl::detach):
        (WebCore::HTMLEmbedElementImpl::HTMLEmbedElementImpl): Made
        embedInstance a RefPtr
        (WebCore::HTMLEmbedElementImpl::getEmbedInstance):
        (WebCore::HTMLEmbedElementImpl::detach):
        (WebCore::HTMLObjectElementImpl::HTMLObjectElementImpl): Made
        objectInstance a RefPtr
        (WebCore::HTMLObjectElementImpl::getObjectInstance):
        (WebCore::HTMLObjectElementImpl::detach):
        * bindings/js/JSDOMCore.cpp:
        * khtml/ecma/kjs_dom.cpp:
        (KJS::getRuntimeObject):
        * khtml/html/html_objectimpl.h:

WebKitTools:

        Reviewed by Eric.

        * DumpRenderTree/DumpRenderTree.m:
        (-[LayoutTestController invokeUndefinedMethodFromWebScript:withArguments:]):
        Added a dummy method for the sake of LayoutTests/plugins/
        undefined-property-crash.html. (It tests a crash due to fallback
        object use. WebCore won't create a fallback object if the method is
        not defined.)

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

20 files changed:
JavaScriptCore/ChangeLog
JavaScriptCore/bindings/jni/jni_instance.cpp
JavaScriptCore/bindings/jni/jni_instance.h
JavaScriptCore/bindings/jni/jni_runtime.cpp
JavaScriptCore/bindings/jni/jni_runtime.h
JavaScriptCore/bindings/objc/objc_runtime.h
JavaScriptCore/bindings/objc/objc_runtime.mm
JavaScriptCore/bindings/runtime.cpp
JavaScriptCore/bindings/runtime.h
JavaScriptCore/bindings/runtime_object.cpp
JavaScriptCore/bindings/runtime_object.h
LayoutTests/ChangeLog
LayoutTests/plugins/undefined-property-crash-expected.txt [new file with mode: 0644]
LayoutTests/plugins/undefined-property-crash.html [new file with mode: 0644]
WebCore/ChangeLog
WebCore/khtml/ecma/kjs_dom.cpp
WebCore/khtml/html/html_objectimpl.cpp
WebCore/khtml/html/html_objectimpl.h
WebKitTools/ChangeLog
WebKitTools/DumpRenderTree/DumpRenderTree.m

index 5df2a58095fa57f3562be8d7b26c6355cf228a99..14316f03e7c92697225d94e50c5a73b72dca5d8e 100644 (file)
@@ -1,3 +1,54 @@
+2006-02-15  Geoffrey Garen  <ggaren@apple.com>
+
+        Reviewed by Maciej, Eric.
+
+        - WebCore half of fix for <rdar://problem/4176077> CrashTracer: 6569
+        crashes in DashboardClient at com.apple.JavaScriptCore:
+        KJS::Bindings::ObjcFallbackObjectImp::type()
+
+        WebCore and JavaScriptCore weren't sharing Instance objects very
+        nicely. I made them use RefPtrs, and sent them to bed without dessert.
+
+        * bindings/jni/jni_instance.cpp: Made _instance a RefPtr
+        (JavaInstance::~JavaInstance):
+        (JObjectWrapper::JObjectWrapper):
+        * bindings/jni/jni_instance.h:
+        (KJS::Bindings::JObjectWrapper::ref):
+        (KJS::Bindings::JObjectWrapper::deref):
+        * bindings/jni/jni_runtime.cpp: Made _array a RefPtr
+        (JavaArray::~JavaArray):
+        (JavaArray::JavaArray):
+        * bindings/jni/jni_runtime.h:
+        (KJS::Bindings::JavaArray::operator=):
+        * bindings/objc/objc_runtime.h:
+        - Prohibited copying because that would muss the ref count.
+        - Prohibited construction without instance because an instance wrapper
+        without an instance is almost certainly a bug.
+        * bindings/objc/objc_runtime.mm:
+        (ObjcFallbackObjectImp::ObjcFallbackObjectImp):
+        * bindings/runtime.cpp:
+        (KJS::Bindings::Instance::Instance):
+        (KJS::Bindings::Instance::createBindingForLanguageInstance):
+        (KJS::Bindings::Instance::createRuntimeObject):
+        * bindings/runtime.h:
+        (KJS::Bindings::Instance::ref):
+        (KJS::Bindings::Instance::deref):
+        * bindings/runtime_object.cpp:
+        (RuntimeObjectImp::RuntimeObjectImp):
+        (RuntimeObjectImp::fallbackObjectGetter):
+        (RuntimeObjectImp::fieldGetter):
+        (RuntimeObjectImp::methodGetter):
+        (RuntimeObjectImp::getOwnPropertySlot):
+        (RuntimeObjectImp::put):
+        (RuntimeObjectImp::canPut):
+        * bindings/runtime_object.h: 
+        - Removed ownsInstance data member because RefPtr takes care of 
+        instance lifetime now. 
+        - Prohibited copying because that would muss the ref count.
+        - Prohibited construction without instance because an instance wrapper
+        without an instance is almost certainly a bug.
+        (KJS::RuntimeObjectImp::getInternalInstance):
+
 2006-02-15  Geoffrey Garen  <ggaren@apple.com>
 
         Reviewed by John.
index 65235a95759ea8266bc2c48f643558fba6a57dfd..db2dbc59648967786487fe2ab07cc8dfa9e2cca6 100644 (file)
@@ -52,7 +52,6 @@ JavaInstance::JavaInstance (jobject instance, const RootObject *r)
 
 JavaInstance::~JavaInstance () 
 {
-    _instance->deref();
     delete _class; 
 }
 
@@ -334,10 +333,10 @@ JSValue *JavaInstance::valueOf() const
 };
 
 JObjectWrapper::JObjectWrapper(jobject instance)
+: _refCount(0)
 {
     assert (instance != 0);
 
-    _ref = 1;
     // Cache the JNIEnv used to get the global ref for this java instanace.
     // It'll be used to delete the reference.
     _env = getJNIEnv();
index 937af8963bf8b91fdfe84b3227482ecb639af653..9d0122d595ab3c494cf9bcf3ccbf2fefcc268313 100644 (file)
@@ -41,26 +41,27 @@ class JavaClass;
 
 class JObjectWrapper
 {
+friend class RefPtr<JObjectWrapper>;
 friend class JavaArray;
 friend class JavaInstance;
 friend class JavaMethod;
 
 protected:
     JObjectWrapper(jobject instance);    
-    void ref() { _ref++; }
-    void deref() { 
-        _ref--;
-        if (_ref == 0)
-            delete this;
-    }
-    
     ~JObjectWrapper();
+    
+    void ref() { _refCount++; }
+    void deref() 
+    { 
+        if (--_refCount == 0) 
+            delete this; 
+    }
 
     jobject _instance;
 
 private:
     JNIEnv *_env;
-    unsigned int _ref;
+    unsigned int _refCount;
 };
 
 class JavaInstance : public Instance
@@ -92,7 +93,7 @@ private:
     JavaInstance (JavaInstance &);           // prevent copying
     JavaInstance &operator=(JavaInstance &); // prevent copying
     
-    JObjectWrapper *_instance;
+    RefPtr<JObjectWrapper> _instance;
     mutable JavaClass *_class;
 };
 
index a6b7095fc6635f8bdafb00af6b3486ce91019c40..664c52ef2b45603eed3c82bd120ebb3cf7a2d849 100644 (file)
@@ -376,7 +376,6 @@ JavaArray::JavaArray (jobject a, const char *t, const RootObject *r)
 
 JavaArray::~JavaArray () 
 {
-    _array->deref();
     free ((void *)_type);
 }
 
@@ -384,7 +383,6 @@ JavaArray::~JavaArray ()
 JavaArray::JavaArray (const JavaArray &other) : Array() 
 {
     _array = other._array;
-    _array->ref();
     _type = strdup(other._type);
 };
 
index 874bb9e97e8b6c479a73f98f1e1fbab6ba2a8384..e26d3de41d9a79607e88778440c36152e9ddd72b 100644 (file)
@@ -281,11 +281,7 @@ public:
         free ((void *)_type);
         _type = strdup(other._type);
         _root = other._root;
-
-        JObjectWrapper *_oldArray = _array;
         _array = other._array;
-        _array->ref();
-        _oldArray->deref();
         
         return *this;
     };
@@ -303,7 +299,7 @@ public:
     const RootObject *executionContext() const { return _root; }
     
 private:
-    JObjectWrapper *_array;
+    RefPtr<JObjectWrapper> _array;
     unsigned int _length;
     const char *_type;
     const RootObject *_root;
index 0866f150116f611d86d581c71c17efc80101784c..4fc81a404b0c8297345bd31236a2f732131a6f61 100644 (file)
@@ -156,7 +156,6 @@ private:
 
 class ObjcFallbackObjectImp : public JSObject {
 public:
-    ObjcFallbackObjectImp(JSObject *proto);
     ObjcFallbackObjectImp(ObjcInstance *i, const Identifier propertyName);
 
     const ClassInfo *classInfo() const { return &info; }
@@ -173,9 +172,13 @@ public:
     virtual bool toBoolean(ExecState *exec) const;
 
 private:
+    ObjcFallbackObjectImp(); // prevent default construction
+    ObjcFallbackObjectImp(const ObjcFallbackObjectImp& other); // prevent copying
+    ObjcFallbackObjectImp& operator=(const ObjcFallbackObjectImp& other); // ditto
+    
     static const ClassInfo info;
 
-    ObjcInstance *_instance;
+    RefPtr<ObjcInstance> _instance;
     Identifier _item;
 };
 
index ead8b868fe11528ea1b359ec1b10589fac8d19f5..7fb462462212f7c588706a94098c55ee5bc843fd 100644 (file)
@@ -236,16 +236,10 @@ unsigned int ObjcArray::getLength() const
 
 const ClassInfo ObjcFallbackObjectImp::info = {"ObjcFallbackObject", 0, 0, 0};
 
-ObjcFallbackObjectImp::ObjcFallbackObjectImp(JSObject *proto)
-  : JSObject(proto)
-{
-    _instance = 0;
-}
-
 ObjcFallbackObjectImp::ObjcFallbackObjectImp(ObjcInstance *i, const KJS::Identifier propertyName)
+: _instance(i)
+, _item(propertyName)
 {
-    _instance = i;
-    _item = propertyName;
 }
 
 bool ObjcFallbackObjectImp::getOwnPropertySlot(ExecState *exec, const Identifier& propertyName, PropertySlot& slot)
index 2d23c54ef9835e745d54749da6a0f9572b536ca7..6bc42857c34900438e68833abb0dfaee6a18cbb6 100644 (file)
@@ -107,6 +107,12 @@ MethodList &MethodList::operator=(const MethodList &other)
 }
 
 
+Instance::Instance()
+: _executionContext(0)
+, _refCount(0)
+{
+}
+
 static KJSDidExecuteFunctionPtr _DidExecuteFunction;
 
 void Instance::setDidExecuteFunction(KJSDidExecuteFunctionPtr func) { _DidExecuteFunction = func; }
@@ -151,10 +157,10 @@ Instance *Instance::createBindingForLanguageInstance(BindingLanguage language, v
 
 JSObject *Instance::createRuntimeObject(BindingLanguage language, void *nativeInstance, const RootObject *executionContext)
 {
-    Instance *interfaceObject = Instance::createBindingForLanguageInstance(language, (void *)nativeInstance, executionContext);
+    Instance *interfaceObject = Instance::createBindingForLanguageInstance(language, nativeInstance, executionContext);
     
     JSLock lock;
-    return new RuntimeObjectImp(interfaceObject, true);
+    return new RuntimeObjectImp(interfaceObject);
 }
 
 void *Instance::createLanguageInstanceForValue(ExecState *exec, BindingLanguage language, JSObject *value, const RootObject *origin, const RootObject *current)
@@ -187,19 +193,4 @@ void *Instance::createLanguageInstanceForValue(ExecState *exec, BindingLanguage
     return result;
 }
 
-Instance::Instance(const Instance &other) 
-{
-    setExecutionContext(other.executionContext());
-}
-
-Instance &Instance::operator=(const Instance &other)
-{
-    if (this == &other)
-        return *this;
-
-    setExecutionContext(other.executionContext());
-    
-    return *this;
-}
-
-} }
+} } // namespace KJS::Bindings
index 4e23e15672bd7d35e5cdb9e053a6d19706f09a40..8bc2e0b35017c112e70bf978fb91d5fb46761bca 100644 (file)
@@ -134,6 +134,8 @@ public:
         CLanguage
     } BindingLanguage;
 
+    Instance();
+
     static void setDidExecuteFunction(KJSDidExecuteFunctionPtr func);
     static KJSDidExecuteFunctionPtr didExecuteFunction();
     
@@ -141,11 +143,12 @@ public:
     static void* createLanguageInstanceForValue(ExecState*, BindingLanguage, JSObject* value, const RootObject* origin, const RootObject* current);
     static JSObject* createRuntimeObject(BindingLanguage, void* nativeInstance, const RootObject* = 0);
 
-    Instance() : _executionContext(0) {}
-    
-    Instance(const Instance &other);
-
-    Instance &operator=(const Instance &other);
+    void ref() { _refCount++; }
+    void deref() 
+    { 
+        if (--_refCount == 0) 
+            delete this; 
+    }
 
     // These functions are called before and after the main entry points into
     // the native implementations.  They can be used to establish and cleanup
@@ -175,6 +178,11 @@ public:
 
 protected:
     const RootObject* _executionContext;
+    unsigned _refCount;
+
+private:
+    Instance(const Instance &other); // prevent copying
+    Instance &operator=(const Instance &other); // ditto
 };
 
 class Array
index d308ae9e36b4f2febb0004e611fd4a8c3dfd389f..46085d07d28eacbfa240099416149f614c6083d0 100644 (file)
@@ -43,29 +43,15 @@ using namespace Bindings;
 
 const ClassInfo RuntimeObjectImp::info = {"RuntimeObject", 0, 0, 0};
 
-RuntimeObjectImp::RuntimeObjectImp(JSObject *proto)
-  : JSObject(proto)
+RuntimeObjectImp::RuntimeObjectImp(Bindings::Instance *i)
+: instance(i)
 {
-    instance = 0;
-}
-
-RuntimeObjectImp::~RuntimeObjectImp()
-{
-    if (ownsInstance) {
-        delete instance;
-    }
-}
-
-RuntimeObjectImp::RuntimeObjectImp(Bindings::Instance *i, bool oi)
-{
-    ownsInstance = oi;
-    instance = i;
 }
 
 JSValue *RuntimeObjectImp::fallbackObjectGetter(ExecState *exec, JSObject *originalObject, const Identifier& propertyName, const PropertySlot& slot)
 {
     RuntimeObjectImp *thisObj = static_cast<RuntimeObjectImp *>(slot.slotBase());
-    Bindings::Instance *instance = thisObj->instance;
+    Bindings::Instance *instance = thisObj->instance.get();
 
     instance->begin();
 
@@ -80,7 +66,7 @@ JSValue *RuntimeObjectImp::fallbackObjectGetter(ExecState *exec, JSObject *origi
 JSValue *RuntimeObjectImp::fieldGetter(ExecState *exec, JSObject *originalObject, const Identifier& propertyName, const PropertySlot& slot)
 {
     RuntimeObjectImp *thisObj = static_cast<RuntimeObjectImp *>(slot.slotBase());
-    Bindings::Instance *instance = thisObj->instance;
+    Bindings::Instance *instance = thisObj->instance.get();
 
     instance->begin();
 
@@ -96,7 +82,7 @@ JSValue *RuntimeObjectImp::fieldGetter(ExecState *exec, JSObject *originalObject
 JSValue *RuntimeObjectImp::methodGetter(ExecState *exec, JSObject *originalObject, const Identifier& propertyName, const PropertySlot& slot)
 {
     RuntimeObjectImp *thisObj = static_cast<RuntimeObjectImp *>(slot.slotBase());
-    Bindings::Instance *instance = thisObj->instance;
+    Bindings::Instance *instance = thisObj->instance.get();
 
     instance->begin();
 
@@ -117,7 +103,7 @@ bool RuntimeObjectImp::getOwnPropertySlot(ExecState *exec, const Identifier& pro
     
     if (aClass) {
         // See if the instance has a field with the specified name.
-        Field *aField = aClass->fieldNamed(propertyName.ascii(), instance);
+        Field *aField = aClass->fieldNamed(propertyName.ascii(), instance.get());
         if (aField) {
             slot.setCustom(this, fieldGetter);
             instance->end();
@@ -125,7 +111,7 @@ bool RuntimeObjectImp::getOwnPropertySlot(ExecState *exec, const Identifier& pro
         } else {
             // Now check if a method with specified name exists, if so return a function object for
             // that method.
-            MethodList methodList = aClass->methodsNamed(propertyName.ascii(), instance);
+            MethodList methodList = aClass->methodsNamed(propertyName.ascii(), instance.get());
             if (methodList.length() > 0) {
                 slot.setCustom(this, methodGetter);
                 instance->end();
@@ -134,7 +120,7 @@ bool RuntimeObjectImp::getOwnPropertySlot(ExecState *exec, const Identifier& pro
         }
 
         // Try a fallback object.
-        if (!aClass->fallbackObject(exec, instance, propertyName)->isUndefined()) {
+        if (!aClass->fallbackObject(exec, instance.get(), propertyName)->isUndefined()) {
             slot.setCustom(this, fallbackObjectGetter);
             instance->end();
             return true;
@@ -153,7 +139,7 @@ void RuntimeObjectImp::put(ExecState *exec, const Identifier &propertyName,
     instance->begin();
 
     // Set the value of the property.
-    Field *aField = instance->getClass()->fieldNamed(propertyName.ascii(), instance);
+    Field *aField = instance->getClass()->fieldNamed(propertyName.ascii(), instance.get());
     if (aField) {
         getInternalInstance()->setValueOfField(exec, aField, value);
     }
@@ -172,7 +158,7 @@ bool RuntimeObjectImp::canPut(ExecState *exec, const Identifier &propertyName) c
 
     instance->begin();
 
-    Field *aField = instance->getClass()->fieldNamed(propertyName.ascii(), instance);
+    Field *aField = instance->getClass()->fieldNamed(propertyName.ascii(), instance.get());
 
     instance->end();
 
index c6855f832f03a19ecc11bf5cbc1c2c61a8018edf..78217136c703f2b072f57e30da95abb2a2cf1daf 100644 (file)
@@ -33,10 +33,7 @@ namespace KJS {
 
 class RuntimeObjectImp : public JSObject {
 public:
-    RuntimeObjectImp(JSObject *proto);
-    ~RuntimeObjectImp();
-    
-    RuntimeObjectImp(Bindings::Instance *i, bool ownsInstance = true);
+    RuntimeObjectImp(Bindings::Instance *i);
 
     const ClassInfo *classInfo() const { return &info; }
 
@@ -48,18 +45,20 @@ public:
     virtual bool implementsCall() const;
     virtual JSValue *callAsFunction(ExecState *exec, JSObject *thisObj, const List &args);
     
-    void setInternalInstance(Bindings::Instance *i) { instance = i; }
-    Bindings::Instance *getInternalInstance() const { return instance; }
+    Bindings::Instance *getInternalInstance() const { return instance.get(); }
 
     static const ClassInfo info;
 
 private:
+    RuntimeObjectImp(); // prevent default construction
+    RuntimeObjectImp(const RuntimeObjectImp& other); // prevent copying
+    RuntimeObjectImp& operator=(const RuntimeObjectImp& other); // ditto
+    
     static JSValue *fallbackObjectGetter(ExecState *, JSObject *, const Identifier&, const PropertySlot&);
     static JSValue *fieldGetter(ExecState *, JSObject *, const Identifier&, const PropertySlot&);
     static JSValue *methodGetter(ExecState *, JSObject *, const Identifier&, const PropertySlot&);
 
-    Bindings::Instance *instance;
-    bool ownsInstance;
+    RefPtr<Bindings::Instance> instance;
 };
     
 } // namespace
index 38acc080f4474ad053769a3215ef53170032f645..289021c3888f943ebccdcba00ab1e3ea86d33858 100644 (file)
@@ -1,3 +1,15 @@
+2006-02-15  Geoffrey Garen  <ggaren@apple.com>
+
+        Reviewed by Eric.
+
+        - Layout test for <rdar://problem/4176077> CrashTracer: 6569
+        crashes in DashboardClient at com.apple.JavaScriptCore:
+        KJS::Bindings::ObjcFallbackObjectImp::type()
+
+        * plugins: Added.
+        * plugins/undefined-property-crash-expected.txt: Added.
+        * plugins/undefined-property-crash.html: Added.
+
 2006-02-15  Eric Seidel  <eseidel@apple.com>
         
         Add files missing from previous commit.
diff --git a/LayoutTests/plugins/undefined-property-crash-expected.txt b/LayoutTests/plugins/undefined-property-crash-expected.txt
new file mode 100644 (file)
index 0000000..f3e2868
--- /dev/null
@@ -0,0 +1,7 @@
+This test checks for a regression against: rdar://problem/4176077 CrashTracer: 6569 crashes in DashboardClient at com.apple.JavaScriptCore: KJS::Bindings::ObjcFallbackObjectImp::type()
+
+This test only works in DumpRenderTree, because it depends on having a plugin object that it can 'delete.'
+
+PASS: You didn't crash.
+
+
diff --git a/LayoutTests/plugins/undefined-property-crash.html b/LayoutTests/plugins/undefined-property-crash.html
new file mode 100644 (file)
index 0000000..09c1654
--- /dev/null
@@ -0,0 +1,57 @@
+<html>
+<head>
+<script>
+function print(message) {
+    var paragraph = document.createElement("p");
+    paragraph.appendChild(document.createTextNode(message));
+    document.getElementById("console").appendChild(paragraph);
+}
+
+function test2()
+{
+    return layoutTestController.doesNotExist;
+}
+
+function test()
+{
+    if (window.layoutTestController)
+        layoutTestController.dumpAsText();
+    else
+        print("FAIL: window.layoutTestController does not exist");
+        
+    var crasher = test2();
+    delete layoutTestController;
+    
+    // create lots of objects to force a garbage collection
+    var i = 0;
+    var s;
+    while (i < 5000) {
+        i = i + 1.11;
+        s = s + " ";
+    }
+    
+    if (crasher) {} // force call to toBoolean
+    if (crasher == null) {} // force call to type() through call to equal
+    
+    if (window.layoutTestController)   
+        print("FAIL: unable to delete layoutTestController");
+    else
+        print("PASS: You didn't crash.");
+}
+</script>
+       
+</head>
+<body onload="test()">
+<p>
+This test checks for a regression against: rdar://problem/4176077 CrashTracer: 
+6569 crashes in DashboardClient at com.apple.JavaScriptCore:
+KJS::Bindings::ObjcFallbackObjectImp::type()
+</p>
+<p>
+This test only works in DumpRenderTree, because it depends on having a plugin object 
+that it can 'delete.'
+</p>
+<hr>
+<div id="console"></div>
+</body>
+</html>
index 449601ad5a3985aff81c6a70cf196da082e8f1aa..eea4a14446dbbb7e1aa7a7b58cb7a93eacf95137 100644 (file)
@@ -1,3 +1,32 @@
+2006-02-15  Geoffrey Garen  <ggaren@apple.com>
+
+        Reviewed by Maciej, Eric.
+
+        - WebCore half of fix for <rdar://problem/4176077> CrashTracer: 6569 
+        crashes in DashboardClient at com.apple.JavaScriptCore: 
+        KJS::Bindings::ObjcFallbackObjectImp::type() 
+
+        WebCore and JavaScriptCore weren't sharing Instance objects very
+        nicely. I made them use RefPtrs, and sent them to bed without dessert.
+
+        * khtml/html/html_objectimpl.cpp: 
+        (WebCore::HTMLAppletElementImpl::HTMLAppletElementImpl): Made 
+        appletInstance a RefPtr
+        (WebCore::HTMLAppletElementImpl::getAppletInstance):
+        (WebCore::HTMLAppletElementImpl::detach):
+        (WebCore::HTMLEmbedElementImpl::HTMLEmbedElementImpl): Made
+        embedInstance a RefPtr
+        (WebCore::HTMLEmbedElementImpl::getEmbedInstance):
+        (WebCore::HTMLEmbedElementImpl::detach):
+        (WebCore::HTMLObjectElementImpl::HTMLObjectElementImpl): Made
+        objectInstance a RefPtr
+        (WebCore::HTMLObjectElementImpl::getObjectInstance): 
+        (WebCore::HTMLObjectElementImpl::detach):
+        * bindings/js/JSDOMCore.cpp:
+        * khtml/ecma/kjs_dom.cpp:
+        (KJS::getRuntimeObject):
+        * khtml/html/html_objectimpl.h:
+
 2006-02-15  Geoffrey Garen  <ggaren@apple.com>
 
         Reviewed by Eric.
index 148fd0e156c946612976c7738a68db000d87d677..1310424ae4ad9428bdc1d1b8aba075c098cfebb6 100644 (file)
@@ -1399,17 +1399,17 @@ JSValue *getRuntimeObject(ExecState *exec, NodeImpl *n)
         HTMLAppletElementImpl *appletElement = static_cast<HTMLAppletElementImpl *>(n);
         if (appletElement->getAppletInstance())
             // The instance is owned by the applet element.
-            return new RuntimeObjectImp(appletElement->getAppletInstance(), false);
+            return new RuntimeObjectImp(appletElement->getAppletInstance());
     }
     else if (n->hasTagName(embedTag)) {
         HTMLEmbedElementImpl *embedElement = static_cast<HTMLEmbedElementImpl *>(n);
         if (embedElement->getEmbedInstance())
-            return new RuntimeObjectImp(embedElement->getEmbedInstance(), false);
+            return new RuntimeObjectImp(embedElement->getEmbedInstance());
     }
     else if (n->hasTagName(objectTag)) {
         HTMLObjectElementImpl *objectElement = static_cast<HTMLObjectElementImpl *>(n);
         if (objectElement->getObjectInstance())
-            return new RuntimeObjectImp(objectElement->getObjectInstance(), false);
+            return new RuntimeObjectImp(objectElement->getObjectInstance());
     }
 #endif
 
index 3d9f132f6d52863aa57186313f8c28b6ed158e8a..ba7cb892b42b0672e5510fdf05f51c04b62f8077 100644 (file)
@@ -52,16 +52,15 @@ using namespace HTMLNames;
 // -------------------------------------------------------------------------
 
 HTMLAppletElementImpl::HTMLAppletElementImpl(DocumentImpl *doc)
-  : HTMLElementImpl(appletTag, doc)
+: HTMLElementImpl(appletTag, doc)
+, m_allParamsAvailable(false)
 {
-    appletInstance = 0;
-    m_allParamsAvailable = false;
 }
 
 HTMLAppletElementImpl::~HTMLAppletElementImpl()
 {
-    // appletInstance should have been cleaned up in detach().
-    assert(!appletInstance);
+    // m_appletInstance should have been cleaned up in detach().
+    assert(!m_appletInstance);
 }
 
 bool HTMLAppletElementImpl::checkDTD(const NodeImpl* newChild)
@@ -202,8 +201,8 @@ KJS::Bindings::Instance *HTMLAppletElementImpl::getAppletInstance() const
     if (!frame || !frame->javaEnabled())
         return 0;
 
-    if (appletInstance)
-        return appletInstance;
+    if (m_appletInstance)
+        return m_appletInstance.get();
     
     RenderApplet *r = static_cast<RenderApplet*>(renderer());
     if (r) {
@@ -211,9 +210,9 @@ KJS::Bindings::Instance *HTMLAppletElementImpl::getAppletInstance() const
         if (r->widget())
             // Call into the frame (and over the bridge) to pull the Bindings::Instance
             // from the guts of the plugin.
-            appletInstance = frame->getAppletInstanceForWidget(r->widget());
+            m_appletInstance = frame->getAppletInstanceForWidget(r->widget());
     }
-    return appletInstance;
+    return m_appletInstance.get();
 }
 
 void HTMLAppletElementImpl::closeRenderer()
@@ -227,12 +226,7 @@ void HTMLAppletElementImpl::closeRenderer()
 
 void HTMLAppletElementImpl::detach()
 {
-    // Delete appletInstance, because it references the widget owned by the renderer we're about to destroy.
-    if (appletInstance) {
-        delete appletInstance;
-        appletInstance = 0;
-    }
-
+    m_appletInstance = 0;
     HTMLElementImpl::detach();
 }
 
@@ -354,13 +348,14 @@ void HTMLAppletElementImpl::setWidth(const DOMString &value)
 // -------------------------------------------------------------------------
 
 HTMLEmbedElementImpl::HTMLEmbedElementImpl(DocumentImpl *doc)
-    : HTMLElementImpl(embedTag, doc), embedInstance(0)
-{}
+: HTMLElementImpl(embedTag, doc)
+{
+}
 
 HTMLEmbedElementImpl::~HTMLEmbedElementImpl()
 {
-    // embedInstance should have been cleaned up in detach().
-    assert(!embedInstance);
+    // m_embedInstance should have been cleaned up in detach().
+    assert(!m_embedInstance);
 }
 
 bool HTMLEmbedElementImpl::checkDTD(const NodeImpl* newChild)
@@ -374,8 +369,8 @@ KJS::Bindings::Instance *HTMLEmbedElementImpl::getEmbedInstance() const
     if (!frame)
         return 0;
 
-    if (embedInstance)
-        return embedInstance;
+    if (m_embedInstance)
+        return m_embedInstance.get();
     
     RenderObject *r = renderer();
     if (!r) {
@@ -388,13 +383,13 @@ KJS::Bindings::Instance *HTMLEmbedElementImpl::getEmbedInstance() const
         if (Widget *widget = static_cast<RenderWidget *>(r)->widget()) {
             // Call into the frame (and over the bridge) to pull the Bindings::Instance
             // from the guts of the Java VM.
-            embedInstance = frame->getEmbedInstanceForWidget(widget);
+            m_embedInstance = frame->getEmbedInstanceForWidget(widget);
             // Applet may specified with <embed> tag.
-            if (!embedInstance)
-                embedInstance = frame->getAppletInstanceForWidget(widget);
+            if (!m_embedInstance)
+                m_embedInstance = frame->getAppletInstanceForWidget(widget);
         }
     }
-    return embedInstance;
+    return m_embedInstance.get();
 }
 
 bool HTMLEmbedElementImpl::mapToEntry(const QualifiedName& attrName, MappedAttributeEntry& result) const
@@ -503,12 +498,7 @@ void HTMLEmbedElementImpl::attach()
 
 void HTMLEmbedElementImpl::detach()
 {
-    // Delete embedInstance, because it references the widget owned by the renderer we're about to destroy.
-    if (embedInstance) {
-        delete embedInstance;
-        embedInstance = 0;
-    }
-
+    m_embedInstance = 0;
     HTMLElementImpl::detach();
 }
 
@@ -540,7 +530,8 @@ bool HTMLEmbedElementImpl::isURLAttribute(AttributeImpl *attr) const
 // -------------------------------------------------------------------------
 
 HTMLObjectElementImpl::HTMLObjectElementImpl(DocumentImpl *doc) 
-: HTMLElementImpl(objectTag, doc), m_imageLoader(0), objectInstance(0)
+: HTMLElementImpl(objectTag, doc)
+, m_imageLoader(0)
 {
     needWidgetUpdate = false;
     m_useFallbackContent = false;
@@ -550,8 +541,8 @@ HTMLObjectElementImpl::HTMLObjectElementImpl(DocumentImpl *doc)
 
 HTMLObjectElementImpl::~HTMLObjectElementImpl()
 {
-    // objectInstance should have been cleaned up in detach().
-    assert(!objectInstance);
+    // m_objectInstance should have been cleaned up in detach().
+    assert(!m_objectInstance);
     
     delete m_imageLoader;
 }
@@ -567,23 +558,23 @@ KJS::Bindings::Instance *HTMLObjectElementImpl::getObjectInstance() const
     if (!frame)
         return 0;
 
-    if (objectInstance)
-        return objectInstance;
+    if (m_objectInstance)
+        return m_objectInstance.get();
 
     if (RenderObject *r = renderer()) {
         if (r->isWidget()) {
             if (Widget *widget = static_cast<RenderWidget *>(r)->widget()) {
                 // Call into the frame (and over the bridge) to pull the Bindings::Instance
                 // from the guts of the plugin.
-                objectInstance = frame->getObjectInstanceForWidget(widget);
+                m_objectInstance = frame->getObjectInstanceForWidget(widget);
                 // Applet may specified with <object> tag.
-                if (!objectInstance)
-                    objectInstance = frame->getAppletInstanceForWidget(widget);
+                if (!m_objectInstance)
+                    m_objectInstance = frame->getAppletInstanceForWidget(widget);
             }
         }
     }
 
-    return objectInstance;
+    return m_objectInstance.get();
 }
 
 HTMLFormElementImpl *HTMLObjectElementImpl::form() const
@@ -760,12 +751,7 @@ void HTMLObjectElementImpl::detach()
         needWidgetUpdate = true;
     }
 
-    // Delete objectInstance, because it references the widget owned by the renderer we're about to destroy.
-    if (objectInstance) {
-        delete objectInstance;
-        objectInstance = 0;
-    }
-    
+    m_objectInstance = 0;
     HTMLElementImpl::detach();
 }
 
index 00dad6f3367fd12922062763824e95d186664849..9856ce82ffe693f307711253758f5a5c61ff08bb 100644 (file)
@@ -101,7 +101,7 @@ private:
     DOMString oldNameAttr;
     DOMString oldIdAttr;
 
-    mutable KJS::Bindings::Instance* appletInstance;
+    mutable RefPtr<KJS::Bindings::Instance> m_appletInstance;
 
     bool m_allParamsAvailable;
 };
@@ -139,7 +139,7 @@ public:
 private:
     DOMString oldNameAttr;
 
-    mutable KJS::Bindings::Instance* embedInstance;
+    mutable RefPtr<KJS::Bindings::Instance> m_embedInstance;
 };
 
 // -------------------------------------------------------------------------
@@ -248,7 +248,7 @@ private:
     DOMString oldIdAttr;
     DOMString oldNameAttr;
 
-    mutable KJS::Bindings::Instance* objectInstance;
+    mutable RefPtr<KJS::Bindings::Instance> m_objectInstance;
 
     bool m_complete;
     bool m_docNamedItem;
index 1de57b92c3f6aed5a3f8856805aab6b0493a6b19..1c240f44820da4f746ef7ea45e1a56da92c6ca3c 100644 (file)
@@ -1,3 +1,14 @@
+2006-02-15  Geoffrey Garen  <ggaren@apple.com>
+
+        Reviewed by Eric.
+
+        * DumpRenderTree/DumpRenderTree.m:
+        (-[LayoutTestController invokeUndefinedMethodFromWebScript:withArguments:]): 
+        Added a dummy method for the sake of LayoutTests/plugins/
+        undefined-property-crash.html. (It tests a crash due to fallback 
+        object use. WebCore won't create a fallback object if the method is 
+        not defined.)
+
 2006-02-14  Eric Seidel  <eseidel@apple.com>
 
         Reviewed by adele.
index aaae06a6830536e64c15797c8fbd361aaf9b367b..cd682e6deef27df35d1bd4f369611a3fa681b095 100644 (file)
@@ -567,6 +567,11 @@ static void dump(void)
         [(WebHTMLView *)[[frame frameView] documentView] _setDisplaysWithFocusAttributes:flag];
 }
 
+- (id)invokeUndefinedMethodFromWebScript:(NSString *)name withArguments:(NSArray *)args
+{
+    return nil;
+}
+
 @end
 
 @implementation EventSendingController