Reland of DevTools: remove V8ScriptRunner use from InjectedScript*. (patchset...

Reland of DevTools: remove V8ScriptRunner use from InjectedScript*. (patchset #1 id:1 of https://codereview.chromium.org/1646803002/ )

Reason for revert:
The test you reverted it for is flaky: http://test-results.appspot.com/dashboards/flakiness_dashboard.html#tests=http/tests/inspector/resource-tree/resource-tree-non-unique-url.html

You should mute it and file a bug.

Original issue's description:
> Revert of DevTools: remove V8ScriptRunner use from InjectedScript*. (patchset #2 id:20001 of https://codereview.chromium.org/1641933002/ )
>
> Reason for revert:
> http/tests/inspector/resource-tree/resource-tree-non-unique-url.html started failing
>
> https://build.chromium.org/p/chromium.webkit/builders/WebKit%20Linux%2032/bui...
>
> Reverting to see if we can get that test to pass again.
>
> Original issue's description:
> > DevTools: remove V8ScriptRunner use from InjectedScript*.
> >
> > BUG=580337
> >
> > Committed: https://crrev.com/ec279b9d63c3a96d237495967faba97be76597eb
> > Cr-Commit-Position: refs/heads/master@{#372020}
>
> TBR=dgozman@chromium.org,jochen@chromium.org,pfeldman@chromium.org
> # Skipping CQ checks because original CL landed less than 1 days ago.
> NOPRESUBMIT=true
> NOTREECHECKS=true
> NOTRY=true
> BUG=580337
>
> Committed: https://crrev.com/718abf542ea95a35036331f3c78caf21959155dc
> Cr-Commit-Position: refs/heads/master@{#372034}

TBR=dgozman@chromium.org,jochen@chromium.org,noel@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=580337

Review URL: https://codereview.chromium.org/1645663004

Cr-Commit-Position: refs/heads/master@{#372090}

third_party/WebKit/Source/bindings/core/v8/custom/V8InjectedScriptManager.cpp

-/*
- * Copyright (C) 2012 Google Inc. All rights reserved.
- *
- * Redistribution and use in source and binary forms, with or without
- * modification, are permitted provided that the following conditions are
- * met:
- *
- *     * Redistributions of source code must retain the above copyright
- * notice, this list of conditions and the following disclaimer.
- *     * Redistributions in binary form must reproduce the above
- * copyright notice, this list of conditions and the following disclaimer
- * in the documentation and/or other materials provided with the
- * distribution.
- *     * Neither the name of Google Inc. nor the names of its
- * contributors may be used to endorse or promote products derived from
- * this software without specific prior written permission.
- *
- * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
- * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
- * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
- * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
- * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
- * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
- * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
- * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
- * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
- * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
- * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
- */
-
-#include "core/inspector/InjectedScriptManager.h"
-
-#include "bindings/core/v8/V8Binding.h"
-#include "bindings/core/v8/V8ScriptRunner.h"
-#include "core/inspector/InjectedScriptHost.h"
-#include "core/inspector/InjectedScriptNative.h"
-#include "core/inspector/v8/V8InjectedScriptHost.h"
-#include "wtf/RefPtr.h"
-
-namespace blink {
-
-v8::Local<v8::Object> InjectedScriptManager::createInjectedScript(const String& scriptSource, v8::Local<v8::Context> context, int id, InjectedScriptNative* injectedScriptNative)
-{
-    v8::Isolate* isolate = context->GetIsolate();
-    v8::Context::Scope scope(context);
-
-    v8::Local<v8::FunctionTemplate> wrapperTemplate = m_injectedScriptHost->wrapperTemplate(isolate);
-    if (wrapperTemplate.IsEmpty()) {
-        wrapperTemplate = V8InjectedScriptHost::createWrapperTemplate(isolate);
-        m_injectedScriptHost->setWrapperTemplate(wrapperTemplate, isolate);
-    }
-
-    v8::Local<v8::Object> scriptHostWrapper = V8InjectedScriptHost::wrap(wrapperTemplate, context, m_injectedScriptHost);
-    if (scriptHostWrapper.IsEmpty())
-        return v8::Local<v8::Object>();
-
-    injectedScriptNative->setOnInjectedScriptHost(scriptHostWrapper);
-
-    // Inject javascript into the context. The compiled script is supposed to evaluate into
-    // a single anonymous function(it's anonymous to avoid cluttering the global object with
-    // inspector's stuff) the function is called a few lines below with InjectedScriptHost wrapper,
-    // injected script id and explicit reference to the inspected global object. The function is expected
-    // to create and configure InjectedScript instance that is going to be used by the inspector.
-    v8::Local<v8::Value> value;
-    if (!V8ScriptRunner::compileAndRunInternalScript(v8String(isolate, scriptSource), isolate).ToLocal(&value))
-        return v8::Local<v8::Object>();
-    ASSERT(value->IsFunction());
-
-    v8::Local<v8::Object> windowGlobal = context->Global();
-    v8::Local<v8::Value> info[] = { scriptHostWrapper, windowGlobal, v8::Number::New(context->GetIsolate(), id) };
-    v8::Local<v8::Value> injectedScriptValue;
-    if (!V8ScriptRunner::callInternalFunction(v8::Local<v8::Function>::Cast(value), windowGlobal, WTF_ARRAY_LENGTH(info), info, context->GetIsolate()).ToLocal(&injectedScriptValue))
-        return v8::Local<v8::Object>();
-    if (!injectedScriptValue->IsObject())
-        return v8::Local<v8::Object>();
-    return injectedScriptValue.As<v8::Object>();
-}
-
-} // namespace blink

third_party/WebKit/Source/bindings/core/v8/custom/custom.gypi

@@ -15,7 +15,6 @@
             'V8EventTargetCustom.cpp',
             'V8HTMLAllCollectionCustom.cpp',
             'V8HTMLPlugInElementCustom.cpp',
-            'V8InjectedScriptManager.cpp',
             'V8IntersectionObserverCustom.cpp',
             'V8MediaQueryListCustom.cpp',
             'V8MessageChannelCustom.cpp',

third_party/WebKit/Source/core/inspector/InjectedScriptManager.cpp

@@ -30,12 +30,14 @@
 
 #include "core/inspector/InjectedScriptManager.h"
 
+#include "bindings/core/v8/V8Binding.h"
 #include "core/inspector/InjectedScript.h"
 #include "core/inspector/InjectedScriptHost.h"
 #include "core/inspector/InjectedScriptNative.h"
 #include "core/inspector/RemoteObjectId.h"
 #include "core/inspector/v8/V8Debugger.h"
 #include "core/inspector/v8/V8DebuggerClient.h"
+#include "core/inspector/v8/V8InjectedScriptHost.h"
 #include "public/platform/Platform.h"
 #include "public/platform/WebData.h"
 #include "wtf/PassOwnPtr.h"
@@ -119,12 +121,6 @@ void InjectedScriptManager::setCustomObjectFormatterEnabled(bool enabled)
     }
 }
 
-String InjectedScriptManager::injectedScriptSource()
-{
-    const WebData& injectedScriptSourceResource = Platform::current()->loadResource("InjectedScriptSource.js");
-    return String(injectedScriptSourceResource.data(), injectedScriptSourceResource.size());
-}
-
 InjectedScript* InjectedScriptManager::injectedScriptFor(v8::Local<v8::Context> context)
 {
     v8::Context::Scope scope(context);
@@ -139,7 +135,11 @@ InjectedScript* InjectedScriptManager::injectedScriptFor(v8::Local<v8::Context>
         return nullptr;
 
     RefPtr<InjectedScriptNative> injectedScriptNative = adoptRef(new InjectedScriptNative(context->GetIsolate()));
-    v8::Local<v8::Object> object = createInjectedScript(injectedScriptSource(), context, contextId, injectedScriptNative.get());
+
+    const WebData& injectedScriptSourceResource = Platform::current()->loadResource("InjectedScriptSource.js");
+    String injectedScriptSource(injectedScriptSourceResource.data(), injectedScriptSourceResource.size());
+
+    v8::Local<v8::Object> object = createInjectedScript(injectedScriptSource, context, contextId, injectedScriptNative.get());
     OwnPtr<InjectedScript> result = adoptPtr(new InjectedScript(this, context, object, m_client, injectedScriptNative.release(), contextId));
     InjectedScript* resultPtr = result.get();
     if (m_customObjectFormatterEnabled)
@@ -148,4 +148,41 @@ InjectedScript* InjectedScriptManager::injectedScriptFor(v8::Local<v8::Context>
     return resultPtr;
 }
 
+v8::Local<v8::Object> InjectedScriptManager::createInjectedScript(const String& scriptSource, v8::Local<v8::Context> context, int id, InjectedScriptNative* injectedScriptNative)
+{
+    v8::Isolate* isolate = context->GetIsolate();
+    v8::Context::Scope scope(context);
+
+    v8::Local<v8::FunctionTemplate> wrapperTemplate = m_injectedScriptHost->wrapperTemplate(isolate);
+    if (wrapperTemplate.IsEmpty()) {
+        wrapperTemplate = V8InjectedScriptHost::createWrapperTemplate(isolate);
+        m_injectedScriptHost->setWrapperTemplate(wrapperTemplate, isolate);
+    }
+
+    v8::Local<v8::Object> scriptHostWrapper = V8InjectedScriptHost::wrap(wrapperTemplate, context, m_injectedScriptHost);
+    if (scriptHostWrapper.IsEmpty())
+        return v8::Local<v8::Object>();
+
+    injectedScriptNative->setOnInjectedScriptHost(scriptHostWrapper);
+
+    // Inject javascript into the context. The compiled script is supposed to evaluate into
+    // a single anonymous function(it's anonymous to avoid cluttering the global object with
+    // inspector's stuff) the function is called a few lines below with InjectedScriptHost wrapper,
+    // injected script id and explicit reference to the inspected global object. The function is expected
+    // to create and configure InjectedScript instance that is going to be used by the inspector.
+    v8::Local<v8::Value> value;
+    if (!m_client->compileAndRunInternalScript(scriptSource).ToLocal(&value))
+        return v8::Local<v8::Object>();
+    ASSERT(value->IsFunction());
+
+    v8::Local<v8::Object> windowGlobal = context->Global();
+    v8::Local<v8::Value> info[] = { scriptHostWrapper, windowGlobal, v8::Number::New(context->GetIsolate(), id) };
+    v8::Local<v8::Value> injectedScriptValue;
+    if (!m_client->callInternalFunction(v8::Local<v8::Function>::Cast(value), windowGlobal, WTF_ARRAY_LENGTH(info), info).ToLocal(&injectedScriptValue))
+        return v8::Local<v8::Object>();
+    if (!injectedScriptValue->IsObject())
+        return v8::Local<v8::Object>();
+    return injectedScriptValue.As<v8::Object>();
+}
+
 } // namespace blink

third_party/WebKit/Source/core/inspector/InjectedScriptManager.h

@@ -67,7 +67,6 @@ public:
 private:
     explicit InjectedScriptManager(V8DebuggerClient*);
 
-    String injectedScriptSource();
     v8::Local<v8::Object> createInjectedScript(const String& source, v8::Local<v8::Context>, int id, InjectedScriptNative*);
 
     typedef HashMap<int, OwnPtr<InjectedScript>> IdToInjectedScriptMap;

third_party/WebKit/Source/core/inspector/InjectedScriptNative.cpp

@@ -4,8 +4,6 @@
 
 #include "core/inspector/InjectedScriptNative.h"
 
-#include "bindings/core/v8/ScriptState.h"
-#include "bindings/core/v8/V8HiddenValue.h"
 #include "platform/JSONValues.h"
 #include "wtf/Vector.h"
 #include "wtf/text/WTFString.h"
@@ -19,21 +17,26 @@ InjectedScriptNative::InjectedScriptNative(v8::Isolate* isolate)
 {
 }
 
+static const char privateKeyName[] = "v8-inspector#injectedScript";
+
 InjectedScriptNative::~InjectedScriptNative() { }
 
 void InjectedScriptNative::setOnInjectedScriptHost(v8::Local<v8::Object> injectedScriptHost)
 {
     v8::HandleScope handleScope(m_isolate);
     v8::Local<v8::External> external = v8::External::New(m_isolate, this);
-    V8HiddenValue::setHiddenValue(ScriptState::current(m_isolate), injectedScriptHost, V8HiddenValue::injectedScriptNative(m_isolate), external);
+    v8::Local<v8::Private> privateKey = v8::Private::ForApi(m_isolate, v8::String::NewFromUtf8(m_isolate, privateKeyName));
+    injectedScriptHost->SetPrivate(m_isolate->GetCurrentContext(), privateKey, external);
 }
 
 InjectedScriptNative* InjectedScriptNative::fromInjectedScriptHost(v8::Local<v8::Object> injectedScriptObject)
 {
     v8::Isolate* isolate = injectedScriptObject->GetIsolate();
     v8::HandleScope handleScope(isolate);
-    v8::Local<v8::Value> value = V8HiddenValue::getHiddenValue(ScriptState::current(isolate), injectedScriptObject, V8HiddenValue::injectedScriptNative(isolate));
-    ASSERT(!value.IsEmpty());
+    v8::Local<v8::Context> context = isolate->GetCurrentContext();
+    v8::Local<v8::Private> privateKey = v8::Private::ForApi(isolate, v8::String::NewFromUtf8(isolate, privateKeyName));
+    v8::Local<v8::Value> value;
+    RELEASE_ASSERT(injectedScriptObject->GetPrivate(context, privateKey).ToLocal(&value));
     v8::Local<v8::External> external = value.As<v8::External>();
     void* ptr = external->Value();
     ASSERT(ptr);

third_party/WebKit/Source/core/inspector/ThreadDebugger.cpp

@@ -7,9 +7,6 @@
 #include "bindings/core/v8/V8Binding.h"
 #include "bindings/core/v8/V8ScriptRunner.h"
 #include "core/inspector/InspectorDOMDebuggerAgent.h"
-#include "public/platform/Platform.h"
-#include "public/platform/WebData.h"
-
 
 namespace blink {
 
@@ -23,26 +20,24 @@ ThreadDebugger::~ThreadDebugger()
 {
 }
 
-v8::Local<v8::Object> ThreadDebugger::compileDebuggerScript()
+void ThreadDebugger::eventListeners(v8::Local<v8::Value> value, EventListenerInfoMap& result)
 {
-    const WebData& debuggerScriptSourceResource = Platform::current()->loadResource("DebuggerScriptSource.js");
-    v8::Local<v8::String> source = v8String(m_isolate, String(debuggerScriptSourceResource.data(), debuggerScriptSourceResource.size()));
-    v8::Local<v8::Value> value;
-    if (!V8ScriptRunner::compileAndRunInternalScript(source, m_isolate).ToLocal(&value))
-        return v8::Local<v8::Object>();
-    ASSERT(value->IsObject());
-    return value.As<v8::Object>();
+    InspectorDOMDebuggerAgent::eventListenersInfoForTarget(m_isolate, value, result);
 }
 
-void ThreadDebugger::eventListeners(v8::Local<v8::Value> value, EventListenerInfoMap& result)
+v8::MaybeLocal<v8::Value> ThreadDebugger::compileAndRunInternalScript(const String& script)
 {
-    InspectorDOMDebuggerAgent::eventListenersInfoForTarget(m_isolate, value, result);
+    return V8ScriptRunner::compileAndRunInternalScript(v8String(m_isolate, script), m_isolate);
 }
 
 v8::MaybeLocal<v8::Value> ThreadDebugger::callFunction(v8::Local<v8::Function> function, v8::Local<v8::Context> context, v8::Local<v8::Value> receiver, int argc, v8::Local<v8::Value> info[])
 {
-    ScriptState* scriptState = ScriptState::from(context);
-    return V8ScriptRunner::callFunction(function, scriptState->executionContext(), receiver, argc, info, m_isolate);
+    return V8ScriptRunner::callFunction(function, toExecutionContext(context), receiver, argc, info, m_isolate);
+}
+
+v8::MaybeLocal<v8::Value> ThreadDebugger::callInternalFunction(v8::Local<v8::Function> function, v8::Local<v8::Value> receiver, int argc, v8::Local<v8::Value> info[])
+{
+    return V8ScriptRunner::callInternalFunction(function, receiver, argc, info, m_isolate);
 }
 
 } // namespace blink

third_party/WebKit/Source/core/inspector/ThreadDebugger.h

@@ -23,9 +23,10 @@ public:
     ~ThreadDebugger() override;
 
     // V8DebuggerClient implementation.
-    v8::Local<v8::Object> compileDebuggerScript() override;
     void eventListeners(v8::Local<v8::Value>, EventListenerInfoMap&) override;
+    v8::MaybeLocal<v8::Value> compileAndRunInternalScript(const String& script) override;
     v8::MaybeLocal<v8::Value> callFunction(v8::Local<v8::Function>, v8::Local<v8::Context>, v8::Local<v8::Value> receiver, int argc, v8::Local<v8::Value> info[]) override;
+    v8::MaybeLocal<v8::Value> callInternalFunction(v8::Local<v8::Function>, v8::Local<v8::Value> receiver, int argc, v8::Local<v8::Value> info[]) override;
 
     V8Debugger* debugger() const { return m_debugger.get(); }
 

third_party/WebKit/Source/core/inspector/v8/V8DebuggerClient.h

@@ -16,12 +16,15 @@ class CORE_EXPORT V8DebuggerClient {
     USING_FAST_MALLOC(V8DebuggerClient);
 public:
     virtual ~V8DebuggerClient() { }
-    virtual v8::Local<v8::Object> compileDebuggerScript() = 0;
     virtual void runMessageLoopOnPause(int contextGroupId) = 0;
     virtual void quitMessageLoopOnPause() = 0;
     virtual void eventListeners(v8::Local<v8::Value>, EventListenerInfoMap&) = 0;
     virtual bool callingContextCanAccessContext(v8::Local<v8::Context> calling, v8::Local<v8::Context> target) = 0;
+
+    virtual v8::MaybeLocal<v8::Value> compileAndRunInternalScript(const String& script) = 0;
     virtual v8::MaybeLocal<v8::Value> callFunction(v8::Local<v8::Function>, v8::Local<v8::Context>, v8::Local<v8::Value> receiver, int argc, v8::Local<v8::Value> info[]) = 0;
+    virtual v8::MaybeLocal<v8::Value> callInternalFunction(v8::Local<v8::Function>, v8::Local<v8::Value> receiver, int argc, v8::Local<v8::Value> info[]) = 0;
+
 };
 
 } // namespace blink

third_party/WebKit/Source/core/inspector/v8/V8DebuggerImpl.cpp

@@ -38,6 +38,8 @@
 #include "core/inspector/v8/V8DebuggerClient.h"
 #include "core/inspector/v8/V8JavaScriptCallFrame.h"
 #include "platform/JSONValues.h"
+#include "public/platform/Platform.h"
+#include "public/platform/WebData.h"
 #include "wtf/Atomics.h"
 #include "wtf/Vector.h"
 #include "wtf/text/CString.h"
@@ -710,10 +712,13 @@ void V8DebuggerImpl::compileDebuggerScript()
 
     v8::HandleScope scope(m_isolate);
     v8::Context::Scope contextScope(debuggerContext());
-    v8::Local<v8::Object> value = m_client->compileDebuggerScript();
-    if (value.IsEmpty())
+    const WebData& debuggerScriptSourceResource = Platform::current()->loadResource("DebuggerScriptSource.js");
+    String source(debuggerScriptSourceResource.data(), debuggerScriptSourceResource.size());
+    v8::Local<v8::Value> value;
+    if (!m_client->compileAndRunInternalScript(source).ToLocal(&value))
         return;
-    m_debuggerScript.Reset(m_isolate, value);
+    ASSERT(value->IsObject());
+    m_debuggerScript.Reset(m_isolate, value.As<v8::Object>());
 }
 
 v8::Local<v8::Context> V8DebuggerImpl::debuggerContext() const