Revert of DevTools: use explicit IPC messages for enabling/disabling tracing...

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}

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) {

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);
   };

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;

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);
 

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

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();
 

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.
 

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