Revert of [inspector] console get all information from inspector when needed...

Revert of [inspector] console get all information from inspector when needed (patchset #4 id:60001 of https://codereview.chromium.org/2784713002/ )

Reason for revert:
Breaks layout tests:
https://build.chromium.org/p/client.v8.fyi/builders/V8-Blink%20Linux%2064/builds/14569

Original issue's description:
> [inspector] console get all information from inspector when needed
>
> With this CL we don't need to store reference to InspectedContext inside of JavaScript console object and able to get all required information from callback data.
> It allows us to implement console methods without taking in account how and where we create and store these methods:
> - later we can move console object implementation to builtins..
> - ..and install command line API methods smarter.
>
> BUG=chromium:588893
> R=dgozman@chromium.org
>
> Review-Url: https://codereview.chromium.org/2784713002
> Cr-Commit-Position: refs/heads/master@{#44212}
> Committed: https://chromium.googlesource.com/v8/v8/+/908cd38123df33ce293e4c8d25e407f7ca915f4c

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

Review-Url: https://codereview.chromium.org/2784603003
Cr-Commit-Position: refs/heads/master@{#44217}

src/inspector/inspected-context.cc

@@ -14,6 +14,23 @@
 
 namespace v8_inspector {
 
+namespace {
+
+void clearContext(const v8::WeakCallbackInfo<v8::Global<v8::Context>>& data) {
+  // Inspected context is created in V8InspectorImpl::contextCreated method
+  // and destroyed in V8InspectorImpl::contextDestroyed.
+  // Both methods takes valid v8::Local<v8::Context> handle to the same context,
+  // it means that context is created before InspectedContext constructor and is
+  // always destroyed after InspectedContext destructor therefore this callback
+  // should be never called.
+  // It's possible only if inspector client doesn't call contextDestroyed which
+  // is considered an error.
+  CHECK(false);
+  data.GetParameter()->Reset();
+}
+
+}  // namespace
+
 InspectedContext::InspectedContext(V8InspectorImpl* inspector,
                                    const V8ContextInfo& info, int contextId)
     : m_inspector(inspector),
@@ -27,18 +44,27 @@ InspectedContext::InspectedContext(V8InspectorImpl* inspector,
   v8::Isolate* isolate = m_inspector->isolate();
   info.context->SetEmbedderData(static_cast<int>(v8::Context::kDebugIdIndex),
                                 v8::Int32::New(isolate, contextId));
+  m_context.SetWeak(&m_context, &clearContext,
+                    v8::WeakCallbackType::kParameter);
+
   v8::Local<v8::Object> global = info.context->Global();
   v8::Local<v8::Object> console =
       V8Console::createConsole(this, info.hasMemoryOnConsole);
   if (!global
            ->Set(info.context, toV8StringInternalized(isolate, "console"),
                  console)
-           .FromMaybe(false)) {
+           .FromMaybe(false))
     return;
-  }
+  m_console.Reset(isolate, console);
+  m_console.SetWeak();
 }
 
 InspectedContext::~InspectedContext() {
+  if (!m_console.IsEmpty()) {
+    v8::HandleScope scope(isolate());
+    V8Console::clearInspectedContextIfNeeded(context(),
+                                             m_console.Get(isolate()));
+  }
 }
 
 // static

src/inspector/inspected-context.h

@@ -53,6 +53,7 @@ class InspectedContext {
   const String16 m_auxData;
   bool m_reported;
   std::unique_ptr<InjectedScript> m_injectedScript;
+  v8::Global<v8::Object> m_console;
 
   DISALLOW_COPY_AND_ASSIGN(InspectedContext);
 };

src/inspector/v8-console-message.cc

@@ -353,11 +353,15 @@ ConsoleAPIType V8ConsoleMessage::type() const { return m_type; }
 
 // static
 std::unique_ptr<V8ConsoleMessage> V8ConsoleMessage::createForConsoleAPI(
-    v8::Local<v8::Context> v8Context, int contextId, int groupId,
-    V8InspectorImpl* inspector, double timestamp, ConsoleAPIType type,
+    double timestamp, ConsoleAPIType type,
     const std::vector<v8::Local<v8::Value>>& arguments,
-    std::unique_ptr<V8StackTraceImpl> stackTrace) {
-  v8::Isolate* isolate = v8Context->GetIsolate();
+    std::unique_ptr<V8StackTraceImpl> stackTrace,
+    InspectedContext* inspectedContext) {
+  v8::Isolate* isolate = inspectedContext->isolate();
+  int contextId = inspectedContext->contextId();
+  int contextGroupId = inspectedContext->contextGroupId();
+  V8InspectorImpl* inspector = inspectedContext->inspector();
+  v8::Local<v8::Context> context = inspectedContext->context();
 
   std::unique_ptr<V8ConsoleMessage> message(
       new V8ConsoleMessage(V8MessageOrigin::kConsole, timestamp, String16()));
@@ -376,8 +380,7 @@ std::unique_ptr<V8ConsoleMessage> V8ConsoleMessage::createForConsoleAPI(
         v8::debug::EstimatedValueSize(isolate, arguments.at(i));
   }
   if (arguments.size())
-    message->m_message =
-        V8ValueStringBuilder::toString(arguments[0], v8Context);
+    message->m_message = V8ValueStringBuilder::toString(arguments[0], context);
 
   v8::Isolate::MessageErrorLevel clientLevel = v8::Isolate::kMessageInfo;
   if (type == ConsoleAPIType::kDebug || type == ConsoleAPIType::kCount ||
@@ -394,7 +397,7 @@ std::unique_ptr<V8ConsoleMessage> V8ConsoleMessage::createForConsoleAPI(
 
   if (type != ConsoleAPIType::kClear) {
     inspector->client()->consoleAPIMessage(
-        groupId, clientLevel, toStringView(message->m_message),
+        contextGroupId, clientLevel, toStringView(message->m_message),
         toStringView(message->m_url), message->m_lineNumber,
         message->m_columnNumber, message->m_stackTrace.get());
   }
@@ -487,37 +490,8 @@ void V8ConsoleMessageStorage::clear() {
   m_messages.clear();
   m_estimatedSize = 0;
   if (V8InspectorSessionImpl* session =
-          m_inspector->sessionForContextGroup(m_contextGroupId)) {
+          m_inspector->sessionForContextGroup(m_contextGroupId))
     session->releaseObjectGroup("console");
-  }
-  m_data.clear();
-}
-
-bool V8ConsoleMessageStorage::shouldReportDeprecationMessage(
-    int contextId, const String16& method) {
-  std::set<String16>& reportedDeprecationMessages =
-      m_data[contextId].m_reportedDeprecationMessages;
-  auto it = reportedDeprecationMessages.find(method);
-  if (it != reportedDeprecationMessages.end()) return false;
-  reportedDeprecationMessages.insert(it, method);
-  return true;
-}
-
-int V8ConsoleMessageStorage::count(int contextId, const String16& id) {
-  return ++m_data[contextId].m_count[id];
-}
-
-void V8ConsoleMessageStorage::time(int contextId, const String16& id) {
-  m_data[contextId].m_time[id] = m_inspector->client()->currentTimeMS();
-}
-
-double V8ConsoleMessageStorage::timeEnd(int contextId, const String16& id) {
-  std::map<String16, double>& time = m_data[contextId].m_time;
-  auto it = time.find(id);
-  if (it == time.end()) return 0.0;
-  double elapsed = m_inspector->client()->currentTimeMS() - it->second;
-  time.erase(it);
-  return elapsed;
 }
 
 void V8ConsoleMessageStorage::contextDestroyed(int contextId) {
@@ -526,8 +500,6 @@ void V8ConsoleMessageStorage::contextDestroyed(int contextId) {
     m_messages[i]->contextDestroyed(contextId);
     m_estimatedSize += m_messages[i]->estimatedSize();
   }
-  auto it = m_data.find(contextId);
-  if (it != m_data.end()) m_data.erase(contextId);
 }
 
 }  // namespace v8_inspector

src/inspector/v8-console-message.h

@@ -6,8 +6,6 @@
 #define V8_INSPECTOR_V8CONSOLEMESSAGE_H_
 
 #include <deque>
-#include <map>
-#include <set>
 #include "include/v8.h"
 #include "src/inspector/protocol/Console.h"
 #include "src/inspector/protocol/Forward.h"
@@ -46,10 +44,9 @@ class V8ConsoleMessage {
   ~V8ConsoleMessage();
 
   static std::unique_ptr<V8ConsoleMessage> createForConsoleAPI(
-      v8::Local<v8::Context> v8Context, int contextId, int groupId,
-      V8InspectorImpl* inspector, double timestamp, ConsoleAPIType,
+      double timestamp, ConsoleAPIType,
       const std::vector<v8::Local<v8::Value>>& arguments,
-      std::unique_ptr<V8StackTraceImpl>);
+      std::unique_ptr<V8StackTraceImpl>, InspectedContext*);
 
   static std::unique_ptr<V8ConsoleMessage> createForException(
       double timestamp, const String16& detailedMessage, const String16& url,
@@ -115,23 +112,11 @@ class V8ConsoleMessageStorage {
   void contextDestroyed(int contextId);
   void clear();
 
-  bool shouldReportDeprecationMessage(int contextId, const String16& method);
-  int count(int contextId, const String16& id);
-  void time(int contextId, const String16& id);
-  double timeEnd(int contextId, const String16& id);
-
  private:
   V8InspectorImpl* m_inspector;
   int m_contextGroupId;
   int m_estimatedSize = 0;
   std::deque<std::unique_ptr<V8ConsoleMessage>> m_messages;
-
-  struct PerContextData {
-    std::set<String16> m_reportedDeprecationMessages;
-    std::map<String16, int> m_count;
-    std::map<String16, double> m_time;
-  };
-  std::map<int, PerContextData> m_data;
 };
 
 }  // namespace v8_inspector

src/inspector/v8-console.h

@@ -19,6 +19,8 @@ class V8Console {
  public:
   static v8::Local<v8::Object> createConsole(InspectedContext*,
                                              bool hasMemoryAttribute);
+  static void clearInspectedContextIfNeeded(v8::Local<v8::Context>,
+                                            v8::Local<v8::Object> console);
   static v8::Local<v8::Object> createCommandLineAPI(InspectedContext*);
 
   class CommandLineAPIScope {

test/inspector/protocol-test.js

@@ -75,8 +75,6 @@ InspectorTest.logMessage = function(originalMessage)
     for (var key in object) {
       if (nonStableFields.has(key))
         object[key] = `<${key}>`;
-      else if (typeof object[key] === "string" && object[key].match(/\d+:\d+:\d+:debug/))
-        object[key] = object[key].replace(/\d+/, '<scriptId>');
       else if (typeof object[key] === "object")
         objects.push(object[key]);
     }

test/inspector/runtime/command-line-api.js

-// Copyright 2017 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.
-
-InspectorTest.log('Checks command line API.');
-
-InspectorTest.runAsyncTestSuite([
-  async function testKeys() {
-    InspectorTest.logMessage(await Protocol.Runtime.evaluate({
-      expression: 'keys', includeCommandLineAPI: true}));
-    InspectorTest.logMessage(await Protocol.Runtime.evaluate({
-      expression: 'keys({a : 1})', includeCommandLineAPI: true, returnByValue: true}));
-
-    Protocol.Runtime.evaluate({expression: 'this.keys = keys', includeCommandLineAPI: true});
-    InspectorTest.logMessage(await Protocol.Runtime.evaluate({
-      expression: 'this.keys({a : 1})', returnByValue: true}));
-  },
-
-  async function testInspect() {
-    InspectorTest.log(await Protocol.Runtime.evaluate({expression: 'inspect', includeCommandLineAPI: true}));
-    await Protocol.Runtime.enable();
-    Protocol.Runtime.onInspectRequested(InspectorTest.logMessage);
-    await Protocol.Runtime.evaluate({expression: 'inspect({})', includeCommandLineAPI: true});
-    await Protocol.Runtime.evaluate({expression: 'inspect(239)', includeCommandLineAPI: true});
-    await Protocol.Runtime.evaluate({expression: 'inspect(-0)', includeCommandLineAPI: true});
-    await Protocol.Runtime.evaluate({expression: 'copy(\'hello\')', includeCommandLineAPI: true});
-    InspectorTest.logMessage(await Protocol.Runtime.evaluate({expression: '$0', includeCommandLineAPI: true}));
-
-    Protocol.Runtime.evaluate({expression: 'this.inspect = inspect', includeCommandLineAPI: true});
-    await Protocol.Runtime.evaluate({expression: 'this.inspect({})'});
-
-    Protocol.Runtime.onInspectRequested(null);
-    await Protocol.Runtime.disable();
-  },
-
-  async function testEvaluationResult() {
-    InspectorTest.logMessage(await Protocol.Runtime.evaluate({expression: '$_', includeCommandLineAPI: true}));
-    await Protocol.Runtime.evaluate({expression: '42', objectGroup: 'console', includeCommandLineAPI: true});
-    InspectorTest.logMessage(await Protocol.Runtime.evaluate({expression: '$_', includeCommandLineAPI: true}));
-    await Protocol.Runtime.evaluate({expression: '239', includeCommandLineAPI: true});
-    InspectorTest.logMessage(await Protocol.Runtime.evaluate({expression: '$_', includeCommandLineAPI: true}));
-    await Protocol.Runtime.evaluate({expression: '-0', objectGroup: 'console', includeCommandLineAPI: true});
-    InspectorTest.logMessage(await Protocol.Runtime.evaluate({expression: '$_', includeCommandLineAPI: true}));
-    await Protocol.Runtime.evaluate({expression: '({})', objectGroup: 'console', includeCommandLineAPI: true});
-    InspectorTest.logMessage(await Protocol.Runtime.evaluate({expression: '$_', includeCommandLineAPI: true, returnByValue: true}));
-  },
-
-  async function testDebug() {
-    InspectorTest.setupScriptMap();
-    await Protocol.Debugger.enable();
-    InspectorTest.logMessage(await Protocol.Runtime.evaluate({expression: 'debug', includeCommandLineAPI: true}));
-    InspectorTest.logMessage(await Protocol.Runtime.evaluate({expression: 'undebug', includeCommandLineAPI: true}));
-    await Protocol.Runtime.evaluate({expression: 'function foo() {}'});
-    await Protocol.Runtime.evaluate({expression: 'debug(foo)', includeCommandLineAPI: true});
-    Protocol.Runtime.evaluate({ expression: 'foo()'});
-    let message = await Protocol.Debugger.oncePaused();
-    InspectorTest.logCallFrames(message.params.callFrames);
-    InspectorTest.logMessage(message.params.hitBreakpoints);
-    await Protocol.Debugger.resume();
-    await Protocol.Runtime.evaluate({expression: 'undebug(foo)', includeCommandLineAPI: true});
-    await Protocol.Runtime.evaluate({ expression: 'foo()'});
-
-    Protocol.Runtime.evaluate({
-      expression: 'this.debug = debug; this.undebug = undebug;', includeCommandLineAPI: true});
-    await Protocol.Runtime.evaluate({expression: 'this.debug(foo)'});
-    Protocol.Runtime.evaluate({ expression: 'foo()'});
-    message = await Protocol.Debugger.oncePaused();
-    InspectorTest.logCallFrames(message.params.callFrames);
-    InspectorTest.logMessage(message.params.hitBreakpoints);
-    await Protocol.Debugger.resume();
-    await Protocol.Runtime.evaluate({expression: 'this.undebug(foo)'});
-    await Protocol.Runtime.evaluate({expression: 'foo()'});
-
-    await Protocol.Debugger.disable();
-  },
-
-  async function testMonitor() {
-    await Protocol.Debugger.enable();
-    await Protocol.Runtime.enable();
-    Protocol.Runtime.onConsoleAPICalled(message => InspectorTest.log(message.params.args[0].value));
-    InspectorTest.logMessage(await Protocol.Runtime.evaluate({expression: 'monitor', includeCommandLineAPI: true}));
-    InspectorTest.logMessage(await Protocol.Runtime.evaluate({expression: 'unmonitor', includeCommandLineAPI: true}));
-    await Protocol.Runtime.evaluate({expression: 'function foo() {}'});
-
-    await Protocol.Runtime.evaluate({expression: 'monitor(foo)', includeCommandLineAPI: true});
-    Protocol.Runtime.evaluate({ expression: 'foo(); console.log(\'after first call\')'});
-    await Protocol.Runtime.evaluate({expression: 'unmonitor(foo)', includeCommandLineAPI: true});
-    await Protocol.Runtime.evaluate({ expression: 'foo()'});
-
-    Protocol.Runtime.evaluate({
-      expression: 'console.log(\'store functions..\'); this.monitor = monitor; this.unmonitor = unmonitor;', includeCommandLineAPI: true});
-    await Protocol.Runtime.evaluate({expression: 'this.monitor(foo)'});
-    Protocol.Runtime.evaluate({ expression: 'foo(); console.log(\'after first call\')'});
-    await Protocol.Runtime.evaluate({expression: 'this.unmonitor(foo)'});
-    await Protocol.Runtime.evaluate({ expression: 'foo()'});
-
-    Protocol.Runtime.onConsoleAPICalled(null);
-    await Protocol.Debugger.disable();
-    await Protocol.Runtime.disable();
-  },
-
-  async function testProfile() {
-    await Protocol.Profiler.enable();
-    InspectorTest.logMessage(await Protocol.Runtime.evaluate({expression: 'profile', includeCommandLineAPI: true}));
-    InspectorTest.logMessage(await Protocol.Runtime.evaluate({expression: 'profileEnd', includeCommandLineAPI: true}));
-
-    Protocol.Runtime.evaluate({expression: 'profile(42)', includeCommandLineAPI: true});
-    InspectorTest.logMessage(await Protocol.Profiler.onceConsoleProfileStarted());
-    Protocol.Runtime.evaluate({expression: 'profileEnd(42)', includeCommandLineAPI: true});
-    let message = await Protocol.Profiler.onceConsoleProfileFinished();
-    message.params.profile = '<profile>';
-    InspectorTest.logMessage(message);
-
-    Protocol.Runtime.evaluate({
-      expression: 'this.profile = profile; this.profileEnd = profileEnd;', includeCommandLineAPI: true});
-    Protocol.Runtime.evaluate({expression: 'this.profile(239)'});
-    InspectorTest.logMessage(await Protocol.Profiler.onceConsoleProfileStarted());
-    Protocol.Runtime.evaluate({expression: 'this.profileEnd(239)'});
-    message = await Protocol.Profiler.onceConsoleProfileFinished();
-    message.params.profile = '<profile>';
-    InspectorTest.logMessage(message);
-
-    await Protocol.Profiler.disable();
-  },
-
-  async function testDir() {
-    InspectorTest.logMessage(await Protocol.Runtime.evaluate({expression: 'dir', includeCommandLineAPI: true}));
-
-    await Protocol.Runtime.enable();
-    Protocol.Runtime.evaluate({expression: 'dir({})', includeCommandLineAPI: true});
-    InspectorTest.logMessage(await Protocol.Runtime.onceConsoleAPICalled());
-    Protocol.Runtime.evaluate({expression: 'dir(42)', includeCommandLineAPI: true});
-    InspectorTest.logMessage(await Protocol.Runtime.onceConsoleAPICalled());
-
-    Protocol.Runtime.evaluate({expression: 'this.dir = dir', includeCommandLineAPI: true});
-    Protocol.Runtime.evaluate({expression: 'this.dir({})'});
-    InspectorTest.logMessage(await Protocol.Runtime.onceConsoleAPICalled());
-    await Protocol.Runtime.disable();
-  },
-
-  async function testDirXML() {
-    InspectorTest.logMessage(await Protocol.Runtime.evaluate({expression: 'dirxml', includeCommandLineAPI: true}));
-
-    await Protocol.Runtime.enable();
-    Protocol.Runtime.evaluate({expression: 'dirxml({})', includeCommandLineAPI: true});
-    InspectorTest.logMessage(await Protocol.Runtime.onceConsoleAPICalled());
-    Protocol.Runtime.evaluate({expression: 'dirxml(42)', includeCommandLineAPI: true});
-    InspectorTest.logMessage(await Protocol.Runtime.onceConsoleAPICalled());
-    await Protocol.Runtime.disable();
-  },
-
-  async function testTable() {
-    InspectorTest.logMessage(await Protocol.Runtime.evaluate({expression: 'table', includeCommandLineAPI: true}));
-
-    await Protocol.Runtime.enable();
-    Protocol.Runtime.evaluate({expression: 'table({})', includeCommandLineAPI: true});
-    InspectorTest.logMessage(await Protocol.Runtime.onceConsoleAPICalled());
-    Protocol.Runtime.evaluate({expression: 'table(42)', includeCommandLineAPI: true});
-    InspectorTest.logMessage(await Protocol.Runtime.onceConsoleAPICalled());
-    await Protocol.Runtime.disable();
-  },
-
-  async function testClear() {
-    InspectorTest.logMessage(await Protocol.Runtime.evaluate({expression: 'clear', includeCommandLineAPI: true}));
-
-    await Protocol.Runtime.enable();
-    Protocol.Runtime.evaluate({expression: 'clear()', includeCommandLineAPI: true});
-    InspectorTest.logMessage(await Protocol.Runtime.onceConsoleAPICalled());
-
-    Protocol.Runtime.evaluate({expression: 'this.clear = clear', includeCommandLineAPI: true});
-    Protocol.Runtime.evaluate({expression: 'this.clear()'});
-    InspectorTest.logMessage(await Protocol.Runtime.onceConsoleAPICalled());
-    await Protocol.Runtime.disable();
-  }
-]);