binidngs: Make callInternalFunction return MaybeLocal

Source/core/inspector/JavaScriptCallFrame.cpp

 /*
  * Copyright (c) 2010, Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
  * met:
  *
  *     * Redistributions of source code must retain the above copyright
  * notice, this list of conditions and the following disclaimer.
  *     * Redistributions in binary form must reproduce the above
@@ skipping 50 matching lines @@
     }
     return m_caller.get();
 }
 
 int JavaScriptCallFrame::callV8FunctionReturnInt(const char* name) const
 {
     v8::HandleScope handleScope(m_isolate);
     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
 {
     v8::HandleScope handleScope(m_isolate);
     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);
 }
 
 int JavaScriptCallFrame::sourceID() const
 {
     return callV8FunctionReturnInt("sourceID");
 }
 
 int JavaScriptCallFrame::line() const
 {
@@ skipping 22 matching lines @@
 
 int JavaScriptCallFrame::functionColumn() const
 {
     return callV8FunctionReturnInt("functionColumn");
 }
 
 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));
     return result;
 }
 
 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();
 }
 
 v8::Local<v8::Value> JavaScriptCallFrame::thisObject() const
 {
     return m_callFrame.newLocal(m_isolate)->Get(v8AtomicString(m_isolate, "thisObject"));
 }
 
 String JavaScriptCallFrame::stepInPositions() const
 {
@@ skipping 17 matching lines @@
 
 v8::Local<v8::Value> JavaScriptCallFrame::evaluateWithExceptionDetails(v8::Local<v8::Value> expression, v8::Local<v8::Value> scopeExtension)
 {
     v8::Local<v8::Object> callFrame = m_callFrame.newLocal(m_isolate);
     v8::Local<v8::Function> evalFunction = v8::Local<v8::Function>::Cast(callFrame->Get(v8AtomicString(m_isolate, "evaluate")));
     v8::Local<v8::Value> argv[] = {
         expression,
         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()) {
+    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()));
-    } else {
-        wrappedResult->Set(v8::String::NewFromUtf8(m_isolate, "result"), result);
-        wrappedResult->Set(v8::String::NewFromUtf8(m_isolate, "exceptionDetails"), v8::Undefined(m_isolate));
     }
     return wrappedResult;
 }
 
 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;
 }
 
 v8::Local<v8::Value> JavaScriptCallFrame::setVariableValue(int scopeNumber, v8::Local<v8::Value> variableName, v8::Local<v8::Value> newValue)
 {
     v8::Local<v8::Object> callFrame = m_callFrame.newLocal(m_isolate);
     v8::Local<v8::Function> setVariableValueFunction = v8::Local<v8::Function>::Cast(callFrame->Get(v8AtomicString(m_isolate, "setVariableValue")));
     v8::Local<v8::Value> argv[] = {
         v8::Local<v8::Value>(v8::Integer::New(m_isolate, scopeNumber)),
         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)
 {
     v8::Local<v8::Object> exceptionDetails = v8::Object::New(isolate);
     exceptionDetails->Set(v8::String::NewFromUtf8(isolate, "text"), message->Get());
     exceptionDetails->Set(v8::String::NewFromUtf8(isolate, "url"), message->GetScriptOrigin().ResourceName());
     exceptionDetails->Set(v8::String::NewFromUtf8(isolate, "scriptId"), v8::Integer::New(isolate, message->GetScriptOrigin().ScriptID()->Value()));
     exceptionDetails->Set(v8::String::NewFromUtf8(isolate, "line"), v8::Integer::New(isolate, message->GetLineNumber()));
     exceptionDetails->Set(v8::String::NewFromUtf8(isolate, "column"), v8::Integer::New(isolate, message->GetStartColumn()));
     if (!message->GetStackTrace().IsEmpty())
         exceptionDetails->Set(v8::String::NewFromUtf8(isolate, "stackTrace"), message->GetStackTrace()->AsArray());
     else
         exceptionDetails->Set(v8::String::NewFromUtf8(isolate, "stackTrace"), v8::Undefined(isolate));
     return exceptionDetails;
 }
 
 DEFINE_TRACE(JavaScriptCallFrame)
 {
     visitor->trace(m_caller);
 }
 
 } // namespace blink