Changeset 136168 in webkit

Timestamp:

Nov 29, 2012, 2:59:14 PM (14 years ago)

Author:

abarth@webkit.org

Message:

[V8] DOM callbacks shouldn't reimplement ScopedPersistent they should use it
​https://bugs.webkit.org/show_bug.cgi?id=103662

Reviewed by Eric Seidel.

This patch replaces yet another instance of the ScopedPersistent
pattern with ScopedPersistent.

• bindings/scripts/CodeGeneratorV8.pm:

(GenerateCallbackHeader):
(GenerateCallbackImplementation):

• bindings/v8/custom/V8CustomSQLStatementErrorCallback.cpp:

(WebCore::V8SQLStatementErrorCallback::handleEvent):

• bindings/v8/custom/V8MutationCallbackCustom.cpp:

(WebCore::V8MutationCallback::handleEvent):

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• bindings/scripts/CodeGeneratorV8.pm (modified) (4 diffs)
• bindings/scripts/test/V8/V8TestCallback.cpp (modified) (7 diffs)
• bindings/scripts/test/V8/V8TestCallback.h (modified) (2 diffs)
• bindings/v8/custom/V8CustomSQLStatementErrorCallback.cpp (modified) (1 diff)
• bindings/v8/custom/V8MutationCallbackCustom.cpp (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2012-11-29  Adam Barth  <abarth@webkit.org>
+
+        [V8] DOM callbacks shouldn't reimplement ScopedPersistent they should use it
+        https://bugs.webkit.org/show_bug.cgi?id=103662
+
+        Reviewed by Eric Seidel.
+
+        This patch replaces yet another instance of the ScopedPersistent
+        pattern with ScopedPersistent.
+
+        * bindings/scripts/CodeGeneratorV8.pm:
+        (GenerateCallbackHeader):
+        (GenerateCallbackImplementation):
+        * bindings/v8/custom/V8CustomSQLStatementErrorCallback.cpp:
+        (WebCore::V8SQLStatementErrorCallback::handleEvent):
+        * bindings/v8/custom/V8MutationCallbackCustom.cpp:
+        (WebCore::V8MutationCallback::handleEvent):
+
 2012-11-29  Min Qin  <qinmin@chromium.org>
 

trunk/Source/WebCore/bindings/scripts/CodeGeneratorV8.pm

@@ -3264 +3264 @@
     push(@unsortedIncludes, "#include \"ActiveDOMCallback.h\"");
     push(@unsortedIncludes, "#include \"$interfaceName.h\"");
+    push(@unsortedIncludes, "#include \"ScopedPersistent.h\"");
     push(@unsortedIncludes, "#include \"WorldContextHandle.h\"");
     push(@unsortedIncludes, "#include <v8.h>");
@@ -3316 +3317 @@
     {
         ${v8InterfaceName}* object = static_cast<${v8InterfaceName}*>(parameter);
-        object->m_callback.Dispose();
-        object->m_callback.Clear();
-    }
-
-    // FIXME: m_callback should be a ScopedPersistent.
-    v8::Persistent<v8::Object> m_callback;
+        object->m_callback.clear();
+    }
+
+    ScopedPersistent<v8::Object> m_callback;
     WorldContextHandle m_worldContext;
 };
@@ -3353 +3352 @@
 ${v8InterfaceName}::${v8InterfaceName}(v8::Handle<v8::Object> callback, ScriptExecutionContext* context, v8::Handle<v8::Object> owner)
     : ActiveDOMCallback(context)
-    , m_callback(v8::Persistent<v8::Object>::New(callback))
+    , m_callback(callback)
     , m_worldContext(UseCurrentWorld)
 {
-    if (!owner.IsEmpty()) {
-        owner->SetHiddenValue(V8HiddenPropertyName::callback(), callback);
-        m_callback.MakeWeak(this, &${v8InterfaceName}::weakCallback);
-    }
+    if (owner.IsEmpty())
+        return;
+    owner->SetHiddenValue(V8HiddenPropertyName::callback(), callback);
+    m_callback.get().MakeWeak(this, &${v8InterfaceName}::weakCallback);
 }
 
 ${v8InterfaceName}::~${v8InterfaceName}()
 {
-    if (!m_callback.IsEmpty())
-        m_callback.Dispose();
 }
 
@@ -3435 +3432 @@
                     next if $param->type ne $thisType;
                     my $paramName = $param->name;
-                    push(@implContent, "    return !invokeCallback(m_callback, v8::Handle<v8::Object>::Cast(${paramName}Handle), " . scalar(@params) . ", argv, callbackReturnValue, scriptExecutionContext());\n");
+                    push(@implContent, "    return !invokeCallback(m_callback.get(), v8::Handle<v8::Object>::Cast(${paramName}Handle), " . scalar(@params) . ", argv, callbackReturnValue, scriptExecutionContext());\n");
                     last;
                 }
             } else {
-                push(@implContent, "    return !invokeCallback(m_callback, " . scalar(@params) . ", argv, callbackReturnValue, scriptExecutionContext());\n");
+                push(@implContent, "    return !invokeCallback(m_callback.get(), " . scalar(@params) . ", argv, callbackReturnValue, scriptExecutionContext());\n");
             }
             push(@implContent, "}\n");

trunk/Source/WebCore/bindings/scripts/test/V8/V8TestCallback.cpp

@@ -42 +42 @@
 V8TestCallback::V8TestCallback(v8::Handle<v8::Object> callback, ScriptExecutionContext* context, v8::Handle<v8::Object> owner)
     : ActiveDOMCallback(context)
-    , m_callback(v8::Persistent<v8::Object>::New(callback))
+    , m_callback(callback)
     , m_worldContext(UseCurrentWorld)
 {
-    if (!owner.IsEmpty()) {
-        owner->SetHiddenValue(V8HiddenPropertyName::callback(), callback);
-        m_callback.MakeWeak(this, &V8TestCallback::weakCallback);
-    }
+    if (owner.IsEmpty())
+        return;
+    owner->SetHiddenValue(V8HiddenPropertyName::callback(), callback);
+    m_callback.get().MakeWeak(this, &V8TestCallback::weakCallback);
 }
 
 V8TestCallback::~V8TestCallback()
 {
-    if (!m_callback.IsEmpty())
-        m_callback.Dispose();
 }
 
@@ -76 +74 @@
 
     bool callbackReturnValue = false;
-    return !invokeCallback(m_callback, 0, argv, callbackReturnValue, scriptExecutionContext());
+    return !invokeCallback(m_callback.get(), 0, argv, callbackReturnValue, scriptExecutionContext());
 }
 
@@ -104 +102 @@
 
     bool callbackReturnValue = false;
-    return !invokeCallback(m_callback, 1, argv, callbackReturnValue, scriptExecutionContext());
+    return !invokeCallback(m_callback.get(), 1, argv, callbackReturnValue, scriptExecutionContext());
 }
 
@@ -139 +137 @@
 
     bool callbackReturnValue = false;
-    return !invokeCallback(m_callback, 2, argv, callbackReturnValue, scriptExecutionContext());
+    return !invokeCallback(m_callback.get(), 2, argv, callbackReturnValue, scriptExecutionContext());
 }
 
@@ -167 +165 @@
 
     bool callbackReturnValue = false;
-    return !invokeCallback(m_callback, 1, argv, callbackReturnValue, scriptExecutionContext());
+    return !invokeCallback(m_callback.get(), 1, argv, callbackReturnValue, scriptExecutionContext());
 }
 
@@ -195 +193 @@
 
     bool callbackReturnValue = false;
-    return !invokeCallback(m_callback, 1, argv, callbackReturnValue, scriptExecutionContext());
+    return !invokeCallback(m_callback.get(), 1, argv, callbackReturnValue, scriptExecutionContext());
 }
 
@@ -232 +230 @@
 
     bool callbackReturnValue = false;
-    return !invokeCallback(m_callback, v8::Handle<v8::Object>::Cast(thisClassParamHandle), 2, argv, callbackReturnValue, scriptExecutionContext());
+    return !invokeCallback(m_callback.get(), v8::Handle<v8::Object>::Cast(thisClassParamHandle), 2, argv, callbackReturnValue, scriptExecutionContext());
 }
 

trunk/Source/WebCore/bindings/scripts/test/V8/V8TestCallback.h

@@ -25 +25 @@
 
 #include "ActiveDOMCallback.h"
+#include "ScopedPersistent.h"
 #include "TestCallback.h"
 #include "WorldContextHandle.h"
@@ -61 +62 @@
     {
         V8TestCallback* object = static_cast<V8TestCallback*>(parameter);
-        object->m_callback.Dispose();
-        object->m_callback.Clear();
+        object->m_callback.clear();
     }
 
-    // FIXME: m_callback should be a ScopedPersistent.
-    v8::Persistent<v8::Object> m_callback;
+    ScopedPersistent<v8::Object> m_callback;
     WorldContextHandle m_worldContext;
 };

trunk/Source/WebCore/bindings/v8/custom/V8CustomSQLStatementErrorCallback.cpp

@@ -74 +74 @@
     // the error callback did not return false, or there was no error callback.
     // Jump to the last step in the overall steps.
-    return invokeCallback(m_callback, 2, argv, callbackReturnValue, scriptExecutionContext()) || callbackReturnValue;
+    return invokeCallback(m_callback.get(), 2, argv, callbackReturnValue, scriptExecutionContext()) || callbackReturnValue;
 }
 

trunk/Source/WebCore/bindings/v8/custom/V8MutationCallbackCustom.cpp

@@ -84 +84 @@
 
     bool callbackReturnValue = false;
-    return !invokeCallback(m_callback, v8::Handle<v8::Object>::Cast(observerHandle), 2, argv, callbackReturnValue, scriptExecutionContext());
+    return !invokeCallback(m_callback.get(), v8::Handle<v8::Object>::Cast(observerHandle), 2, argv, callbackReturnValue, scriptExecutionContext());
 }