DevTools: use explicit IPC messages for enabling/disabling tracing instead of intercepting protocol…

content/browser/devtools/devtools_tracing_handler.cc

Index: content/browser/devtools/devtools_tracing_handler.cc
diff --git a/content/browser/devtools/devtools_tracing_handler.cc b/content/browser/devtools/devtools_tracing_handler.cc
index de81bf3cb6ad3b3f556c7cb779e8cb464007d2d4..238cf24d5f8d2f8b8f4bc31a90126e0254cb4e60 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),

[dcheng, 2014/08/27 16:43:31]

I'm not super familiar with inspector. From the CL description, it sounds like
the renderer could already pass arbitrary category_filter strings to the
browser, via 'protocol events' (I am guessing this is what
OnDispatchOnInspectorFrontend is?) Is this true?

       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;