[V8] setDOMException() should return v8::Handle<v8::Value>()

https://bugs.webkit.org/show_bug.cgi?id=87083

Reviewed by Adam Barth.

The following patterns are used here and there in V8 bindings:

    setDOMException();
    return v8::Handle<v8::Value>();

and

    setDOMException();
    return v8::Undefined();

By returning v8::Handle<v8::Value>() from setDOMException(), we can simplify the above patterns into this:

    return setDOMException();

This patch just replaces the code in CodeGeneratorV8.pm. I'll replace
all other custom bindings in a follow-up patch.

No tests. No change in behavior.

* bindings/scripts/CodeGeneratorV8.pm:
(GenerateSetDOMException):
(GenerateFunctionCallback):
(GenerateFunctionCallString):
* bindings/scripts/test/V8/V8TestEventTarget.cpp:
(WebCore::TestEventTargetV8Internal::itemCallback):
(WebCore::TestEventTargetV8Internal::dispatchEventCallback):
* bindings/scripts/test/V8/V8TestInterface.cpp:
(WebCore::TestInterfaceV8Internal::supplementalMethod2Callback):
* bindings/scripts/test/V8/V8TestObj.cpp:
(WebCore::TestObjV8Internal::attrWithGetterExceptionAttrGetter):
(WebCore::TestObjV8Internal::stringAttrWithGetterExceptionAttrGetter):
(WebCore::TestObjV8Internal::withScriptStateAttributeRaisesAttrGetter):
(WebCore::TestObjV8Internal::withScriptExecutionContextAttributeRaisesAttrGetter):
(WebCore::TestObjV8Internal::withScriptExecutionContextAndScriptStateAttributeRaisesAttrGetter):
(WebCore::TestObjV8Internal::methodThatRequiresAllArgsAndThrowsCallback):
(WebCore::TestObjV8Internal::methodWithExceptionCallback):
(WebCore::TestObjV8Internal::withScriptStateVoidExceptionCallback):
(WebCore::TestObjV8Internal::withScriptStateObjExceptionCallback):
(WebCore::TestObjV8Internal::withScriptExecutionContextAndScriptStateObjExceptionCallback):
(WebCore::TestObjV8Internal::getSVGDocumentCallback):
(WebCore::TestObjV8Internal::strictFunctionCallback):
* bindings/v8/V8Proxy.cpp:
(WebCore::V8Proxy::setDOMException):
* bindings/v8/V8Proxy.h:
(V8Proxy):

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

diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index f8d0110..f484b34 100644 (file)
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,56 @@
+2012-05-22  Kentaro Hara  <haraken@chromium.org>
+
+        [V8] setDOMException() should return v8::Handle<v8::Value>()
+        https://bugs.webkit.org/show_bug.cgi?id=87083
+
+        Reviewed by Adam Barth.
+
+        The following patterns are used here and there in V8 bindings:
+
+            setDOMException();
+            return v8::Handle<v8::Value>();
+
+        and
+
+            setDOMException();
+            return v8::Undefined();
+
+        By returning v8::Handle<v8::Value>() from setDOMException(), we can simplify the above patterns into this:
+
+            return setDOMException();
+
+        This patch just replaces the code in CodeGeneratorV8.pm. I'll replace
+        all other custom bindings in a follow-up patch.
+
+        No tests. No change in behavior.
+
+        * bindings/scripts/CodeGeneratorV8.pm:
+        (GenerateSetDOMException):
+        (GenerateFunctionCallback):
+        (GenerateFunctionCallString):
+        * bindings/scripts/test/V8/V8TestEventTarget.cpp:
+        (WebCore::TestEventTargetV8Internal::itemCallback):
+        (WebCore::TestEventTargetV8Internal::dispatchEventCallback):
+        * bindings/scripts/test/V8/V8TestInterface.cpp:
+        (WebCore::TestInterfaceV8Internal::supplementalMethod2Callback):
+        * bindings/scripts/test/V8/V8TestObj.cpp:
+        (WebCore::TestObjV8Internal::attrWithGetterExceptionAttrGetter):
+        (WebCore::TestObjV8Internal::stringAttrWithGetterExceptionAttrGetter):
+        (WebCore::TestObjV8Internal::withScriptStateAttributeRaisesAttrGetter):
+        (WebCore::TestObjV8Internal::withScriptExecutionContextAttributeRaisesAttrGetter):
+        (WebCore::TestObjV8Internal::withScriptExecutionContextAndScriptStateAttributeRaisesAttrGetter):
+        (WebCore::TestObjV8Internal::methodThatRequiresAllArgsAndThrowsCallback):
+        (WebCore::TestObjV8Internal::methodWithExceptionCallback):
+        (WebCore::TestObjV8Internal::withScriptStateVoidExceptionCallback):
+        (WebCore::TestObjV8Internal::withScriptStateObjExceptionCallback):
+        (WebCore::TestObjV8Internal::withScriptExecutionContextAndScriptStateObjExceptionCallback):
+        (WebCore::TestObjV8Internal::getSVGDocumentCallback):
+        (WebCore::TestObjV8Internal::strictFunctionCallback):
+        * bindings/v8/V8Proxy.cpp:
+        (WebCore::V8Proxy::setDOMException):
+        * bindings/v8/V8Proxy.h:
+        (V8Proxy):
+
 2012-05-22  MORITA Hajime  <morrita@google.com>
 
         Unreviewed expectations update against r117989.

diff --git a/Source/WebCore/bindings/scripts/CodeGeneratorV8.pm b/Source/WebCore/bindings/scripts/CodeGeneratorV8.pm
index d31b043..a97a434 100644 (file)
--- a/Source/WebCore/bindings/scripts/CodeGeneratorV8.pm
+++ b/Source/WebCore/bindings/scripts/CodeGeneratorV8.pm
@@ -650,11 +650,8 @@ sub GenerateSetDOMException
     my $getIsolate = shift;
     my $result = "";
 
-    $result .= $indent . "if (UNLIKELY(ec)) {\n";
-    $result .= $indent . "    V8Proxy::setDOMException(ec, $getIsolate);\n";
-    $result .= $indent . "    return v8::Handle<v8::Value>();\n";
-    $result .= $indent . "}\n";
-
+    $result .= $indent . "if (UNLIKELY(ec))\n";
+    $result .= $indent . "    return V8Proxy::setDOMException(ec, $getIsolate);\n";
     return $result;
 }
 
@@ -1408,10 +1405,8 @@ END
         } else {
             AddToImplIncludes("ExceptionCode.h");
             push(@implContentDecls, "    $nativeClassName wrapper = V8${implClassName}::toNative(args.Holder());\n");
-            push(@implContentDecls, "    if (wrapper->role() == AnimValRole) {\n");
-            push(@implContentDecls, "        V8Proxy::setDOMException(NO_MODIFICATION_ALLOWED_ERR, args.GetIsolate());\n");
-            push(@implContentDecls, "        return v8::Handle<v8::Value>();\n");
-            push(@implContentDecls, "    }\n");
+            push(@implContentDecls, "    if (wrapper->role() == AnimValRole)\n");
+            push(@implContentDecls, "        return V8Proxy::setDOMException(NO_MODIFICATION_ALLOWED_ERR, args.GetIsolate());\n");
             my $svgWrappedNativeType = $codeGenerator->GetSVGWrappedTypeNeedingTearOff($implClassName);
             push(@implContentDecls, "    $svgWrappedNativeType& impInstance = wrapper->propertyReference();\n");
             push(@implContentDecls, "    $svgWrappedNativeType* imp = &impInstance;\n");
@@ -1466,8 +1461,7 @@ END
     if ($raisesExceptions) {
         push(@implContentDecls, "    }\n");
         push(@implContentDecls, "    fail:\n");
-        push(@implContentDecls, "    V8Proxy::setDOMException(ec, args.GetIsolate());\n");
-        push(@implContentDecls, "    return v8::Handle<v8::Value>();\n");
+        push(@implContentDecls, "    return V8Proxy::setDOMException(ec, args.GetIsolate());\n");
     }
 
     push(@implContentDecls, "}\n\n");
@@ -3333,10 +3327,8 @@ sub GenerateFunctionCallString()
             push @arguments, "$paramName.get()";
         } elsif ($codeGenerator->IsSVGTypeNeedingTearOff($parameter->type) and not $implClassName =~ /List$/) {
             push @arguments, "$paramName->propertyReference()";
-            $result .= $indent . "if (!$paramName) {\n";
-            $result .= $indent . "    V8Proxy::setDOMException(WebCore::TYPE_MISMATCH_ERR, args.GetIsolate());\n";
-            $result .= $indent . "    return v8::Handle<v8::Value>();\n";
-            $result .= $indent . "}\n";
+            $result .= $indent . "if (!$paramName)\n";
+            $result .= $indent . "    return V8Proxy::setDOMException(WebCore::TYPE_MISMATCH_ERR, args.GetIsolate());\n";
         } elsif ($parameter->type eq "SVGMatrix" and $implClassName eq "SVGTransformList") {
             push @arguments, "$paramName.get()";
         } else {

diff --git a/Source/WebCore/bindings/scripts/test/V8/V8TestEventTarget.cpp b/Source/WebCore/bindings/scripts/test/V8/V8TestEventTarget.cpp
index b36b125..4a15127 100644 (file)
--- a/Source/WebCore/bindings/scripts/test/V8/V8TestEventTarget.cpp
+++ b/Source/WebCore/bindings/scripts/test/V8/V8TestEventTarget.cpp
@@ -59,8 +59,7 @@ static v8::Handle<v8::Value> itemCallback(const v8::Arguments& args)
     return toV8(imp->item(index), args.GetIsolate());
     }
     fail:
-    V8Proxy::setDOMException(ec, args.GetIsolate());
-    return v8::Handle<v8::Value>();
+    return V8Proxy::setDOMException(ec, args.GetIsolate());
 }
 
 static v8::Handle<v8::Value> addEventListenerCallback(const v8::Arguments& args)
@@ -100,8 +99,7 @@ static v8::Handle<v8::Value> dispatchEventCallback(const v8::Arguments& args)
     return v8Boolean(result);
     }
     fail:
-    V8Proxy::setDOMException(ec, args.GetIsolate());
-    return v8::Handle<v8::Value>();
+    return V8Proxy::setDOMException(ec, args.GetIsolate());
 }
 
 } // namespace TestEventTargetV8Internal

diff --git a/Source/WebCore/bindings/scripts/test/V8/V8TestInterface.cpp b/Source/WebCore/bindings/scripts/test/V8/V8TestInterface.cpp
index acbded4..0575885 100644 (file)
--- a/Source/WebCore/bindings/scripts/test/V8/V8TestInterface.cpp
+++ b/Source/WebCore/bindings/scripts/test/V8/V8TestInterface.cpp
@@ -140,8 +140,7 @@ static v8::Handle<v8::Value> supplementalMethod2Callback(const v8::Arguments& ar
     return toV8(result.release(), args.GetIsolate());
     }
     fail:
-    V8Proxy::setDOMException(ec, args.GetIsolate());
-    return v8::Handle<v8::Value>();
+    return V8Proxy::setDOMException(ec, args.GetIsolate());
 }
 
 #endif // ENABLE(Condition11) || ENABLE(Condition12)

diff --git a/Source/WebCore/bindings/scripts/test/V8/V8TestObj.cpp b/Source/WebCore/bindings/scripts/test/V8/V8TestObj.cpp
index 829759f..509044f 100644 (file)
--- a/Source/WebCore/bindings/scripts/test/V8/V8TestObj.cpp
+++ b/Source/WebCore/bindings/scripts/test/V8/V8TestObj.cpp
@@ -626,10 +626,8 @@ static v8::Handle<v8::Value> attrWithGetterExceptionAttrGetter(v8::Local<v8::Str
     TestObj* imp = V8TestObj::toNative(info.Holder());
     ExceptionCode ec = 0;
     int v = imp->attrWithGetterException(ec);
-    if (UNLIKELY(ec)) {
-        V8Proxy::setDOMException(ec, info.GetIsolate());
-        return v8::Handle<v8::Value>();
-    }
+    if (UNLIKELY(ec))
+        return V8Proxy::setDOMException(ec, info.GetIsolate());
     return v8::Integer::New(v);
 }
 
@@ -670,10 +668,8 @@ static v8::Handle<v8::Value> stringAttrWithGetterExceptionAttrGetter(v8::Local<v
     TestObj* imp = V8TestObj::toNative(info.Holder());
     ExceptionCode ec = 0;
     String v = imp->stringAttrWithGetterException(ec);
-    if (UNLIKELY(ec)) {
-        V8Proxy::setDOMException(ec, info.GetIsolate());
-        return v8::Handle<v8::Value>();
-    }
+    if (UNLIKELY(ec))
+        return V8Proxy::setDOMException(ec, info.GetIsolate());
     return v8String(v, info.GetIsolate());
 }
 
@@ -763,10 +759,8 @@ static v8::Handle<v8::Value> withScriptStateAttributeRaisesAttrGetter(v8::Local<
     if (!state)
         return v8::Undefined();
     RefPtr<TestObj> v = imp->withScriptStateAttributeRaises(state, ec);
-    if (UNLIKELY(ec)) {
-        V8Proxy::setDOMException(ec, info.GetIsolate());
-        return v8::Handle<v8::Value>();
-    }
+    if (UNLIKELY(ec))
+        return V8Proxy::setDOMException(ec, info.GetIsolate());
     if (state.hadException())
         return throwError(state.exception(), info.GetIsolate());
     return toV8(v.release(), info.GetIsolate());
@@ -798,10 +792,8 @@ static v8::Handle<v8::Value> withScriptExecutionContextAttributeRaisesAttrGetter
     if (!scriptContext)
         return v8::Undefined();
     RefPtr<TestObj> v = imp->withScriptExecutionContextAttributeRaises(scriptContext, ec);
-    if (UNLIKELY(ec)) {
-        V8Proxy::setDOMException(ec, info.GetIsolate());
-        return v8::Handle<v8::Value>();
-    }
+    if (UNLIKELY(ec))
+        return V8Proxy::setDOMException(ec, info.GetIsolate());
     return toV8(v.release(), info.GetIsolate());
 }
 
@@ -862,10 +854,8 @@ static v8::Handle<v8::Value> withScriptExecutionContextAndScriptStateAttributeRa
     if (!scriptContext)
         return v8::Undefined();
     RefPtr<TestObj> v = imp->withScriptExecutionContextAndScriptStateAttributeRaises(state, scriptContext, ec);
-    if (UNLIKELY(ec)) {
-        V8Proxy::setDOMException(ec, info.GetIsolate());
-        return v8::Handle<v8::Value>();
-    }
+    if (UNLIKELY(ec))
+        return V8Proxy::setDOMException(ec, info.GetIsolate());
     if (state.hadException())
         return throwError(state.exception(), info.GetIsolate());
     return toV8(v.release(), info.GetIsolate());
@@ -1347,8 +1337,7 @@ static v8::Handle<v8::Value> methodThatRequiresAllArgsAndThrowsCallback(const v8
     return toV8(result.release(), args.GetIsolate());
     }
     fail:
-    V8Proxy::setDOMException(ec, args.GetIsolate());
-    return v8::Handle<v8::Value>();
+    return V8Proxy::setDOMException(ec, args.GetIsolate());
 }
 
 static v8::Handle<v8::Value> serializedValueCallback(const v8::Arguments& args)
@@ -1410,8 +1399,7 @@ static v8::Handle<v8::Value> methodWithExceptionCallback(const v8::Arguments& ar
     return v8::Handle<v8::Value>();
     }
     fail:
-    V8Proxy::setDOMException(ec, args.GetIsolate());
-    return v8::Handle<v8::Value>();
+    return V8Proxy::setDOMException(ec, args.GetIsolate());
 }
 
 static v8::Handle<v8::Value> addEventListenerCallback(const v8::Arguments& args)
@@ -1473,8 +1461,7 @@ static v8::Handle<v8::Value> withScriptStateVoidExceptionCallback(const v8::Argu
     return v8::Handle<v8::Value>();
     }
     fail:
-    V8Proxy::setDOMException(ec, args.GetIsolate());
-    return v8::Handle<v8::Value>();
+    return V8Proxy::setDOMException(ec, args.GetIsolate());
 }
 
 static v8::Handle<v8::Value> withScriptStateObjExceptionCallback(const v8::Arguments& args)
@@ -1492,8 +1479,7 @@ static v8::Handle<v8::Value> withScriptStateObjExceptionCallback(const v8::Argum
     return toV8(result.release(), args.GetIsolate());
     }
     fail:
-    V8Proxy::setDOMException(ec, args.GetIsolate());
-    return v8::Handle<v8::Value>();
+    return V8Proxy::setDOMException(ec, args.GetIsolate());
 }
 
 static v8::Handle<v8::Value> withScriptExecutionContextCallback(const v8::Arguments& args)
@@ -1539,8 +1525,7 @@ static v8::Handle<v8::Value> withScriptExecutionContextAndScriptStateObjExceptio
     return toV8(result.release(), args.GetIsolate());
     }
     fail:
-    V8Proxy::setDOMException(ec, args.GetIsolate());
-    return v8::Handle<v8::Value>();
+    return V8Proxy::setDOMException(ec, args.GetIsolate());
 }
 
 static v8::Handle<v8::Value> withScriptExecutionContextAndScriptStateWithSpacesCallback(const v8::Arguments& args)
@@ -1928,8 +1913,7 @@ static v8::Handle<v8::Value> getSVGDocumentCallback(const v8::Arguments& args)
     return toV8(result.release(), args.GetIsolate());
     }
     fail:
-    V8Proxy::setDOMException(ec, args.GetIsolate());
-    return v8::Handle<v8::Value>();
+    return V8Proxy::setDOMException(ec, args.GetIsolate());
 }
 
 static v8::Handle<v8::Value> convert1Callback(const v8::Arguments& args)
@@ -2026,8 +2010,7 @@ static v8::Handle<v8::Value> strictFunctionCallback(const v8::Arguments& args)
     return toV8(result.release(), args.GetIsolate());
     }
     fail:
-    V8Proxy::setDOMException(ec, args.GetIsolate());
-    return v8::Handle<v8::Value>();
+    return V8Proxy::setDOMException(ec, args.GetIsolate());
 }
 
 } // namespace TestObjV8Internal

diff --git a/Source/WebCore/bindings/v8/V8Proxy.cpp b/Source/WebCore/bindings/v8/V8Proxy.cpp
index d12151f..67d528f 100644 (file)
--- a/Source/WebCore/bindings/v8/V8Proxy.cpp
+++ b/Source/WebCore/bindings/v8/V8Proxy.cpp
@@ -586,10 +586,10 @@ static void DOMExceptionStackSetter(v8::Local<v8::String> name, v8::Local<v8::Va
         exception = toV8(interfaceName::create(description), isolate); \
         break;
 
-void V8Proxy::setDOMException(int ec, v8::Isolate* isolate)
+v8::Handle<v8::Value> V8Proxy::setDOMException(int ec, v8::Isolate* isolate)
 {
     if (ec <= 0)
-        return;
+        return v8::Handle<v8::Value>();
 
     ExceptionCodeDescription description(ec);
 
@@ -599,7 +599,7 @@ void V8Proxy::setDOMException(int ec, v8::Isolate* isolate)
     }
 
     if (exception.IsEmpty())
-        return;
+        return v8::Handle<v8::Value>();
 
     // Attach an Error object to the DOMException. This is then lazily used to get the stack value.
     v8::Handle<v8::Value> error = v8::Exception::Error(v8String(description.description, isolate));
@@ -607,7 +607,7 @@ void V8Proxy::setDOMException(int ec, v8::Isolate* isolate)
     ASSERT(exception->IsObject());
     exception->ToObject()->SetAccessor(v8String("stack", isolate), DOMExceptionStackGetter, DOMExceptionStackSetter, error);
 
-    v8::ThrowException(exception);
+    return v8::ThrowException(exception);
 }
 
 #undef TRY_TO_CREATE_EXCEPTION

diff --git a/Source/WebCore/bindings/v8/V8Proxy.h b/Source/WebCore/bindings/v8/V8Proxy.h
index 9751979..1718bc8 100644 (file)
--- a/Source/WebCore/bindings/v8/V8Proxy.h
+++ b/Source/WebCore/bindings/v8/V8Proxy.h
@@ -234,7 +234,7 @@ namespace WebCore {
 
         // If the exception code is different from zero, a DOM exception is
         // schedule to be thrown.
-        static void setDOMException(int exceptionCode, v8::Isolate*);
+        static v8::Handle<v8::Value> setDOMException(int exceptionCode, v8::Isolate*);
 
         // Schedule an error object to be thrown.
         static v8::Handle<v8::Value> throwError(ErrorType, const char* message, v8::Isolate* = 0);