Revert of DevTools: use explicit IPC messages for enabling/disabling tracing instead of intercepting protocol… (patchset #3 of https://codereview.chromium.org/511873002/)

Reason for revert:
Creates timeout failures on a lot of inspector/ tests in Blink:
http://build.chromium.org/p/chromium.webkit/builders/WebKit%20Linux%20%28deps%29/builds/33453/steps/webkit_tests/logs/stdio

Original issue's description:
> DevTools: use explicit IPC messages for enabling/disabling tracing instead of intercepting protocol events
>
> Added IPC plumbing that allows InspectorTracingAgent to enable/disable  browser-wide tracing. It was implemented by intercepting Tracing.started/Tracing.stopped events in the browser. Tracing.stopped event in the DevTools protocol was used by the renderer to notify the browser that it should stop trace event recording and it didn't make much sense for the client as there is already Tracing.tracingComplete event in the protocol.
>
> BUG=398787
>
> Committed: https://chromium.googlesource.com/chromium/src/+/a449adae35631e4791547bd0b14403d1766f9ddf

TBR=dcheng@chromium.org,caseq@chromium.org,loislo@chromium.org,vsevik@chromium.org,yurys@chromium.org
NOTREECHECKS=true
NOTRY=true
BUG=398787

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

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

8 files changed, 59 insertions, 37 deletions

diff --git a/content/browser/devtools/devtools_protocol.cc b/content/browser/devtools/devtools_protocol.cc
index e3d5314..c01f6f2 100644
--- a/content/browser/devtools/devtools_protocol.cc
+++ b/content/browser/devtools/devtools_protocol.cc
@@ -178,6 +178,15 @@ DevToolsProtocol::Handler::HandleCommand(
   return (it->second).Run(command);
 }
 
+void DevToolsProtocol::Handler::HandleNotification(
+    scoped_refptr<DevToolsProtocol::Notification> notification) {
+  NotificationHandlers::iterator it =
+    notification_handlers_.find(notification->method());
+  if (it == notification_handlers_.end())
+    return;
+  (it->second).Run(notification);
+}
+
 void DevToolsProtocol::Handler::SetNotifier(const Notifier& notifier) {
   notifier_ = notifier;
 }
@@ -191,6 +200,12 @@ void DevToolsProtocol::Handler::RegisterCommandHandler(
   command_handlers_[command] = handler;
 }
 
+void DevToolsProtocol::Handler::RegisterNotificationHandler(
+    const std::string& notification,
+    const NotificationHandler& handler) {
+  notification_handlers_[notification] = handler;
+}
+
 void DevToolsProtocol::Handler::SendNotification(
     const std::string& method,
     base::DictionaryValue* params) {
diff --git a/content/browser/devtools/devtools_protocol.h b/content/browser/devtools/devtools_protocol.h
index cbc075b..71c24ab 100644
--- a/content/browser/devtools/devtools_protocol.h
+++ b/content/browser/devtools/devtools_protocol.h
@@ -127,12 +127,18 @@ class DevToolsProtocol {
    public:
     typedef base::Callback<scoped_refptr<DevToolsProtocol::Response>(
         scoped_refptr<DevToolsProtocol::Command> command)> CommandHandler;
+    typedef base::Callback<void(
+        scoped_refptr<DevToolsProtocol::Notification> notification)>
+            NotificationHandler;
 
     virtual ~Handler();
 
     virtual scoped_refptr<DevToolsProtocol::Response> HandleCommand(
         scoped_refptr<DevToolsProtocol::Command> command);
 
+    virtual void HandleNotification(
+        scoped_refptr<DevToolsProtocol::Notification> notification);
+
     void SetNotifier(const Notifier& notifier);
 
    protected:
@@ -141,6 +147,9 @@ class DevToolsProtocol {
     void RegisterCommandHandler(const std::string& command,
                                 const CommandHandler& handler);
 
+    void RegisterNotificationHandler(const std::string& notification,
+                                     const NotificationHandler& handler);
+
     // Sends notification to the client. Takes ownership of |params|.
     void SendNotification(const std::string& method,
                           base::DictionaryValue* params);
@@ -153,9 +162,11 @@ class DevToolsProtocol {
 
    private:
     typedef std::map<std::string, CommandHandler> CommandHandlers;
+    typedef std::map<std::string, NotificationHandler> NotificationHandlers;
 
     Notifier notifier_;
     CommandHandlers command_handlers_;
+    NotificationHandlers notification_handlers_;
 
     DISALLOW_COPY_AND_ASSIGN(Handler);
   };
diff --git a/content/browser/devtools/devtools_tracing_handler.cc b/content/browser/devtools/devtools_tracing_handler.cc
index 238cf24..de81bf3 100644
--- a/content/browser/devtools/devtools_tracing_handler.cc
+++ b/content/browser/devtools/devtools_tracing_handler.cc
@@ -65,6 +65,12 @@ DevToolsTracingHandler::DevToolsTracingHandler(
   RegisterCommandHandler(devtools::Tracing::getCategories::kName,
                          base::Bind(&DevToolsTracingHandler::OnGetCategories,
                                     base::Unretained(this)));
+  RegisterNotificationHandler(devtools::Tracing::started::kName,
+                         base::Bind(&DevToolsTracingHandler::OnTracingStarted,
+                                    base::Unretained(this)));
+  RegisterNotificationHandler(devtools::Tracing::stopped::kName,
+                         base::Bind(&DevToolsTracingHandler::OnTracingStopped,
+                                    base::Unretained(this)));
 }
 
 DevToolsTracingHandler::~DevToolsTracingHandler() {
@@ -148,11 +154,6 @@ base::debug::TraceOptions DevToolsTracingHandler::TraceOptionsFromString(
 scoped_refptr<DevToolsProtocol::Response>
 DevToolsTracingHandler::OnStart(
     scoped_refptr<DevToolsProtocol::Command> command) {
-  // If inspected target is a render process Tracing.start will be handled by
-  // tracing agent in the renderer.
-  if (target_ == Renderer)
-    return NULL;
-
   is_recording_ = true;
 
   std::string categories;
@@ -174,6 +175,16 @@ DevToolsTracingHandler::OnStart(
 
   SetupTimer(usage_reporting_interval);
 
+  // If inspected target is a render process Tracing.start will be handled by
+  // tracing agent in the renderer.
+  if (target_ == Renderer) {
+    TracingController::GetInstance()->EnableRecording(
+        base::debug::CategoryFilter(categories),
+        options,
+        TracingController::EnableRecordingDoneCallback());
+    return NULL;
+  }
+
   TracingController::GetInstance()->EnableRecording(
       base::debug::CategoryFilter(categories),
       options,
@@ -217,10 +228,6 @@ void DevToolsTracingHandler::OnBufferUsage(float usage) {
 scoped_refptr<DevToolsProtocol::Response>
 DevToolsTracingHandler::OnEnd(
     scoped_refptr<DevToolsProtocol::Command> command) {
-  // If inspected target is a render process Tracing.end will be handled by
-  // tracing agent in the renderer.
-  if (target_ == Renderer)
-    return NULL;
   DisableRecording(
       base::Bind(&DevToolsTracingHandler::BeginReadingRecordingResult,
                  weak_factory_.GetWeakPtr()));
@@ -265,7 +272,8 @@ void DevToolsTracingHandler::OnCategoriesReceived(
   SendAsyncResponse(command->SuccessResponse(response));
 }
 
-void DevToolsTracingHandler::EnableTracing(const std::string& category_filter) {
+void DevToolsTracingHandler::OnTracingStarted(
+    scoped_refptr<DevToolsProtocol::Notification> notification) {
   if (is_recording_)
     return;
   is_recording_ = true;
@@ -273,13 +281,13 @@ void DevToolsTracingHandler::EnableTracing(const std::string& category_filter) {
   SetupTimer(kDefaultReportingInterval);
 
   TracingController::GetInstance()->EnableRecording(
-      base::debug::CategoryFilter(category_filter),
+      base::debug::CategoryFilter(kDefaultCategories),
       base::debug::TraceOptions(),
       TracingController::EnableRecordingDoneCallback());
-  SendNotification(devtools::Tracing::started::kName, NULL);
 }
 
-void DevToolsTracingHandler::DisableTracing() {
+void DevToolsTracingHandler::OnTracingStopped(
+    scoped_refptr<DevToolsProtocol::Notification> notification) {
   if (!is_recording_)
     return;
   is_recording_ = false;
diff --git a/content/browser/devtools/devtools_tracing_handler.h b/content/browser/devtools/devtools_tracing_handler.h
index fe270bd..20ebca8 100644
--- a/content/browser/devtools/devtools_tracing_handler.h
+++ b/content/browser/devtools/devtools_tracing_handler.h
@@ -30,9 +30,6 @@ class DevToolsTracingHandler : public DevToolsProtocol::Handler {
 
   void OnClientDetached();
 
-  void EnableTracing(const std::string& category_filter);
-  void DisableTracing();
-
  private:
   void BeginReadingRecordingResult(const base::FilePath& path);
   void ReadRecordingResult(const scoped_refptr<base::RefCountedString>& result);
@@ -48,6 +45,12 @@ class DevToolsTracingHandler : public DevToolsProtocol::Handler {
   scoped_refptr<DevToolsProtocol::Response> OnGetCategories(
       scoped_refptr<DevToolsProtocol::Command> command);
 
+  void OnTracingStarted(
+      scoped_refptr<DevToolsProtocol::Notification> notification);
+
+  void OnTracingStopped(
+      scoped_refptr<DevToolsProtocol::Notification> notification);
+
   void OnCategoriesReceived(scoped_refptr<DevToolsProtocol::Command> command,
                             const std::set<std::string>& category_set);
 
diff --git a/content/browser/devtools/render_view_devtools_agent_host.cc b/content/browser/devtools/render_view_devtools_agent_host.cc
index a858ff8..12a05d5 100644
--- a/content/browser/devtools/render_view_devtools_agent_host.cc
+++ b/content/browser/devtools/render_view_devtools_agent_host.cc
@@ -441,8 +441,6 @@ bool RenderViewDevToolsAgentHost::DispatchIPCMessage(
                         OnDispatchOnInspectorFrontend)
     IPC_MESSAGE_HANDLER(DevToolsHostMsg_SaveAgentRuntimeState,
                         OnSaveAgentRuntimeState)
-    IPC_MESSAGE_HANDLER(DevToolsHostMsg_EnableTracing, OnEnableTracing)
-    IPC_MESSAGE_HANDLER(DevToolsHostMsg_DisableTracing, OnDisableTracing)
     IPC_MESSAGE_HANDLER_GENERIC(ViewHostMsg_SwapCompositorFrame,
                                 handled = false; OnSwapCompositorFrame(msg))
     IPC_MESSAGE_UNHANDLED(handled = false)
@@ -476,16 +474,14 @@ void RenderViewDevToolsAgentHost::OnDispatchOnInspectorFrontend(
     const std::string& message) {
   if (!render_view_host_)
     return;
-  SendMessageToClient(message);
-}
 
-void RenderViewDevToolsAgentHost::OnEnableTracing(
-    const std::string& category_filter) {
-  tracing_handler_->EnableTracing(category_filter);
-}
+  scoped_refptr<DevToolsProtocol::Notification> notification =
+      DevToolsProtocol::ParseNotification(message);
 
-void RenderViewDevToolsAgentHost::OnDisableTracing() {
-  tracing_handler_->DisableTracing();
+  if (notification.get()) {
+    tracing_handler_->HandleNotification(notification);
+  }
+  SendMessageToClient(message);
 }
 
 }  // namespace content
diff --git a/content/browser/devtools/render_view_devtools_agent_host.h b/content/browser/devtools/render_view_devtools_agent_host.h
index 8657bfd..7b35388 100644
--- a/content/browser/devtools/render_view_devtools_agent_host.h
+++ b/content/browser/devtools/render_view_devtools_agent_host.h
@@ -97,8 +97,6 @@ class CONTENT_EXPORT RenderViewDevToolsAgentHost
 
   void OnDispatchOnInspectorFrontend(const std::string& message);
   void OnSaveAgentRuntimeState(const std::string& state);
-  void OnEnableTracing(const std::string& category_filter);
-  void OnDisableTracing();
 
   void ClientDetachedFromRenderer();
 
diff --git a/content/common/devtools_messages.h b/content/common/devtools_messages.h
index 151d88f..3e14864 100644
--- a/content/common/devtools_messages.h
+++ b/content/common/devtools_messages.h
@@ -114,13 +114,6 @@ IPC_MESSAGE_ROUTED1(DevToolsHostMsg_DispatchOnEmbedder,
 IPC_MESSAGE_ROUTED1(DevToolsHostMsg_SaveAgentRuntimeState,
                     std::string /* state */)
 
-// Tells the host to enable trace event recording.
-IPC_MESSAGE_ROUTED1(DevToolsHostMsg_EnableTracing,
-                    std::string /* category_filter */)
-
-// Tells the host to disable trace event recording.
-IPC_MESSAGE_ROUTED0(DevToolsHostMsg_DisableTracing)
-
 //-----------------------------------------------------------------------------
 // These are messages sent from the GPU process to the inspected renderer.
 
diff --git a/content/renderer/devtools/devtools_agent.cc b/content/renderer/devtools/devtools_agent.cc
index 71f55bb..7cf4676 100644
--- a/content/renderer/devtools/devtools_agent.cc
+++ b/content/renderer/devtools/devtools_agent.cc
@@ -171,7 +171,6 @@ void DevToolsAgent::setTraceEventCallback(const WebString& category_filter,
 }
 
 void DevToolsAgent::enableTracing(const WebString& category_filter) {
-  Send(new DevToolsHostMsg_EnableTracing(routing_id(), category_filter.utf8()));
   TraceLog* trace_log = TraceLog::GetInstance();
   trace_log->SetEnabled(base::debug::CategoryFilter(category_filter.utf8()),
                         TraceLog::RECORDING_MODE,
@@ -180,7 +179,6 @@ void DevToolsAgent::enableTracing(const WebString& category_filter) {
 
 void DevToolsAgent::disableTracing() {
   TraceLog::GetInstance()->SetDisabled();
-  Send(new DevToolsHostMsg_DisableTracing(routing_id()));
 }
 
 // static