Use vanilla context for exception meta data

Bug: chromium:1213393, chromium:1218340
Change-Id: Icde33c97d39a3504ca2ab8290ec2f0b0d923060d
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2953194
Commit-Queue: Sigurd Schneider <sigurds@chromium.org>
Reviewed-by: Toon Verwaest <verwaest@chromium.org>
Reviewed-by: Yang Guo <yangguo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#75201}

include/js_protocol.pdl

@@ -1254,7 +1254,7 @@ domain Runtime
       optional RemoteObject exception
       # Identifier of the context where exception happened.
       optional ExecutionContextId executionContextId
-      # Dictionary with entries of meta deta that the client associated
+      # Dictionary with entries of meta data that the client associated
       # with this exception, such as information about associated network
       # requests, etc.
       experimental optional object exceptionMetaData

src/inspector/v8-console-message.cc

@@ -332,7 +332,8 @@ void V8ConsoleMessage::reportToFrontend(protocol::Runtime::Frontend* frontend,
     }
     if (m_contextId) exceptionDetails->setExecutionContextId(m_contextId);
     if (exception) exceptionDetails->setException(std::move(exception));
-    auto data = getAssociatedExceptionData(inspector, session);
+    std::unique_ptr<protocol::DictionaryValue> data =
+        getAssociatedExceptionData(inspector, session);
     if (data) exceptionDetails->setExceptionMetaData(std::move(data));
     frontend->exceptionThrown(m_timestamp, std::move(exceptionDetails));
     return;
@@ -388,23 +389,24 @@ V8ConsoleMessage::getAssociatedExceptionData(
     V8InspectorImpl* inspector, V8InspectorSessionImpl* session) const {
   if (!m_arguments.size() || !m_contextId) return nullptr;
   DCHECK_EQ(1u, m_arguments.size());
-  InspectedContext* inspectedContext =
-      session->inspector()->getContext(session->contextGroupId(), m_contextId);
-  if (!inspectedContext) return nullptr;
 
-  v8::Isolate* isolate = inspectedContext->isolate();
+  v8::Isolate* isolate = inspector->isolate();
   v8::HandleScope handles(isolate);
+  v8::Local<v8::Context> context;
+  if (!inspector->exceptionMetaDataContext().ToLocal(&context)) return nullptr;
   v8::MaybeLocal<v8::Value> maybe_exception = m_arguments[0]->Get(isolate);
   v8::Local<v8::Value> exception;
   if (!maybe_exception.ToLocal(&exception)) return nullptr;
 
-  v8::MaybeLocal<v8::Object> maybe_data = inspector->getAssociatedExceptionData(
+  v8::MaybeLocal<v8::Object> maybe_data =
-      inspectedContext->context(), exception);
+      inspector->getAssociatedExceptionData(exception);
   v8::Local<v8::Object> data;
   if (!maybe_data.ToLocal(&data)) return nullptr;
-
+  v8::MicrotasksScope microtasksScope(isolate,
+                                      v8::MicrotasksScope::kDoNotRunMicrotasks);
+  v8::Context::Scope contextScope(context);
   std::unique_ptr<protocol::DictionaryValue> jsonObject;
-  objectToProtocolValue(inspectedContext->context(), data, 2, &jsonObject);
+  objectToProtocolValue(context, data, 2, &jsonObject);
   return jsonObject;
 }
 

src/inspector/v8-inspector-impl.cc

@@ -377,6 +377,17 @@ v8::MaybeLocal<v8::Context> V8InspectorImpl::regexContext() {
   return m_regexContext.Get(m_isolate);
 }
 
+v8::MaybeLocal<v8::Context> V8InspectorImpl::exceptionMetaDataContext() {
+  if (m_exceptionMetaDataContext.IsEmpty()) {
+    m_exceptionMetaDataContext.Reset(m_isolate, v8::Context::New(m_isolate));
+    if (m_exceptionMetaDataContext.IsEmpty()) {
+      DCHECK(m_isolate->IsExecutionTerminating());
+      return {};
+    }
+  }
+  return m_exceptionMetaDataContext.Get(m_isolate);
+}
+
 void V8InspectorImpl::discardInspectedContext(int contextGroupId,
                                               int contextId) {
   auto* context = getContext(contextGroupId, contextId);
@@ -492,23 +503,26 @@ protocol::Response V8InspectorImpl::EvaluateScope::setTimeout(double timeout) {
   return protocol::Response::Success();
 }
 
-bool V8InspectorImpl::associateExceptionData(v8::Local<v8::Context> context,
+bool V8InspectorImpl::associateExceptionData(v8::Local<v8::Context>,
                                              v8::Local<v8::Value> exception,
                                              v8::Local<v8::Name> key,
                                              v8::Local<v8::Value> value) {
+  v8::Local<v8::Context> context;
+  if (!exceptionMetaDataContext().ToLocal(&context)) return false;
   v8::HandleScope handles(m_isolate);
-  if (m_excepetionMetaData.IsEmpty())
+  if (m_exceptionMetaData.IsEmpty())
-    m_excepetionMetaData.Reset(m_isolate, v8::debug::WeakMap::New(m_isolate));
+    m_exceptionMetaData.Reset(m_isolate, v8::debug::WeakMap::New(m_isolate));
 
-  v8::Local<v8::debug::WeakMap> map = m_excepetionMetaData.Get(m_isolate);
+  v8::Local<v8::debug::WeakMap> map = m_exceptionMetaData.Get(m_isolate);
   v8::MaybeLocal<v8::Value> entry = map->Get(context, exception);
   v8::Local<v8::Object> object;
   if (entry.IsEmpty() || !entry.ToLocalChecked()->IsObject()) {
-    object = v8::Object::New(m_isolate);
+    object =
+        v8::Object::New(m_isolate, v8::Null(m_isolate), nullptr, nullptr, 0);
     v8::MaybeLocal<v8::debug::WeakMap> new_map =
         map->Set(context, exception, object);
     if (!new_map.IsEmpty()) {
-      m_excepetionMetaData.Reset(m_isolate, new_map.ToLocalChecked());
+      m_exceptionMetaData.Reset(m_isolate, new_map.ToLocalChecked());
     }
   } else {
     object = entry.ToLocalChecked().As<v8::Object>();
@@ -519,12 +533,17 @@ bool V8InspectorImpl::associateExceptionData(v8::Local<v8::Context> context,
 }
 
 v8::MaybeLocal<v8::Object> V8InspectorImpl::getAssociatedExceptionData(
-    v8::Local<v8::Context> context, v8::Local<v8::Value> exception) {
+    v8::Local<v8::Value> exception) {
-  if (m_excepetionMetaData.IsEmpty()) return v8::MaybeLocal<v8::Object>();
+  v8::EscapableHandleScope scope(m_isolate);
-
+  v8::Local<v8::Context> context;
-  v8::Local<v8::debug::WeakMap> map = m_excepetionMetaData.Get(m_isolate);
+  if (m_exceptionMetaData.IsEmpty() ||
+      !exceptionMetaDataContext().ToLocal(&context)) {
+    return v8::MaybeLocal<v8::Object>();
+  }
+  v8::Local<v8::debug::WeakMap> map = m_exceptionMetaData.Get(m_isolate);
   auto entry = map->Get(context, exception);
-  if (entry.IsEmpty()) return v8::MaybeLocal<v8::Object>();
+  v8::Local<v8::Value> object;
-  return entry.ToLocalChecked().As<v8::Object>();
+  if (!entry.ToLocal(&object)) return v8::MaybeLocal<v8::Object>();
+  return scope.Escape(object.As<v8::Object>());
 }
 }  // namespace v8_inspector

src/inspector/v8-inspector-impl.h

@@ -36,13 +36,12 @@
 #include <memory>
 #include <unordered_map>
 
+#include "include/v8-inspector.h"
 #include "src/base/macros.h"
 #include "src/base/platform/mutex.h"
 #include "src/inspector/injected-script.h"
 #include "src/inspector/protocol/Protocol.h"
 
-#include "include/v8-inspector.h"
-
 namespace v8_inspector {
 
 class InspectedContext;
@@ -76,6 +75,7 @@ class V8InspectorImpl : public V8Inspector {
                                            const String16& code,
                                            const String16& fileName);
   v8::MaybeLocal<v8::Context> regexContext();
+  v8::MaybeLocal<v8::Context> exceptionMetaDataContext();
 
   // V8Inspector implementation.
   std::unique_ptr<V8InspectorSession> connect(int contextGroupId,
@@ -137,7 +137,7 @@ class V8InspectorImpl : public V8Inspector {
       const std::function<void(V8InspectorSessionImpl*)>& callback);
   int64_t generateUniqueId();
   v8::MaybeLocal<v8::Object> getAssociatedExceptionData(
-      v8::Local<v8::Context> context, v8::Local<v8::Value> exception);
+      v8::Local<v8::Value> exception);
 
   class EvaluateScope {
    public:
@@ -163,7 +163,8 @@ class V8InspectorImpl : public V8Inspector {
   V8InspectorClient* m_client;
   std::unique_ptr<V8Debugger> m_debugger;
   v8::Global<v8::Context> m_regexContext;
-  v8::Global<v8::debug::WeakMap> m_excepetionMetaData;
+  v8::Global<v8::Context> m_exceptionMetaDataContext;
+  v8::Global<v8::debug::WeakMap> m_exceptionMetaData;
   int m_capturingStackTracesCount;
   unsigned m_lastExceptionId;
   int m_lastContextId;

test/inspector/inspector-test.cc

@@ -475,6 +475,9 @@ class InspectorExtension : public IsolateData::SetupGlobalTask {
     inspector->Set(isolate, "setResourceNamePrefix",
                    v8::FunctionTemplate::New(
                        isolate, &InspectorExtension::SetResourceNamePrefix));
+    inspector->Set(isolate, "newExceptionWithMetaData",
+                   v8::FunctionTemplate::New(
+                       isolate, &InspectorExtension::newExceptionWithMetaData));
     global->Set(isolate, "inspector", inspector);
   }
 
@@ -723,7 +726,6 @@ class InspectorExtension : public IsolateData::SetupGlobalTask {
     args.GetIsolate()->GetCurrentContext()->AllowCodeGenerationFromStrings(
         args[0].As<v8::Boolean>()->Value());
   }
-
   static void SetResourceNamePrefix(
       const v8::FunctionCallbackInfo<v8::Value>& args) {
     if (args.Length() != 1 || !args[0]->IsString()) {
@@ -734,6 +736,24 @@ class InspectorExtension : public IsolateData::SetupGlobalTask {
     IsolateData* data = IsolateData::FromContext(context);
     data->SetResourceNamePrefix(v8::Local<v8::String>::Cast(args[0]));
   }
+
+  static void newExceptionWithMetaData(
+      const v8::FunctionCallbackInfo<v8::Value>& args) {
+    if (args.Length() != 3 || !args[0]->IsString() || !args[1]->IsString() ||
+        !args[2]->IsString()) {
+      FATAL(
+          "Internal error: newExceptionWithMetaData('message', 'key', "
+          "'value').");
+    }
+    v8::Isolate* isolate = args.GetIsolate();
+    v8::Local<v8::Context> context = isolate->GetCurrentContext();
+    IsolateData* data = IsolateData::FromContext(context);
+
+    auto error = v8::Exception::Error(args[0].As<v8::String>());
+    CHECK(data->AssociateExceptionData(error, args[1].As<v8::String>(),
+                                       args[2].As<v8::String>()));
+    args.GetReturnValue().Set(error);
+  }
 };
 
 int InspectorTestMain(int argc, char* argv[]) {

test/inspector/isolate-data.cc

@@ -473,6 +473,13 @@ void IsolateData::SetResourceNamePrefix(v8::Local<v8::String> prefix) {
   resource_name_prefix_.Reset(isolate(), prefix);
 }
 
+bool IsolateData::AssociateExceptionData(v8::Local<v8::Value> exception,
+                                         v8::Local<v8::Name> key,
+                                         v8::Local<v8::Value> value) {
+  return inspector_->associateExceptionData(
+      this->isolate()->GetCurrentContext(), exception, key, value);
+}
+
 namespace {
 class StringBufferImpl : public v8_inspector::StringBuffer {
  public:

test/inspector/isolate-data.h

@@ -97,6 +97,9 @@ class IsolateData : public v8_inspector::V8InspectorClient {
   void FireContextDestroyed(v8::Local<v8::Context> context);
   void FreeContext(v8::Local<v8::Context> context);
   void SetResourceNamePrefix(v8::Local<v8::String> prefix);
+  bool AssociateExceptionData(v8::Local<v8::Value> exception,
+                              v8::Local<v8::Name> key,
+                              v8::Local<v8::Value> value);
 
  private:
   static v8::MaybeLocal<v8::Module> ModuleResolveCallback(

test/inspector/runtime/exception-thrown-metadata-expected.txt

@@ -0,0 +1,105 @@
+Check that exceptionThrown is supported by test runner.
+{
+    method : Runtime.exceptionThrown
+    params : {
+        exceptionDetails : {
+            columnNumber : 0
+            exception : {
+                className : Error
+                description : Error: myerror     at <anonymous>:1:17
+                objectId : <objectId>
+                preview : {
+                    description : Error: myerror     at <anonymous>:1:17
+                    overflow : false
+                    properties : [
+                        [0] : {
+                            name : stack
+                            type : string
+                            value : Error: myerror     at <anonymous>:1:17
+                        }
+                        [1] : {
+                            name : message
+                            type : string
+                            value : myerror
+                        }
+                    ]
+                    subtype : error
+                    type : object
+                }
+                subtype : error
+                type : object
+            }
+            exceptionId : <exceptionId>
+            exceptionMetaData : {
+                foo : bar
+            }
+            executionContextId : <executionContextId>
+            lineNumber : 0
+            stackTrace : {
+                callFrames : [
+                    [0] : {
+                        columnNumber : 16
+                        functionName : 
+                        lineNumber : 0
+                        scriptId : <scriptId>
+                        url : 
+                    }
+                ]
+            }
+            text : Uncaught Error: myerror
+        }
+        timestamp : <timestamp>
+    }
+}
+{
+    method : Runtime.exceptionThrown
+    params : {
+        exceptionDetails : {
+            columnNumber : 2
+            exception : {
+                className : Error
+                description : Error: myerror2     at <anonymous>:2:19
+                objectId : <objectId>
+                preview : {
+                    description : Error: myerror2     at <anonymous>:2:19
+                    overflow : false
+                    properties : [
+                        [0] : {
+                            name : stack
+                            type : string
+                            value : Error: myerror2     at <anonymous>:2:19
+                        }
+                        [1] : {
+                            name : message
+                            type : string
+                            value : myerror2
+                        }
+                    ]
+                    subtype : error
+                    type : object
+                }
+                subtype : error
+                type : object
+            }
+            exceptionId : <exceptionId>
+            exceptionMetaData : {
+                foo2 : bar2
+            }
+            executionContextId : <executionContextId>
+            lineNumber : 1
+            stackTrace : {
+                callFrames : [
+                    [0] : {
+                        columnNumber : 18
+                        functionName : 
+                        lineNumber : 1
+                        scriptId : <scriptId>
+                        url : 
+                    }
+                ]
+            }
+            text : Uncaught Error: myerror2
+        }
+        timestamp : <timestamp>
+    }
+}

test/inspector/runtime/exception-thrown-metadata.js

@@ -0,0 +1,11 @@
+// Copyright 2021 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.
+
+let {session, contextGroup, Protocol} = InspectorTest.start("Check that exceptionThrown is supported by test runner.")
+
+Protocol.Runtime.enable();
+Protocol.Runtime.onExceptionThrown(message => InspectorTest.logMessage(message));
+contextGroup.addScript("throw inspector.newExceptionWithMetaData('myerror', 'foo', 'bar');");
+Protocol.Runtime.evaluate({ expression: "setTimeout(() => { \n  throw inspector.newExceptionWithMetaData('myerror2', 'foo2', 'bar2'); }, 0)" });
+InspectorTest.waitForPendingTasks().then(InspectorTest.completeTest);