bindings: Use Maybe APIs in V8ScriptRunner (part 1)

Source/bindings/core/v8/V8ScriptRunner.cpp

 /*
  * Copyright (C) 2009 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:
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2. Redistributions in binary form must reproduce the above copyright
  *    notice, this list of conditions and the following disclaimer in the
@@ skipping 62 matching lines @@
 }
 
 V8CompileHistogram::~V8CompileHistogram()
 {
     int64_t elapsedMicroSeconds = static_cast<int64_t>((WTF::currentTime() - m_timeStamp) * 1000000);
     const char* name = (m_cacheability == Cacheable) ? "V8.CompileCacheableMicroSeconds" : "V8.CompileNoncacheableMicroSeconds";
     blink::Platform::current()->histogramCustomCounts(name, elapsedMicroSeconds, 0, 1000000, 50);
 }
 
 // In order to make sure all pending messages to be processed in
-// v8::Function::Call, we don't call handleMaxRecursionDepthExceeded
+// v8::Function::Call, we don't call throwStackOverflowException
 // directly. Instead, we create a v8::Function of
 // throwStackOverflowException and call it.

[haraken, 2015/03/18 01:58:51]

Not related to your CL, this comment doesn't make much sense... here we're not
creating a v8::Function nor calling it...

[bashi, 2015/03/18 02:33:12]

I guess this comment explains what throwStackOverflowExceptionIfNeeded() does.

 void throwStackOverflowException(const v8::FunctionCallbackInfo<v8::Value>& info)
 {
     V8ThrowException::throwRangeError(info.GetIsolate(), "Maximum call stack size exceeded.");
 }
 
+void throwScriptForbiddenException(v8::Isolate* isolate)
+{
+    V8ThrowException::throwGeneralError(isolate, "Script execution is forbidden.");
+}
+
 v8::Local<v8::Value> throwStackOverflowExceptionIfNeeded(v8::Isolate* isolate)
 {
     if (V8PerIsolateData::from(isolate)->isHandlingRecursionLevelError()) {
         // If we are already handling a recursion level error, we should
         // not invoke v8::Function::Call.
         return v8::Undefined(isolate);
     }
     V8PerIsolateData::from(isolate)->setIsHandlingRecursionLevelError(true);
     v8::Local<v8::Value> result = v8::Function::New(isolate, throwStackOverflowException)->Call(v8::Undefined(isolate), 0, 0);
     V8PerIsolateData::from(isolate)->setIsHandlingRecursionLevelError(false);
     return result;
 }
 
 // Compile a script without any caching or compile options.
-v8::Local<v8::Script> compileWithoutOptions(V8CompileHistogram::Cacheability cacheability, v8::Isolate* isolate, v8::Handle<v8::String> code, v8::ScriptOrigin origin)
+v8::MaybeLocal<v8::Script> compileWithoutOptions(V8CompileHistogram::Cacheability cacheability, v8::Isolate* isolate, v8::Handle<v8::String> code, v8::ScriptOrigin origin)
 {
     V8CompileHistogram histogramScope(cacheability);
     v8::ScriptCompiler::Source source(code, origin);
-    return v8::ScriptCompiler::Compile(isolate, &source, v8::ScriptCompiler::kNoCompileOptions);
+    return v8::ScriptCompiler::Compile(isolate->GetCurrentContext(), &source, v8::ScriptCompiler::kNoCompileOptions);
 }
 
 // Compile a script, and consume a V8 cache that was generated previously.
-v8::Local<v8::Script> compileAndConsumeCache(CachedMetadataHandler* cacheHandler, unsigned tag, v8::ScriptCompiler::CompileOptions compileOptions, bool compressed, v8::Isolate* isolate, v8::Handle<v8::String> code, v8::ScriptOrigin origin)
+v8::MaybeLocal<v8::Script> compileAndConsumeCache(CachedMetadataHandler* cacheHandler, unsigned tag, v8::ScriptCompiler::CompileOptions compileOptions, bool compressed, v8::Isolate* isolate, v8::Handle<v8::String> code, v8::ScriptOrigin origin)
 {
     V8CompileHistogram histogramScope(V8CompileHistogram::Cacheable);
     CachedMetadata* cachedMetadata = cacheHandler->cachedMetadata(tag);
     const char* data = cachedMetadata->data();
     int length = cachedMetadata->size();
     std::string uncompressedOutput;
     bool invalidCache = false;
     if (compressed) {
         if (snappy::Uncompress(data, length, &uncompressedOutput)) {
             data = uncompressedOutput.data();
             length = uncompressedOutput.length();
         } else {
             invalidCache = true;
         }
     }
-    v8::Local<v8::Script> script;
+    v8::MaybeLocal<v8::Script> script;
     if (invalidCache) {
         v8::ScriptCompiler::Source source(code, origin);
-        script = v8::ScriptCompiler::Compile(isolate, &source, v8::ScriptCompiler::kNoCompileOptions);
+        script = v8::ScriptCompiler::Compile(isolate->GetCurrentContext(), &source, v8::ScriptCompiler::kNoCompileOptions);
     } else {
         v8::ScriptCompiler::CachedData* cachedData = new v8::ScriptCompiler::CachedData(
             reinterpret_cast<const uint8_t*>(data), length, v8::ScriptCompiler::CachedData::BufferNotOwned);
         v8::ScriptCompiler::Source source(code, origin, cachedData);
-        script = v8::ScriptCompiler::Compile(isolate, &source, compileOptions);
+        script = v8::ScriptCompiler::Compile(isolate->GetCurrentContext(), &source, compileOptions);
         invalidCache = cachedData->rejected;
     }
     if (invalidCache)
         cacheHandler->clearCachedMetadata(CachedMetadataHandler::SendToPlatform);
     return script;
 }
 
 // Compile a script, and produce a V8 cache for future use.
-v8::Local<v8::Script> compileAndProduceCache(CachedMetadataHandler* cacheHandler, unsigned tag, v8::ScriptCompiler::CompileOptions compileOptions, bool compressed, CachedMetadataHandler::CacheType cacheType, v8::Isolate* isolate, v8::Handle<v8::String> code, v8::ScriptOrigin origin)
+v8::MaybeLocal<v8::Script> compileAndProduceCache(CachedMetadataHandler* cacheHandler, unsigned tag, v8::ScriptCompiler::CompileOptions compileOptions, bool compressed, CachedMetadataHandler::CacheType cacheType, v8::Isolate* isolate, v8::Handle<v8::String> code, v8::ScriptOrigin origin)
 {
     V8CompileHistogram histogramScope(V8CompileHistogram::Cacheable);
     v8::ScriptCompiler::Source source(code, origin);
-    v8::Local<v8::Script> script = v8::ScriptCompiler::Compile(isolate, &source, compileOptions);
+    v8::MaybeLocal<v8::Script> script = v8::ScriptCompiler::Compile(isolate->GetCurrentContext(), &source, compileOptions);
     const v8::ScriptCompiler::CachedData* cachedData = source.GetCachedData();
     if (cachedData) {
         const char* data = reinterpret_cast<const char*>(cachedData->data);
         int length = cachedData->length;
         std::string compressedOutput;
         if (compressed) {
             snappy::Compress(data, length, &compressedOutput);
             data = compressedOutput.data();
             length = compressedOutput.length();
         }
         if (length > 1024) {
             // Omit histogram samples for small cache data to avoid outliers.
             int cacheSizeRatio = static_cast<int>(100.0 * length / code->Length());
             blink::Platform::current()->histogramCustomCounts("V8.CodeCacheSizeRatio", cacheSizeRatio, 0, 10000, 50);
         }
         cacheHandler->clearCachedMetadata(CachedMetadataHandler::CacheLocally);
         cacheHandler->setCachedMetadata(tag, data, length, cacheType);
     }
     return script;
 }
 
 // Compile a script, and consume or produce a V8 Cache, depending on whether the
 // given resource already has cached data available.
-v8::Local<v8::Script> compileAndConsumeOrProduce(CachedMetadataHandler* cacheHandler, unsigned tag, v8::ScriptCompiler::CompileOptions consumeOptions, v8::ScriptCompiler::CompileOptions produceOptions, bool compressed, CachedMetadataHandler::CacheType cacheType, v8::Isolate* isolate, v8::Handle<v8::String> code, v8::ScriptOrigin origin)
+v8::MaybeLocal<v8::Script> compileAndConsumeOrProduce(CachedMetadataHandler* cacheHandler, unsigned tag, v8::ScriptCompiler::CompileOptions consumeOptions, v8::ScriptCompiler::CompileOptions produceOptions, bool compressed, CachedMetadataHandler::CacheType cacheType, v8::Isolate* isolate, v8::Handle<v8::String> code, v8::ScriptOrigin origin)
 {
     return cacheHandler->cachedMetadata(tag)
         ? compileAndConsumeCache(cacheHandler, tag, consumeOptions, compressed, isolate, code, origin)
         : compileAndProduceCache(cacheHandler, tag, produceOptions, compressed, cacheType, isolate, code, origin);
 }
 
 enum CacheTagKind {
     CacheTagParser = 0,
     CacheTagCode = 1,
     CacheTagCodeCompressed = 2,
@@ skipping 35 matching lines @@
         return false;
     double timeStamp;
     const int size = sizeof(timeStamp);
     ASSERT(cachedMetadata->size() == size);
     memcpy(&timeStamp, cachedMetadata->data(), size);
     return (WTF::currentTime() - timeStamp) < kCacheWithinSeconds;
 }
 
 // Final compile call for a streamed compilation. Most decisions have already
 // been made, but we need to write back data into the cache.
-v8::Local<v8::Script> postStreamCompile(CachedMetadataHandler* cacheHandler, ScriptStreamer* streamer, v8::Isolate* isolate, v8::Handle<v8::String> code, v8::ScriptOrigin origin)
+v8::MaybeLocal<v8::Script> postStreamCompile(CachedMetadataHandler* cacheHandler, ScriptStreamer* streamer, v8::Isolate* isolate, v8::Handle<v8::String> code, v8::ScriptOrigin origin)
 {
     V8CompileHistogram histogramScope(V8CompileHistogram::Noncacheable);
-    v8::Local<v8::Script> script = v8::ScriptCompiler::Compile(isolate, streamer->source(), code, origin);
+    v8::MaybeLocal<v8::Script> script = v8::ScriptCompiler::Compile(isolate->GetCurrentContext(), streamer->source(), code, origin);
 
     if (!cacheHandler)
         return script;
 
     // Whether to produce the cached data or not is decided when the
     // streamer is started. Here we only need to get the data out.
     const v8::ScriptCompiler::CachedData* newCachedData = streamer->source()->GetCachedData();
     if (newCachedData) {
         cacheHandler->clearCachedMetadata(CachedMetadataHandler::CacheLocally);
         v8::ScriptCompiler::CompileOptions options = streamer->compileOptions();
         switch (options) {
         case v8::ScriptCompiler::kProduceParserCache:
             cacheHandler->setCachedMetadata(cacheTag(CacheTagParser, cacheHandler), reinterpret_cast<const char*>(newCachedData->data), newCachedData->length, CachedMetadataHandler::CacheLocally);
             break;
         case v8::ScriptCompiler::kProduceCodeCache:
             cacheHandler->setCachedMetadata(cacheTag(CacheTagCode, cacheHandler), reinterpret_cast<const char*>(newCachedData->data), newCachedData->length, CachedMetadataHandler::SendToPlatform);
             break;
         default:
             break;
         }
     }
 
     return script;
 }
 
-typedef Function<v8::Local<v8::Script>(v8::Isolate*, v8::Handle<v8::String>, v8::ScriptOrigin)> CompileFn;
+typedef Function<v8::MaybeLocal<v8::Script>(v8::Isolate*, v8::Handle<v8::String>, v8::ScriptOrigin)> CompileFn;
 
 // A notation convenience: WTF::bind<...> needs to be given the right argument
 // types. We have an awful lot of bind calls below, all with the same types, so
 // this local bind lets WTF::bind to all the work, but 'knows' the right
 // parameter types.
 // This version isn't quite as smart as the real WTF::bind, though, so you
 // sometimes may still have to call the original.
 template<typename... A>
 PassOwnPtr<CompileFn> bind(const A&... args)
 {
@@ skipping 83 matching lines @@
     // We don't stream scripts which don't have a Resource.
     ASSERT(resource);
     // Failed resources should never get this far.
     ASSERT(!resource->errorOccurred());
     ASSERT(streamer->isFinished());
     ASSERT(!streamer->streamingSuppressed());
     return WTF::bind<v8::Isolate*, v8::Handle<v8::String>, v8::ScriptOrigin>(postStreamCompile, resource->cacheHandler(), streamer);
 }
 } // namespace
 
-v8::Local<v8::Script> V8ScriptRunner::compileScript(const ScriptSourceCode& source, v8::Isolate* isolate, AccessControlStatus corsStatus, V8CacheOptions cacheOptions)
+v8::MaybeLocal<v8::Script> V8ScriptRunner::compileScript(const ScriptSourceCode& source, v8::Isolate* isolate, AccessControlStatus corsStatus, V8CacheOptions cacheOptions)
 {
     return compileScript(v8String(isolate, source.source()), source.url(), source.sourceMapUrl(), source.startPosition(), isolate, source.resource(), source.streamer(), source.resource() ? source.resource()->cacheHandler() : nullptr, corsStatus, cacheOptions);
 }
 
-v8::Local<v8::Script> V8ScriptRunner::compileScript(v8::Handle<v8::String> code, const String& fileName, const String& sourceMapUrl, const TextPosition& scriptStartPosition, v8::Isolate* isolate, ScriptResource* resource, ScriptStreamer* streamer, CachedMetadataHandler* cacheHandler, AccessControlStatus corsStatus, V8CacheOptions cacheOptions, bool isInternalScript)
+v8::MaybeLocal<v8::Script> V8ScriptRunner::compileScript(v8::Handle<v8::String> code, const String& fileName, const String& sourceMapUrl, const TextPosition& scriptStartPosition, v8::Isolate* isolate, ScriptResource* resource, ScriptStreamer* streamer, CachedMetadataHandler* cacheHandler, AccessControlStatus corsStatus, V8CacheOptions cacheOptions, bool isInternalScript)
 {
     TRACE_EVENT1("v8", "v8.compile", "fileName", fileName.utf8());
     TRACE_EVENT_SCOPED_SAMPLING_STATE("v8", "V8Compile");
 
     ASSERT(!streamer || resource);
     ASSERT(!resource || resource->cacheHandler() == cacheHandler);
 
     // NOTE: For compatibility with WebCore, ScriptSourceCode's line starts at
     // 1, whereas v8 starts at 0.
     v8::ScriptOrigin origin(
         v8String(isolate, fileName),
         v8::Integer::New(isolate, scriptStartPosition.m_line.zeroBasedInt()),
         v8::Integer::New(isolate, scriptStartPosition.m_column.zeroBasedInt()),
         v8Boolean(corsStatus == SharableCrossOrigin, isolate),
         v8::Handle<v8::Integer>(),
         v8Boolean(isInternalScript, isolate),
         v8String(isolate, sourceMapUrl));
 
     OwnPtr<CompileFn> compileFn = streamer
         ? selectCompileFunction(resource, streamer)
         : selectCompileFunction(cacheOptions, cacheHandler, code);
 
     return (*compileFn)(isolate, code, origin);
 }
 
-v8::Local<v8::Value> V8ScriptRunner::runCompiledScript(v8::Isolate* isolate, v8::Handle<v8::Script> script, ExecutionContext* context)
+v8::MaybeLocal<v8::Value> V8ScriptRunner::runCompiledScript(v8::Isolate* isolate, v8::Local<v8::Script> script, ExecutionContext* context)
 {
-    if (script.IsEmpty())

[haraken, 2015/03/18 01:58:51]

Why can we change this to ASSERT?

[bashi, 2015/03/18 02:33:12]

I should have mentioned about this. I changed the contract of this method so
that script should always be non-empty. For consistency, I'd like to make all
methods which return Maybe throw an exception on failure. But I didn't come up
with what kind of exception should be return when script is empty. Instead of
throwing exceptions, I think it'd be better not to accept empty script.

Every call site checks IsEmpty() before.

-        return v8::Local<v8::Value>();
+    ASSERT(!script.IsEmpty());
     TRACE_EVENT_SCOPED_SAMPLING_STATE("v8", "V8Execution");
     TRACE_EVENT1("v8", "v8.run", "fileName", TRACE_STR_COPY(*v8::String::Utf8Value(script->GetUnboundScript()->GetScriptName())));
 
     if (V8RecursionScope::recursionLevel(isolate) >= kMaxRecursionDepth)
         return throwStackOverflowExceptionIfNeeded(isolate);
 
     RELEASE_ASSERT(!context->isIteratingOverObservers());
 
     // Run the script and keep track of the current recursion depth.
-    v8::Local<v8::Value> result;
+    v8::MaybeLocal<v8::Value> result;
     {
-        if (ScriptForbiddenScope::isScriptForbidden())
-            return v8::Local<v8::Value>();
+        if (ScriptForbiddenScope::isScriptForbidden()) {
+            throwScriptForbiddenException(isolate);

[haraken, 2015/03/18 01:58:51]

Shall we make this change in a separate CL? I'm not sure if it is a right thing
to throw an exception in this case.

If I'm understanding correctly, we must not hit this branch. So this should be
RELEASE_ASSERT_NOT_REACHED(). However, we sometimes hit this branch in reality
(this is a bug to be fixed), so we just return an empty value instead of
crashing. But I'm not sure.

[bashi, 2015/03/18 02:33:12]

This means that you want to allow methods which return Maybe fail without
throwing exceptions, which you said you didn't want :)

I'm fine with either, but if not throwing exceptions here, we need to have

if (!methodReturnsMaybe(...).ToLocal(&value)) {
  if (tryCatch.HasCaught()) { ... }
}

checks.

+            return v8::MaybeLocal<v8::Value>();
+        }
         V8RecursionScope recursionScope(isolate);
-        result = script->Run();
+        result = script->Run(isolate->GetCurrentContext());
     }
 
-    if (result.IsEmpty())
-        return v8::Local<v8::Value>();
-
     crashIfV8IsDead();
     return result;
 }
 
-v8::Local<v8::Value> V8ScriptRunner::compileAndRunInternalScript(v8::Handle<v8::String> source, v8::Isolate* isolate, const String& fileName, const TextPosition& scriptStartPosition)
+v8::MaybeLocal<v8::Value> V8ScriptRunner::compileAndRunInternalScript(v8::Handle<v8::String> source, v8::Isolate* isolate, const String& fileName, const TextPosition& scriptStartPosition)
 {
-    v8::Handle<v8::Script> script = V8ScriptRunner::compileScript(source, fileName, String(), scriptStartPosition, isolate, nullptr, nullptr, nullptr, SharableCrossOrigin, V8CacheOptionsDefault, true);
-    if (script.IsEmpty())
-        return v8::Local<v8::Value>();
+    v8::Local<v8::Script> script;
+    if (!V8ScriptRunner::compileScript(source, fileName, String(), scriptStartPosition, isolate, nullptr, nullptr, nullptr, SharableCrossOrigin, V8CacheOptionsDefault, true).ToLocal(&script))
+        return v8::MaybeLocal<v8::Value>();
 
     TRACE_EVENT0("v8", "v8.run");
     TRACE_EVENT_SCOPED_SAMPLING_STATE("v8", "V8Execution");
     V8RecursionScope::MicrotaskSuppression recursionScope(isolate);
-    v8::Local<v8::Value> result = script->Run();
+    v8::MaybeLocal<v8::Value> result = script->Run(isolate->GetCurrentContext());
     crashIfV8IsDead();
     return result;
 }
 
-v8::Local<v8::Value> V8ScriptRunner::runCompiledInternalScript(v8::Isolate* isolate, v8::Handle<v8::Script> script)
+v8::MaybeLocal<v8::Value> V8ScriptRunner::runCompiledInternalScript(v8::Isolate* isolate, v8::Local<v8::Script> script)
 {
     TRACE_EVENT0("v8", "v8.run");
     TRACE_EVENT_SCOPED_SAMPLING_STATE("v8", "V8Execution");
     V8RecursionScope::MicrotaskSuppression recursionScope(isolate);
-    v8::Local<v8::Value> result = script->Run();
+    v8::MaybeLocal<v8::Value> result = script->Run(isolate->GetCurrentContext());
     crashIfV8IsDead();
     return result;
 }
 
 v8::Local<v8::Value> V8ScriptRunner::callFunction(v8::Handle<v8::Function> function, ExecutionContext* context, v8::Handle<v8::Value> receiver, int argc, v8::Handle<v8::Value> args[], v8::Isolate* isolate)
 {
     TRACE_EVENT0("v8", "v8.callFunction");
     TRACE_EVENT_SCOPED_SAMPLING_STATE("v8", "V8Execution");
 
     if (V8RecursionScope::recursionLevel(isolate) >= kMaxRecursionDepth)
         return throwStackOverflowExceptionIfNeeded(isolate);
 
     RELEASE_ASSERT(!context->isIteratingOverObservers());
 
-    if (ScriptForbiddenScope::isScriptForbidden())
+    if (ScriptForbiddenScope::isScriptForbidden()) {
+        throwScriptForbiddenException(isolate);

[haraken, 2015/03/18 01:58:51]

Ditto.

         return v8::Local<v8::Value>();
+    }
     V8RecursionScope recursionScope(isolate);
     v8::Local<v8::Value> result = function->Call(receiver, argc, args);
     crashIfV8IsDead();
     return result;
 }
 
 v8::Local<v8::Value> V8ScriptRunner::callInternalFunction(v8::Handle<v8::Function> function, v8::Handle<v8::Value> receiver, int argc, v8::Handle<v8::Value> args[], v8::Isolate* isolate)
 {
     TRACE_EVENT0("v8", "v8.callFunction");
     TRACE_EVENT_SCOPED_SAMPLING_STATE("v8", "V8Execution");
@@ skipping 41 matching lines @@
 {
     return cacheTag(CacheTagParser, cacheHandler);
 }
 
 unsigned V8ScriptRunner::tagForCodeCache(CachedMetadataHandler* cacheHandler)
 {
     return cacheTag(CacheTagCode, cacheHandler);
 }
 
 } // namespace blink