De-client tracing.TraceController interface

shell/context.cc

Index: shell/context.cc
diff --git a/shell/context.cc b/shell/context.cc
index cd65273ae99839d1e47f6bb9febfde2f7883ca67..91ee054d8b921950e8f5abd29e84112a4845a892 100644
--- a/shell/context.cc
+++ b/shell/context.cc
@@ -114,6 +114,26 @@ bool ConfigureURLMappings(const std::string& mappings,
 
 }  // 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());
 }
@@ -182,10 +202,12 @@ bool Context::Init() {
     // 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();