De-client tracing.TraceController interface

shell/context.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "shell/context.h"
 
 #include <vector>
 
 #include "base/bind.h"
 #include "base/command_line.h"
@@ skipping 96 matching lines @@
     const GURL to(pair.second);
     if (!from.is_valid() || !to.is_valid())
       return false;
     resolver->AddCustomMapping(from, to);
   }
   return true;
 }
 
 }  // namespace
 
+class Context::TracingServiceProvider : public ServiceProvider {
+ public:
+  explicit TracingServiceProvider(ScopedMessagePipeHandle handle)
+      : binding_(this, handle.Pass()) {}
+  ~TracingServiceProvider() override {}
+
+  void ConnectToService(const mojo::String& service_name,
+                        ScopedMessagePipeHandle client_handle) override {
+    if (service_name != tracing::TraceController::Name_) {
+      client_handle.reset();
+      return;
+    }
+    trace_controller_.Bind(client_handle.Pass());
+  }
+
+ private:
+  TraceControllerImpl trace_controller_;
+  StrongBinding<ServiceProvider> binding_;

[qsr, 2014/12/19 09:47:14]

I might be missing something, but if I understand correctly, this means that
this object will be auto-deleted when the pipe is closed, but you still keep it
in a scoped_ptr on the context. Wouldn't that generate a double free if the
handle is closed?

[jamesr, 2014/12/29 18:57:58]

You are correct.  I'll get rid of the scoped_ptr (there's no need to retain this
on the Context)

+};
+
 Context::Context() : application_manager_(this) {
   DCHECK(!base::MessageLoop::current());
 }
 
 Context::~Context() {
   DCHECK(!base::MessageLoop::current());
 }
 
 void Context::EnsureEmbedderIsInitialized() {
   setup.Get();
@@ skipping 48 matching lines @@
 
   if (command_line->HasSwitch(switches::kSpy)) {
     spy_.reset(
         new mojo::Spy(&application_manager_,
                       command_line->GetSwitchValueASCII(switches::kSpy)));
     // TODO(cpu): the spy can snoop, but can't tell anybody until
     // the Spy::WebSocketDelegate is implemented. In the original repo this
     // was implemented by src\mojo\spy\websocket_server.h and .cc.
   }
 
-  tracing::TraceDataCollectorPtr trace_data_collector_ptr;
-  application_manager_.ConnectToService(GURL("mojo:tracing"),
-                                        &trace_data_collector_ptr);
-  TracingImpl::Create(trace_data_collector_ptr.Pass());
+  MessagePipe pipe;
+  ServiceProviderPtr spp;
+  tracing_sp_.reset(new Context::TracingServiceProvider(pipe.handle0.Pass()));
+  spp.Bind(pipe.handle1.Pass());

[qsr, 2014/12/19 09:47:14]

Did we lose the ability to bury the pipes with the new mojo::Bindings interface?
Or should TracingServiceProvider takes an InterfaceRequest so that we can do:

ServiceProviderPtr spp;
InterfaceRequest<ServiceProvider> irsp = GetProxy(&spp);
tracing_sp_.reset(new Context::TracingServiceProvider(irsp.Pass()));

It is conceptually exactly the same, but I was on the impression that we
prefered burying pipe when possible.

[jamesr, 2014/12/29 18:57:58]

I think hiding these makes this code a lot harder to understand.

[qsr, 2015/01/05 13:38:31]

 Hum... Ok, I have a tendency to think the opposite, because burying the pipe
makes it type safe (as everything is linked to the final type, instead of
passing a pipe and TracingServiceProvider constructor is explicit about what it
expects).

+  application_manager_.ConnectToApplication(GURL("mojo:tracing"), GURL(""),
+                                            spp.Pass());
 
   if (listener_)
     listener_->WaitForListening();
 
   return true;
 }
 
 void Context::OnApplicationError(const GURL& url) {
   if (app_urls_.find(url) != app_urls_.end()) {
     app_urls_.erase(url);
@@ skipping 18 matching lines @@
 ScopedMessagePipeHandle Context::ConnectToServiceByName(
     const GURL& application_url,
     const std::string& service_name) {
   app_urls_.insert(application_url);
   return application_manager_.ConnectToServiceByName(application_url,
                                                      service_name).Pass();
 }
 
 }  // namespace shell
 }  // namespace mojo