[debug] fix NewInstanceWithSideEffectType dcheck for mode

The DCHECK was incorrect. This new API method can be called from any
debug mode since the embedder does not know which mode we are in.

It should only apply the side effect logic when the mode is
kSideEffects.

Bug: chromium:829571
Cq-Include-Trybots: luci.chromium.try:linux_chromium_rel_ng
Change-Id: I11b0e5194b151a2b88171d6be21c3ccbba9cd408
Reviewed-on: https://chromium-review.googlesource.com/1046162Reviewed-by: Yang Guo <yangguo@chromium.org>
Commit-Queue: Erik Luo <luoe@chromium.org>
Cr-Commit-Position: refs/heads/master@{#53030}

src/api.cc

@@ -5135,10 +5135,12 @@ MaybeLocal<Object> Function::NewInstanceWithSideEffectType(
   i::TimerEventScope<i::TimerEventExecute> timer_scope(isolate);
   auto self = Utils::OpenHandle(this);
   STATIC_ASSERT(sizeof(v8::Local<v8::Value>) == sizeof(i::Object**));
-  if (side_effect_type == SideEffectType::kHasNoSideEffect) {
+  bool should_set_has_no_side_effect =
+      side_effect_type == SideEffectType::kHasNoSideEffect &&
+      isolate->debug_execution_mode() == i::DebugInfo::kSideEffects;
+  if (should_set_has_no_side_effect) {
     CHECK(self->IsJSFunction() &&
           i::JSFunction::cast(*self)->shared()->IsApiFunction());
-    DCHECK(isolate->debug_execution_mode() == i::DebugInfo::kSideEffects);
     i::Object* obj =
         i::JSFunction::cast(*self)->shared()->get_api_func_data()->call_code();
     if (obj->IsCallHandlerInfo()) {
@@ -5152,7 +5154,7 @@ MaybeLocal<Object> Function::NewInstanceWithSideEffectType(
   Local<Object> result;
   has_pending_exception = !ToLocal<Object>(
       i::Execution::New(isolate, self, self, argc, args), &result);
-  if (side_effect_type == SideEffectType::kHasNoSideEffect) {
+  if (should_set_has_no_side_effect) {
     i::Object* obj =
         i::JSFunction::cast(*self)->shared()->get_api_func_data()->call_code();
     if (obj->IsCallHandlerInfo()) {

test/cctest/test-api.cc

@@ -13185,6 +13185,10 @@ TEST(FunctionNewInstanceHasNoSideEffect) {
   CHECK(
       context->Global()->Set(context.local(), v8_str("f3"), func3).FromJust());
   CHECK(v8::debug::EvaluateGlobal(isolate, v8_str("f3()"), true).IsEmpty());
+
+  // Check that using side effect free NewInstance works in normal evaluation
+  // (without throwOnSideEffect).
+  v8::debug::EvaluateGlobal(isolate, v8_str("f2()"), false).ToLocalChecked();
 }
 
 TEST(CallHandlerAsFunctionHasNoSideEffectNotSupported) {