binidngs: Make callInternalFunction return MaybeLocal

Source/core/inspector/JavaScriptCallFrame.cpp

Index: Source/core/inspector/JavaScriptCallFrame.cpp
diff --git a/Source/core/inspector/JavaScriptCallFrame.cpp b/Source/core/inspector/JavaScriptCallFrame.cpp
index f9c9c1a7390a2ca47a9df816db953e35d57834b8..d9e6c51574ac1ba9b662ae1645a4c0d1fb79ef99 100644
--- a/Source/core/inspector/JavaScriptCallFrame.cpp
+++ b/Source/core/inspector/JavaScriptCallFrame.cpp
@@ -68,10 +68,10 @@ int JavaScriptCallFrame::callV8FunctionReturnInt(const char* name) const
     v8::Context::Scope contextScope(m_debuggerContext.newLocal(m_isolate));
     v8::Local<v8::Object> callFrame = m_callFrame.newLocal(m_isolate);
     v8::Local<v8::Function> func = v8::Local<v8::Function>::Cast(callFrame->Get(v8AtomicString(m_isolate, name)));
-    v8::Local<v8::Value> result = V8ScriptRunner::callInternalFunction(func, callFrame, 0, 0, m_isolate);
-    if (result.IsEmpty() || !result->IsInt32())
+    v8::Local<v8::Value> result;
+    if (!V8ScriptRunner::callInternalFunction(func, callFrame, 0, 0, m_isolate).ToLocal(&result) || !result->IsInt32())
         return 0;
-    return result->Int32Value();
+    return result.As<v8::Int32>()->Value();
 }
 
 String JavaScriptCallFrame::callV8FunctionReturnString(const char* name) const
@@ -80,7 +80,9 @@ String JavaScriptCallFrame::callV8FunctionReturnString(const char* name) const
     v8::Context::Scope contextScope(m_debuggerContext.newLocal(m_isolate));
     v8::Local<v8::Object> callFrame = m_callFrame.newLocal(m_isolate);
     v8::Local<v8::Function> func = v8::Local<v8::Function>::Cast(callFrame->Get(v8AtomicString(m_isolate, name)));
-    v8::Local<v8::Value> result = V8ScriptRunner::callInternalFunction(func, callFrame, 0, 0, m_isolate);
+    v8::Local<v8::Value> result;
+    if (!V8ScriptRunner::callInternalFunction(func, callFrame, 0, 0, m_isolate).ToLocal(&result))
+        return String();
     return toCoreStringWithUndefinedOrNullCheck(result);
 }
 
@@ -123,7 +125,7 @@ v8::Local<v8::Value> JavaScriptCallFrame::scopeChain() const
 {
     v8::Local<v8::Object> callFrame = m_callFrame.newLocal(m_isolate);
     v8::Local<v8::Function> func = v8::Local<v8::Function>::Cast(callFrame->Get(v8AtomicString(m_isolate, "scopeChain")));
-    v8::Local<v8::Array> scopeChain = v8::Local<v8::Array>::Cast(V8ScriptRunner::callInternalFunction(func, callFrame, 0, 0, m_isolate));
+    v8::Local<v8::Array> scopeChain = v8::Local<v8::Array>::Cast(V8ScriptRunner::callInternalFunction(func, callFrame, 0, 0, m_isolate).ToLocalChecked());
     v8::Local<v8::Array> result = v8::Array::New(m_isolate, scopeChain->Length());
     for (uint32_t i = 0; i < scopeChain->Length(); i++)
         result->Set(i, scopeChain->Get(i));
@@ -134,7 +136,7 @@ int JavaScriptCallFrame::scopeType(int scopeIndex) const
 {
     v8::Local<v8::Object> callFrame = m_callFrame.newLocal(m_isolate);
     v8::Local<v8::Function> func = v8::Local<v8::Function>::Cast(callFrame->Get(v8AtomicString(m_isolate, "scopeType")));
-    v8::Local<v8::Array> scopeType = v8::Local<v8::Array>::Cast(V8ScriptRunner::callInternalFunction(func, callFrame, 0, 0, m_isolate));
+    v8::Local<v8::Array> scopeType = v8::Local<v8::Array>::Cast(V8ScriptRunner::callInternalFunction(func, callFrame, 0, 0, m_isolate).ToLocalChecked());
     return scopeType->Get(scopeIndex)->Int32Value();
 }
 
@@ -172,15 +174,14 @@ v8::Local<v8::Value> JavaScriptCallFrame::evaluateWithExceptionDetails(v8::Local
         scopeExtension
     };
     v8::TryCatch tryCatch;
-    v8::Local<v8::Value> result = V8ScriptRunner::callInternalFunction(evalFunction, callFrame, WTF_ARRAY_LENGTH(argv), argv, m_isolate);
-
     v8::Local<v8::Object> wrappedResult = v8::Object::New(m_isolate);
-    if (tryCatch.HasCaught()) {
-        wrappedResult->Set(v8::String::NewFromUtf8(m_isolate, "result"), tryCatch.Exception());
-        wrappedResult->Set(v8::String::NewFromUtf8(m_isolate, "exceptionDetails"), createExceptionDetails(m_isolate, tryCatch.Message()));
-    } else {
+    v8::Local<v8::Value> result;
+    if (V8ScriptRunner::callInternalFunction(evalFunction, callFrame, WTF_ARRAY_LENGTH(argv), argv, m_isolate).ToLocal(&result)) {
         wrappedResult->Set(v8::String::NewFromUtf8(m_isolate, "result"), result);
         wrappedResult->Set(v8::String::NewFromUtf8(m_isolate, "exceptionDetails"), v8::Undefined(m_isolate));
+    } else {
+        wrappedResult->Set(v8::String::NewFromUtf8(m_isolate, "result"), tryCatch.Exception());
+        wrappedResult->Set(v8::String::NewFromUtf8(m_isolate, "exceptionDetails"), createExceptionDetails(m_isolate, tryCatch.Message()));
     }
     return wrappedResult;
 }
@@ -190,7 +191,7 @@ v8::Local<v8::Value> JavaScriptCallFrame::restart()
     v8::Local<v8::Object> callFrame = m_callFrame.newLocal(m_isolate);
     v8::Local<v8::Function> restartFunction = v8::Local<v8::Function>::Cast(callFrame->Get(v8AtomicString(m_isolate, "restart")));
     v8::Debug::SetLiveEditEnabled(m_isolate, true);
-    v8::Local<v8::Value> result = V8ScriptRunner::callInternalFunction(restartFunction, callFrame, 0, 0, m_isolate);
+    v8::Local<v8::Value> result = V8ScriptRunner::callInternalFunction(restartFunction, callFrame, 0, 0, m_isolate).ToLocalChecked();
     v8::Debug::SetLiveEditEnabled(m_isolate, false);
     return result;
 }
@@ -204,7 +205,10 @@ v8::Local<v8::Value> JavaScriptCallFrame::setVariableValue(int scopeNumber, v8::
         variableName,
         newValue
     };
-    return V8ScriptRunner::callInternalFunction(setVariableValueFunction, callFrame, WTF_ARRAY_LENGTH(argv), argv, m_isolate);
+    v8::Local<v8::Value> result;
+    bool success = V8ScriptRunner::callInternalFunction(setVariableValueFunction, callFrame, WTF_ARRAY_LENGTH(argv), argv, m_isolate).ToLocal(&result);
+    ALLOW_UNUSED_LOCAL(success);

[yurys, 2015/03/31 07:20:20]

This is wrong assumption as setVariableValue may throw and the caller should
deal with the exception. The method is called from
V8JavaScriptCallFrame::setVariableValueMethodCustom which simply propagates
returned value to the caller.

Since we don't want to deal with the exception here I'd probably recommend
changing return type to MaybeLocal but this would in turn require either dealing
explicitly with the exception in
V8JavaScriptCallFrame::setVariableValueMethodCustom or adding overload for
v8SetReturnValue that would accept MaybeLocal and be no-op (alternatively we
could change v8::FunctionCallbackInfo so that it allowed to specify explicitly
that the result is an exception). WDYT?

[bashi, 2015/04/01 01:51:27]

You said an empty handle is valid here, and this is exactly what the previous
code does. That said, I already added v8SetReturnValue which takes MaybeLocal,
so just changing the return type should work. Done.

+    return result;
 }
 
 v8::Local<v8::Object> JavaScriptCallFrame::createExceptionDetails(v8::Isolate* isolate, v8::Local<v8::Message> message)