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

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

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

8 files changed, 37 insertions, 59 deletions

diff --git a/content/browser/devtools/devtools_protocol.cc b/content/browser/devtools/devtools_protocol.cc
index c01f6f2..e3d5314 100644
--- a/content/browser/devtools/devtools_protocol.cc
+++ b/content/browser/devtools/devtools_protocol.cc
@@ -178,15 +178,6 @@ 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;
 }
@@ -200,12 +191,6 @@ 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 71c24ab..cbc075b 100644
--- a/content/browser/devtools/devtools_protocol.h
+++ b/content/browser/devtools/devtools_protocol.h
@@ -127,18 +127,12 @@ 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:
@@ -147,9 +141,6 @@ 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);
@@ -162,11 +153,9 @@ 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 de81bf3..238cf24 100644
--- a/content/browser/devtools/devtools_tracing_handler.cc
+++ b/content/browser/devtools/devtools_tracing_handler.cc
@@ -65,12 +65,6 @@ 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() {
@@ -154,6 +148,11 @@ 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;
@@ -175,16 +174,6 @@ 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,
@@ -228,6 +217,10 @@ 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()));
@@ -272,8 +265,7 @@ void DevToolsTracingHandler::OnCategoriesReceived(
   SendAsyncResponse(command->SuccessResponse(response));
 }
 
-void DevToolsTracingHandler::OnTracingStarted(
-    scoped_refptr<DevToolsProtocol::Notification> notification) {
+void DevToolsTracingHandler::EnableTracing(const std::string& category_filter) {
   if (is_recording_)
     return;
   is_recording_ = true;
@@ -281,13 +273,13 @@ void DevToolsTracingHandler::OnTracingStarted(
   SetupTimer(kDefaultReportingInterval);
 
   TracingController::GetInstance()->EnableRecording(
-      base::debug::CategoryFilter(kDefaultCategories),
+      base::debug::CategoryFilter(category_filter),
       base::debug::TraceOptions(),
       TracingController::EnableRecordingDoneCallback());
+  SendNotification(devtools::Tracing::started::kName, NULL);
 }
 
-void DevToolsTracingHandler::OnTracingStopped(
-    scoped_refptr<DevToolsProtocol::Notification> notification) {
+void DevToolsTracingHandler::DisableTracing() {
   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 20ebca8..fe270bd 100644
--- a/content/browser/devtools/devtools_tracing_handler.h
+++ b/content/browser/devtools/devtools_tracing_handler.h
@@ -30,6 +30,9 @@ 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);
@@ -45,12 +48,6 @@ 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 12a05d5..a858ff8 100644
--- a/content/browser/devtools/render_view_devtools_agent_host.cc
+++ b/content/browser/devtools/render_view_devtools_agent_host.cc
@@ -441,6 +441,8 @@ 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)
@@ -474,14 +476,16 @@ void RenderViewDevToolsAgentHost::OnDispatchOnInspectorFrontend(
     const std::string& message) {
   if (!render_view_host_)
     return;
+  SendMessageToClient(message);
+}
 
-  scoped_refptr<DevToolsProtocol::Notification> notification =
-      DevToolsProtocol::ParseNotification(message);
+void RenderViewDevToolsAgentHost::OnEnableTracing(
+    const std::string& category_filter) {
+  tracing_handler_->EnableTracing(category_filter);
+}
 
-  if (notification.get()) {
-    tracing_handler_->HandleNotification(notification);
-  }
-  SendMessageToClient(message);
+void RenderViewDevToolsAgentHost::OnDisableTracing() {
+  tracing_handler_->DisableTracing();
 }
 
 }  // 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 7b35388..8657bfd 100644
--- a/content/browser/devtools/render_view_devtools_agent_host.h
+++ b/content/browser/devtools/render_view_devtools_agent_host.h
@@ -97,6 +97,8 @@ 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 3e14864..151d88f 100644
--- a/content/common/devtools_messages.h
+++ b/content/common/devtools_messages.h
@@ -114,6 +114,13 @@ 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 7cf4676..71f55bb 100644
--- a/content/renderer/devtools/devtools_agent.cc
+++ b/content/renderer/devtools/devtools_agent.cc
@@ -171,6 +171,7 @@ 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,
@@ -179,6 +180,7 @@ void DevToolsAgent::enableTracing(const WebString& category_filter) {
 
 void DevToolsAgent::disableTracing() {
   TraceLog::GetInstance()->SetDisabled();
+  Send(new DevToolsHostMsg_DisableTracing(routing_id()));
 }
 
 // static