Issue 6526 - *ToV8String() return MaybeLocal<> and check Call() return value

src/JsEngine.cpp

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * Copyright (C) 2006-present eyeo GmbH
  *
  * Adblock Plus is free software: you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 3 as
  * published by the Free Software Foundation.
  *
  * Adblock Plus is distributed in the hope that it will be useful,
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
@@ skipping 11 matching lines @@
 #include "Utils.h"
 #include <libplatform/libplatform.h>
 #include <AdblockPlus/Platform.h>
 
 namespace
 {
   v8::MaybeLocal<v8::Script> CompileScript(v8::Isolate* isolate,
     const std::string& source, const std::string& filename)
   {
     using AdblockPlus::Utils::ToV8String;
-    const v8::Local<v8::String> v8Source = ToV8String(isolate, source);
+    auto maybeV8Source = ToV8String(isolate, source);
+    if (maybeV8Source.IsEmpty())
+      return v8::MaybeLocal<v8::Script>();
+    const v8::Local<v8::String> v8Source = maybeV8Source.ToLocalChecked();
     if (filename.length())
     {
-      const v8::Local<v8::String> v8Filename = ToV8String(isolate, filename);
-      v8::ScriptOrigin scriptOrigin(v8Filename);
+      auto maybeV8Filename = ToV8String(isolate, filename);
+      if (maybeV8Filename.IsEmpty())
+        return v8::MaybeLocal<v8::Script>();
+      v8::ScriptOrigin scriptOrigin(maybeV8Filename.ToLocalChecked());
       return v8::Script::Compile(isolate->GetCurrentContext(), v8Source, &scriptOrigin);
     }
     else
       return v8::Script::Compile(isolate->GetCurrentContext(), v8Source);
   }
 
   class V8Initializer
   {
     V8Initializer()
       : platform{nullptr}
@@ skipping 178 matching lines @@
 }
 
 void AdblockPlus::JsEngine::Gc()
 {
   while (!GetIsolate()->IdleNotificationDeadline(1000));
 }
 
 AdblockPlus::JsValue AdblockPlus::JsEngine::NewValue(const std::string& val)
 {
   const JsContext context(*this);
-  return JsValue(shared_from_this(), Utils::ToV8String(GetIsolate(), val));
+  auto maybeValue = Utils::ToV8String(GetIsolate(), val);
+  if (maybeValue.IsEmpty())
+    return JsValue(shared_from_this(), v8::Undefined(GetIsolate()));

[hub, 2018/06/22 00:34:31]

This is where I think we should break APIs to some extent. For now I return
`undefined` if there is an error. 

[sergei, 2018/06/22 06:57:46]

Let's just use CHECKED_TO_LOCAL.

[hub, 2018/06/22 15:50:56]

Done.

+  return JsValue(shared_from_this(), maybeValue.ToLocalChecked());
 }
 
 AdblockPlus::JsValue AdblockPlus::JsEngine::NewValue(int64_t val)
 {
   const JsContext context(*this);
   return JsValue(shared_from_this(), v8::Number::New(GetIsolate(), val));
 }
 
 AdblockPlus::JsValue AdblockPlus::JsEngine::NewValue(bool val)
 {
@@ skipping 88 matching lines @@
     list.push_back(JsValue(shared_from_this(), arguments[i]));
   return list;
 }
 
 void AdblockPlus::JsEngine::SetGlobalProperty(const std::string& name,
                                               const AdblockPlus::JsValue& value)
 {
   auto global = GetGlobalObject();
   global.SetProperty(name, value);
 }