[inspector] postpone API interrupts during creation of injected script

DevTools may process another protocol message during API interrupt this
API may lead to createInjectedScript reentrance and will fail.
Let's postpone interrupts.

Bug: chromium:846099
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: Ia06e034a6287087e4674559d8911d2f4a0b1b459
Reviewed-on: https://chromium-review.googlesource.com/1086372
Commit-Queue: Aleksey Kozyatinskiy <kozyatinskiy@chromium.org>
Reviewed-by: Yang Guo <yangguo@chromium.org>
Reviewed-by: Dmitry Gozman <dgozman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#53531}

src/api.cc

@@ -9839,6 +9839,13 @@ bool debug::SetFunctionBreakpoint(v8::Local<v8::Function> function,
                                                     condition_string, id);
 }
 
+debug::PostponeInterruptsScope::PostponeInterruptsScope(v8::Isolate* isolate)
+    : scope_(
+          new i::PostponeInterruptsScope(reinterpret_cast<i::Isolate*>(isolate),
+                                         i::StackGuard::API_INTERRUPT)) {}
+
+debug::PostponeInterruptsScope::~PostponeInterruptsScope() {}
+
 Local<String> CpuProfileNode::GetFunctionName() const {
   const i::ProfileNode* node = reinterpret_cast<const i::ProfileNode*>(this);
   i::Isolate* isolate = node->isolate();

src/debug/debug-interface.h

@@ -21,6 +21,7 @@ struct CoverageScript;
 struct TypeProfileEntry;
 struct TypeProfileScript;
 class Coverage;
+class PostponeInterruptsScope;
 class Script;
 class TypeProfile;
 }  // namespace internal
@@ -477,6 +478,15 @@ bool SetFunctionBreakpoint(v8::Local<v8::Function> function,
 
 v8::Platform* GetCurrentPlatform();
 
+class PostponeInterruptsScope {
+ public:
+  explicit PostponeInterruptsScope(v8::Isolate* isolate);
+  ~PostponeInterruptsScope();
+
+ private:
+  std::unique_ptr<i::PostponeInterruptsScope> scope_;
+};
+
 }  // namespace debug
 }  // namespace v8
 

src/inspector/injected-script.cc

@@ -263,6 +263,7 @@ std::unique_ptr<InjectedScript> InjectedScript::create(
   v8::HandleScope handles(isolate);
   v8::TryCatch tryCatch(isolate);
   v8::Local<v8::Context> context = inspectedContext->context();
+  v8::debug::PostponeInterruptsScope postponeInterrupts(isolate);
   v8::Context::Scope scope(context);
   v8::MicrotasksScope microtasksScope(isolate,
                                       v8::MicrotasksScope::kDoNotRunMicrotasks);

src/inspector/inspected-context.cc

@@ -110,11 +110,11 @@ InjectedScript* InspectedContext::getInjectedScript(int sessionId) {
 }
 
 bool InspectedContext::createInjectedScript(int sessionId) {
-  DCHECK(m_injectedScripts.find(sessionId) == m_injectedScripts.end());
   std::unique_ptr<InjectedScript> injectedScript =
       InjectedScript::create(this, sessionId);
   // InjectedScript::create can destroy |this|.
   if (!injectedScript) return false;
+  CHECK(m_injectedScripts.find(sessionId) == m_injectedScripts.end());
   m_injectedScripts[sessionId] = std::move(injectedScript);
   return true;
 }

test/cctest/BUILD.gn

@@ -192,6 +192,7 @@ v8_source_set("cctest_sources") {
     "test-heap-profiler.cc",
     "test-identity-map.cc",
     "test-inobject-slack-tracking.cc",
+    "test-inspector.cc",
     "test-intl.cc",
     "test-isolate-independent-builtins.cc",
     "test-liveedit.cc",

test/cctest/test-inspector.cc

+// Copyright 2018 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include <memory>
+
+#include "test/cctest/cctest.h"
+
+#include "include/v8-inspector.h"
+#include "include/v8.h"
+#include "src/inspector/protocol/Runtime.h"
+
+using v8_inspector::StringBuffer;
+using v8_inspector::StringView;
+using v8_inspector::V8ContextInfo;
+using v8_inspector::V8Inspector;
+using v8_inspector::V8InspectorSession;
+
+namespace {
+
+class NoopChannel : public V8Inspector::Channel {
+ public:
+  virtual ~NoopChannel() {}
+  void sendResponse(int callId,
+                    std::unique_ptr<StringBuffer> message) override {}
+  void sendNotification(std::unique_ptr<StringBuffer> message) override {}
+  void flushProtocolNotifications() override {}
+};
+
+void WrapOnInterrupt(v8::Isolate* isolate, void* data) {
+  const char* object_group = "";
+  StringView object_group_view(reinterpret_cast<const uint8_t*>(object_group),
+                               strlen(object_group));
+  reinterpret_cast<V8InspectorSession*>(data)->wrapObject(
+      isolate->GetCurrentContext(), v8::Null(isolate), object_group_view,
+      false);
+}
+
+}  // namespace
+
+TEST(WrapInsideWrapOnInterrupt) {
+  LocalContext env;
+  v8::Isolate* isolate = env->GetIsolate();
+  v8::HandleScope handle_scope(isolate);
+
+  v8_inspector::V8InspectorClient default_client;
+  std::unique_ptr<V8Inspector> inspector =
+      V8Inspector::create(isolate, &default_client);
+  const char* name = "";
+  StringView name_view(reinterpret_cast<const uint8_t*>(name), strlen(name));
+  V8ContextInfo context_info(env.local(), 1, name_view);
+  inspector->contextCreated(context_info);
+
+  NoopChannel channel;
+  const char* state = "{}";
+  StringView state_view(reinterpret_cast<const uint8_t*>(state), strlen(state));
+  std::unique_ptr<V8InspectorSession> session =
+      inspector->connect(1, &channel, state_view);
+
+  const char* object_group = "";
+  StringView object_group_view(reinterpret_cast<const uint8_t*>(object_group),
+                               strlen(object_group));
+  isolate->RequestInterrupt(&WrapOnInterrupt, session.get());
+  session->wrapObject(env.local(), v8::Null(isolate), object_group_view, false);
+}

tools/gcmole/BUILD.gn

@@ -23,6 +23,7 @@ group("v8_run_gcmole") {
     "../../testing/gtest/include/gtest/gtest_prod.h",
     "../../third_party/googletest/src/googletest/include/gtest/gtest_prod.h",
     "../../third_party/icu/source/",
+    "$target_gen_dir/../../",
     "$target_gen_dir/../../torque-generated/",
   ]