[deprecation] Deprecate ToBoolean(Local<Context>)

ToBoolean and BooleanValue cannot throw exceptions so the Maybe versions
of the functions don't make sense. As such this deprecates the Maybe
versions and undeprecates ToBoolean(Isolate*). It also adds
BooleanValue(Isolate*).

Fix up all of the v8 code to not use the deprecated functions.

Bug: v8:7279, v8:8015
Cq-Include-Trybots: luci.chromium.try:linux_chromium_headless_rel;luci.chromium.try:linux_chromium_rel_ng;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I50e7474d205c75baa153f0dea7f02dcf60232d1d
Reviewed-on: https://chromium-review.googlesource.com/1238476
Commit-Queue: Dan Elphick <delphick@chromium.org>
Reviewed-by: Clemens Hammacher <clemensh@chromium.org>
Reviewed-by: Yang Guo <yangguo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#56163}

include/v8.h

@@ -2389,8 +2389,9 @@ class V8_EXPORT Value : public Data {
 
   V8_WARN_UNUSED_RESULT MaybeLocal<BigInt> ToBigInt(
       Local<Context> context) const;
+  V8_DEPRECATE_SOON("ToBoolean can never throw. Use Local version.",
                     V8_WARN_UNUSED_RESULT MaybeLocal<Boolean> ToBoolean(
-      Local<Context> context) const;
+                        Local<Context> context) const);
   V8_WARN_UNUSED_RESULT MaybeLocal<Number> ToNumber(
       Local<Context> context) const;
   V8_WARN_UNUSED_RESULT MaybeLocal<String> ToString(
@@ -2405,8 +2406,7 @@ class V8_EXPORT Value : public Data {
       Local<Context> context) const;
   V8_WARN_UNUSED_RESULT MaybeLocal<Int32> ToInt32(Local<Context> context) const;
 
-  V8_DEPRECATE_SOON("Use maybe version",
-                    Local<Boolean> ToBoolean(Isolate* isolate) const);
+  Local<Boolean> ToBoolean(Isolate* isolate) const;
   V8_DEPRECATE_SOON("Use maybe version",
                     Local<Number> ToNumber(Isolate* isolate) const);
   V8_DEPRECATE_SOON("Use maybe version",
@@ -2425,7 +2425,11 @@ class V8_EXPORT Value : public Data {
   V8_WARN_UNUSED_RESULT MaybeLocal<Uint32> ToArrayIndex(
       Local<Context> context) const;
 
-  V8_WARN_UNUSED_RESULT Maybe<bool> BooleanValue(Local<Context> context) const;
+  bool BooleanValue(Isolate* isolate) const;
+
+  V8_DEPRECATE_SOON("BooleanValue can never throw. Use Isolate version.",
+                    V8_WARN_UNUSED_RESULT Maybe<bool> BooleanValue(
+                        Local<Context> context) const);
   V8_WARN_UNUSED_RESULT Maybe<double> NumberValue(Local<Context> context) const;
   V8_WARN_UNUSED_RESULT Maybe<int64_t> IntegerValue(
       Local<Context> context) const;

src/api.cc

@@ -3568,17 +3568,20 @@ MaybeLocal<BigInt> Value::ToBigInt(Local<Context> context) const {
   RETURN_ESCAPED(result);
 }
 
+bool Value::BooleanValue(Isolate* v8_isolate) const {
+  return Utils::OpenHandle(this)->BooleanValue(
+      reinterpret_cast<i::Isolate*>(v8_isolate));
+}
+
 MaybeLocal<Boolean> Value::ToBoolean(Local<Context> context) const {
-  auto obj = Utils::OpenHandle(this);
-  if (obj->IsBoolean()) return ToApiHandle<Boolean>(obj);
-  auto isolate = reinterpret_cast<i::Isolate*>(context->GetIsolate());
-  auto val = isolate->factory()->ToBoolean(obj->BooleanValue(isolate));
-  return ToApiHandle<Boolean>(val);
+  return ToBoolean(context->GetIsolate());
 }
 
 
 Local<Boolean> Value::ToBoolean(Isolate* v8_isolate) const {
-  return ToBoolean(v8_isolate->GetCurrentContext()).ToLocalChecked();
+  auto isolate = reinterpret_cast<i::Isolate*>(v8_isolate);
+  return ToApiHandle<Boolean>(
+      isolate->factory()->ToBoolean(BooleanValue(v8_isolate)));
 }
 
 

src/d8-console.cc

@@ -39,16 +39,9 @@ D8Console::D8Console(Isolate* isolate) : isolate_(isolate) {
 
 void D8Console::Assert(const debug::ConsoleCallArguments& args,
                        const v8::debug::ConsoleContext&) {
-  Local<Boolean> arg;
-  if (args.Length() > 0) {
-    if (!args[0]->ToBoolean(isolate_->GetCurrentContext()).ToLocal(&arg)) {
-      return;
-    }
-  } else {
-    // No arguments given, the "first" argument is undefined which is false-ish.
-    arg = v8::False(isolate_);
-  }
-  if (arg->IsTrue()) return;
+  // If no arguments given, the "first" argument is undefined which is
+  // false-ish.
+  if (args.Length() > 0 && args[0]->BooleanValue(isolate_)) return;
   WriteToFile("console.assert", stdout, isolate_, args);
   isolate_->ThrowException(v8::Exception::Error(
       v8::String::NewFromUtf8(isolate_, "console.assert failed",

src/d8.cc

@@ -363,8 +363,7 @@ class TraceConfigParser {
                          Local<v8::Object> object, const char* property) {
     Local<Value> value = GetValue(isolate, context, object, property);
     if (value->IsNumber()) {
-      Local<Boolean> v8_boolean = value->ToBoolean(context).ToLocalChecked();
-      return v8_boolean->Value();
+      return value->BooleanValue(isolate);
     }
     return false;
   }

src/extensions/externalize-string-extension.cc

@@ -70,10 +70,7 @@ void ExternalizeStringExtension::Externalize(
   bool force_two_byte = false;
   if (args.Length() >= 2) {
     if (args[1]->IsBoolean()) {
-      force_two_byte =
-          args[1]
-              ->BooleanValue(args.GetIsolate()->GetCurrentContext())
-              .FromJust();
+      force_two_byte = args[1]->BooleanValue(args.GetIsolate());
     } else {
       args.GetIsolate()->ThrowException(
           v8::String::NewFromUtf8(

src/extensions/gc-extension.cc

@@ -18,9 +18,7 @@ v8::Local<v8::FunctionTemplate> GCExtension::GetNativeFunctionTemplate(
 
 void GCExtension::GC(const v8::FunctionCallbackInfo<v8::Value>& args) {
   args.GetIsolate()->RequestGarbageCollectionForTesting(
-      args[0]
-              ->BooleanValue(args.GetIsolate()->GetCurrentContext())
-              .FromMaybe(false)
+      args[0]->BooleanValue(args.GetIsolate())
           ? v8::Isolate::kMinorGarbageCollection
           : v8::Isolate::kFullGarbageCollection);
 }

src/extensions/statistics-extension.cc

@@ -63,10 +63,7 @@ void StatisticsExtension::GetCounters(
   Heap* heap = isolate->heap();
 
   if (args.Length() > 0) {  // GC if first argument evaluates to true.
-    if (args[0]->IsBoolean() &&
-        args[0]
-            ->BooleanValue(args.GetIsolate()->GetCurrentContext())
-            .FromMaybe(false)) {
+    if (args[0]->IsBoolean() && args[0]->BooleanValue(args.GetIsolate())) {
       heap->CollectAllGarbage(Heap::kNoGCFlags,
                               GarbageCollectionReason::kCountersExtension);
     }

src/inspector/v8-console.cc

@@ -107,7 +107,7 @@ class ConsoleHelper {
   bool firstArgToBoolean(bool defaultValue) {
     if (m_info.Length() < 1) return defaultValue;
     if (m_info[0]->IsBoolean()) return m_info[0].As<v8::Boolean>()->Value();
-    return m_info[0]->BooleanValue(m_context).FromMaybe(defaultValue);
+    return m_info[0]->BooleanValue(m_context->GetIsolate());
   }
 
   String16 firstArgToString(const String16& defaultValue,

src/wasm/wasm-js.cc

@@ -956,7 +956,7 @@ void WebAssemblyMemory(const v8::FunctionCallbackInfo<v8::Value>& args) {
       v8::MaybeLocal<v8::Value> maybe = descriptor->Get(context, shared_key);
       v8::Local<v8::Value> value;
       if (maybe.ToLocal(&value)) {
-        if (!value->BooleanValue(context).To(&is_shared_memory)) return;
+        is_shared_memory = value->BooleanValue(isolate);
       }
     }
     // Throw TypeError if shared is true, and the descriptor has no "maximum"
@@ -1012,7 +1012,7 @@ void WebAssemblyGlobal(const v8::FunctionCallbackInfo<v8::Value>& args) {
     v8::MaybeLocal<v8::Value> maybe = descriptor->Get(context, mutable_key);
     v8::Local<v8::Value> value;
     if (maybe.ToLocal(&value)) {
-      if (!value->BooleanValue(context).To(&is_mutable)) return;
+      is_mutable = value->BooleanValue(isolate);
     }
   }
 

test/cctest/cctest.h

@@ -507,9 +507,7 @@ static inline void ExpectInt32(const char* code, int expected) {
 static inline void ExpectBoolean(const char* code, bool expected) {
   v8::Local<v8::Value> result = CompileRun(code);
   CHECK(result->IsBoolean());
-  CHECK_EQ(expected,
-           result->BooleanValue(v8::Isolate::GetCurrent()->GetCurrentContext())
-               .FromJust());
+  CHECK_EQ(expected, result->BooleanValue(v8::Isolate::GetCurrent()));
 }
 
 

test/cctest/heap/test-alloc.cc

@@ -168,7 +168,7 @@ TEST(StressJS) {
           .ToLocalChecked()
           ->Run(env)
           .ToLocalChecked();
-  CHECK_EQ(true, result->BooleanValue(env).FromJust());
+  CHECK_EQ(true, result->BooleanValue(CcTest::isolate()));
   env->Exit();
 }
 

test/cctest/test-api-interceptors.cc

@@ -393,9 +393,9 @@ void QueryCallback(Local<Name> property,
 
 // Examples that show when the query callback is triggered.
 THREADED_TEST(QueryInterceptor) {
-  v8::HandleScope scope(CcTest::isolate());
-  v8::Local<v8::FunctionTemplate> templ =
-      v8::FunctionTemplate::New(CcTest::isolate());
+  v8::Isolate* isolate = CcTest::isolate();
+  v8::HandleScope scope(isolate);
+  v8::Local<v8::FunctionTemplate> templ = v8::FunctionTemplate::New(isolate);
   templ->InstanceTemplate()->SetHandler(
       v8::NamedPropertyHandlerConfiguration(nullptr, nullptr, QueryCallback));
   LocalContext env;
@@ -430,43 +430,37 @@ THREADED_TEST(QueryInterceptor) {
   CHECK(v8_compile("obj.propertyIsEnumerable('enum');")
             ->Run(env.local())
             .ToLocalChecked()
-            ->BooleanValue(env.local())
-            .FromJust());
+            ->BooleanValue(isolate));
   CHECK_EQ(4, query_counter_int);
 
   CHECK(!v8_compile("obj.propertyIsEnumerable('not_enum');")
              ->Run(env.local())
              .ToLocalChecked()
-             ->BooleanValue(env.local())
-             .FromJust());
+             ->BooleanValue(isolate));
   CHECK_EQ(5, query_counter_int);
 
   CHECK(v8_compile("obj.hasOwnProperty('enum');")
             ->Run(env.local())
             .ToLocalChecked()
-            ->BooleanValue(env.local())
-            .FromJust());
+            ->BooleanValue(isolate));
   CHECK_EQ(5, query_counter_int);
 
   CHECK(v8_compile("obj.hasOwnProperty('not_enum');")
             ->Run(env.local())
             .ToLocalChecked()
-            ->BooleanValue(env.local())
-            .FromJust());
+            ->BooleanValue(isolate));
   CHECK_EQ(5, query_counter_int);
 
   CHECK(!v8_compile("obj.hasOwnProperty('x');")
              ->Run(env.local())
              .ToLocalChecked()
-             ->BooleanValue(env.local())
-             .FromJust());
+             ->BooleanValue(isolate));
   CHECK_EQ(6, query_counter_int);
 
   CHECK(!v8_compile("obj.propertyIsEnumerable('undef');")
              ->Run(env.local())
              .ToLocalChecked()
-             ->BooleanValue(env.local())
-             .FromJust());
+             ->BooleanValue(isolate));
   CHECK_EQ(7, query_counter_int);
 
   v8_compile("Object.defineProperty(obj, 'enum', {value: 42});")
@@ -835,15 +829,15 @@ THREADED_TEST(InterceptorHasOwnProperty) {
   v8::Local<Value> value = CompileRun(
       "var o = new constructor();"
       "o.hasOwnProperty('ostehaps');");
-  CHECK(!value->BooleanValue(context.local()).FromJust());
+  CHECK(!value->BooleanValue(isolate));
   value = CompileRun(
       "o.ostehaps = 42;"
       "o.hasOwnProperty('ostehaps');");
-  CHECK(value->BooleanValue(context.local()).FromJust());
+  CHECK(value->BooleanValue(isolate));
   value = CompileRun(
       "var p = new constructor();"
       "p.hasOwnProperty('ostehaps');");
-  CHECK(!value->BooleanValue(context.local()).FromJust());
+  CHECK(!value->BooleanValue(isolate));
 }
 
 
@@ -877,7 +871,7 @@ THREADED_TEST(InterceptorHasOwnPropertyCausingGC) {
       "var o = new constructor();"
       "o.__proto__ = new String(x);"
       "o.hasOwnProperty('ostehaps');");
-  CHECK(!value->BooleanValue(context.local()).FromJust());
+  CHECK(!value->BooleanValue(isolate));
 }
 
 
@@ -1354,7 +1348,7 @@ THREADED_TEST(InterceptorLoadGlobalICGlobalWithInterceptor) {
       "  f();"
       "};"
       "f();");
-  CHECK(value->BooleanValue(context.local()).FromJust());
+  CHECK(value->BooleanValue(isolate));
 
   value = CompileRun(
       "var f = function() { "
@@ -1369,7 +1363,7 @@ THREADED_TEST(InterceptorLoadGlobalICGlobalWithInterceptor) {
       "  f();"
       "};"
       "f();");
-  CHECK(value->BooleanValue(context.local()).FromJust());
+  CHECK(value->BooleanValue(isolate));
 
   value = CompileRun(
       "var f = function() { "
@@ -1384,7 +1378,7 @@ THREADED_TEST(InterceptorLoadGlobalICGlobalWithInterceptor) {
       "  f();"
       "};"
       "f();");
-  CHECK(value->BooleanValue(context.local()).FromJust());
+  CHECK(value->BooleanValue(isolate));
 }
 
 // Test load of a non-existing global through prototype chain when a global
@@ -1575,9 +1569,9 @@ THREADED_TEST(GenericInterceptorDoesSeeSymbols) {
 
 THREADED_TEST(NamedPropertyHandlerGetter) {
   echo_named_call_count = 0;
-  v8::HandleScope scope(CcTest::isolate());
-  v8::Local<v8::FunctionTemplate> templ =
-      v8::FunctionTemplate::New(CcTest::isolate());
+  v8::Isolate* isolate = CcTest::isolate();
+  v8::HandleScope scope(isolate);
+  v8::Local<v8::FunctionTemplate> templ = v8::FunctionTemplate::New(isolate);
   templ->InstanceTemplate()->SetHandler(v8::NamedPropertyHandlerConfiguration(
       EchoNamedProperty, nullptr, nullptr, nullptr, nullptr, v8_str("data")));
   LocalContext env;
@@ -1592,7 +1586,7 @@ THREADED_TEST(NamedPropertyHandlerGetter) {
   CHECK_EQ(1, echo_named_call_count);
   const char* code = "var str = 'oddle'; obj[str] + obj.poddle;";
   v8::Local<Value> str = CompileRun(code);
-  String::Utf8Value value(CcTest::isolate(), str);
+  String::Utf8Value value(isolate, str);
   CHECK_EQ(0, strcmp(*value, "oddlepoddle"));
   // Check default behavior
   CHECK_EQ(10, v8_compile("obj.flob = 10;")
@@ -1603,13 +1597,11 @@ THREADED_TEST(NamedPropertyHandlerGetter) {
   CHECK(v8_compile("'myProperty' in obj")
             ->Run(env.local())
             .ToLocalChecked()
-            ->BooleanValue(env.local())
-            .FromJust());
+            ->BooleanValue(isolate));
   CHECK(v8_compile("delete obj.myProperty")
             ->Run(env.local())
             .ToLocalChecked()
-            ->BooleanValue(env.local())
-            .FromJust());
+            ->BooleanValue(isolate));
 }
 
 namespace {
@@ -1838,10 +1830,10 @@ THREADED_TEST(PropertyDefinerCallbackIndexed) {
 
 // Test that freeze() is intercepted.
 THREADED_TEST(PropertyDefinerCallbackForFreeze) {
-  v8::HandleScope scope(CcTest::isolate());
+  v8::Isolate* isolate = CcTest::isolate();
+  v8::HandleScope scope(isolate);
   LocalContext env;
-  v8::Local<v8::FunctionTemplate> templ =
-      v8::FunctionTemplate::New(CcTest::isolate());
+  v8::Local<v8::FunctionTemplate> templ = v8::FunctionTemplate::New(isolate);
   templ->InstanceTemplate()->SetHandler(v8::NamedPropertyHandlerConfiguration(
       nullptr, nullptr, nullptr, nullptr, nullptr,
       InterceptingPropertyDefineCallback));
@@ -1859,8 +1851,7 @@ THREADED_TEST(PropertyDefinerCallbackForFreeze) {
   CHECK(v8_compile(code)
             ->Run(env.local())
             .ToLocalChecked()
-            ->BooleanValue(env.local())
-            .FromJust());
+            ->BooleanValue(isolate));
 }
 
 // Check that the descriptor passed to the callback is enumerable.
@@ -4141,7 +4132,7 @@ THREADED_TEST(InterceptorICReferenceErrors) {
       "  return false;"
       "};"
       "f();");
-  CHECK(value->BooleanValue(context.local()).FromJust());
+  CHECK(value->BooleanValue(isolate));
   interceptor_call_count = 0;
   value = CompileRun(
       "function g() {"
@@ -4151,7 +4142,7 @@ THREADED_TEST(InterceptorICReferenceErrors) {
       "  return false;"
       "};"
       "g();");
-  CHECK(value->BooleanValue(context.local()).FromJust());
+  CHECK(value->BooleanValue(isolate));
 }
 
 
@@ -4197,7 +4188,7 @@ THREADED_TEST(InterceptorICGetterExceptions) {
       "  return false;"
       "};"
       "f();");
-  CHECK(value->BooleanValue(context.local()).FromJust());
+  CHECK(value->BooleanValue(isolate));
   interceptor_ic_exception_get_count = 0;
   value = CompileRun(
       "function f() {"
@@ -4207,7 +4198,7 @@ THREADED_TEST(InterceptorICGetterExceptions) {
       "  return false;"
       "};"
       "f();");
-  CHECK(value->BooleanValue(context.local()).FromJust());
+  CHECK(value->BooleanValue(isolate));
 }
 
 
@@ -4241,7 +4232,7 @@ THREADED_TEST(InterceptorICSetterExceptions) {
       "  return false;"
       "};"
       "f();");
-  CHECK(value->BooleanValue(context.local()).FromJust());
+  CHECK(value->BooleanValue(isolate));
 }
 
 

test/cctest/test-deoptimization.cc

@@ -720,8 +720,7 @@ UNINITIALIZED_TEST(DeoptimizeCompare) {
     CHECK_EQ(true, env->Global()
                        ->Get(env.local(), v8_str("result"))
                        .ToLocalChecked()
-                       ->BooleanValue(env.local())
-                       .FromJust());
+                       ->BooleanValue(isolate));
     CHECK_EQ(0, Deoptimizer::GetDeoptimizedCodeCount(i_isolate));
   }
   isolate->Exit();

test/cctest/test-javascript-arm64.cc

@@ -46,7 +46,7 @@ namespace test_javascript_arm64 {
 static void ExpectBoolean(Local<v8::Context> context, bool expected,
                           Local<Value> result) {
   CHECK(result->IsBoolean());
-  CHECK_EQ(expected, result->BooleanValue(context).FromJust());
+  CHECK_EQ(expected, result->BooleanValue(context->GetIsolate()));
 }
 
 static void ExpectInt32(Local<v8::Context> context, int32_t expected,

test/cctest/test-trace-event.cc

@@ -289,7 +289,6 @@ TEST(BuiltinsIsTraceCategoryEnabled) {
 
   v8::Isolate* isolate = CcTest::isolate();
   v8::HandleScope handle_scope(isolate);
-  v8::Local<v8::Context> context = isolate->GetCurrentContext();
   LocalContext env;
 
   v8::Local<v8::Object> binding = env->GetExtrasBindingObject();
@@ -308,7 +307,7 @@ TEST(BuiltinsIsTraceCategoryEnabled) {
                       .ToLocalChecked()
                       .As<v8::Boolean>();
 
-    CHECK(result->BooleanValue(context).ToChecked());
+    CHECK(result->BooleanValue(isolate));
   }
 
   {
@@ -318,7 +317,7 @@ TEST(BuiltinsIsTraceCategoryEnabled) {
                       .ToLocalChecked()
                       .As<v8::Boolean>();
 
-    CHECK(!result->BooleanValue(context).ToChecked());
+    CHECK(!result->BooleanValue(isolate));
   }
 
   {
@@ -328,7 +327,7 @@ TEST(BuiltinsIsTraceCategoryEnabled) {
                       .ToLocalChecked()
                       .As<v8::Boolean>();
 
-    CHECK(result->BooleanValue(context).ToChecked());
+    CHECK(result->BooleanValue(isolate));
   }
 }
 
@@ -362,7 +361,7 @@ TEST(BuiltinsTrace) {
                       .ToLocalChecked()
                       .As<v8::Boolean>();
 
-    CHECK(!result->BooleanValue(context).ToChecked());
+    CHECK(!result->BooleanValue(isolate));
     CHECK_EQ(0, GET_TRACE_OBJECTS_LIST->size());
   }
 
@@ -381,7 +380,7 @@ TEST(BuiltinsTrace) {
                       .ToLocalChecked()
                       .As<v8::Boolean>();
 
-    CHECK(result->BooleanValue(context).ToChecked());
+    CHECK(result->BooleanValue(isolate));
     CHECK_EQ(1, GET_TRACE_OBJECTS_LIST->size());
 
     CHECK_EQ(123, GET_TRACE_OBJECT(0)->id);
@@ -405,7 +404,7 @@ TEST(BuiltinsTrace) {
                       .ToLocalChecked()
                       .As<v8::Boolean>();
 
-    CHECK(result->BooleanValue(context).ToChecked());
+    CHECK(result->BooleanValue(isolate));
     CHECK_EQ(2, GET_TRACE_OBJECTS_LIST->size());
 
     CHECK_EQ(123, GET_TRACE_OBJECT(1)->id);

test/unittests/value-serializer-unittest.cc

@@ -228,7 +228,7 @@ class ValueSerializerTest : public TestWithIsolate {
     Local<Script> script =
         Script::Compile(deserialization_context_, source).ToLocalChecked();
     Local<Value> value = script->Run(deserialization_context_).ToLocalChecked();
-    EXPECT_TRUE(value->BooleanValue(deserialization_context_).FromJust());
+    EXPECT_TRUE(value->BooleanValue(isolate()));
   }
 
   Local<String> StringFromUtf8(const char* source) {