Reduce code size of BindingState

mojo/public/cpp/bindings/lib/binding_state.h

Index: mojo/public/cpp/bindings/lib/binding_state.h
diff --git a/mojo/public/cpp/bindings/lib/binding_state.h b/mojo/public/cpp/bindings/lib/binding_state.h
index c8d3e834a39a6d70a944c1e660283dcd75ef98f9..24f48dc9f114092292fd6a80153aedb1937f2946 100644
--- a/mojo/public/cpp/bindings/lib/binding_state.h
+++ b/mojo/public/cpp/bindings/lib/binding_state.h
@@ -31,6 +31,57 @@
 namespace mojo {
 namespace internal {
 
+// Non-templated base class to reduce the amount of code generated for every
+// mojom file.

[Ken Rockot(use gerrit already), 2016/08/03 14:55:27]

nit: Maybe document that it's a base class used for templated Binding primitives
which bind a pipe exclusively to a single interface. The detail about code
generation size isn't particularly interesting to someone who just wants to
understand the code, though I don't think it hurts to include that information
as well.

+class SimpleBindingStateBase {

[Ken Rockot(use gerrit already), 2016/08/03 14:55:28]

nit: Maybe drop the "Base" suffix? Seems superfluous.

[tibell, 2016/08/03 23:42:51]

Done.

+ public:
+  SimpleBindingStateBase();
+  ~SimpleBindingStateBase();
+
+  void Bind(

[Ken Rockot(use gerrit already), 2016/08/03 14:55:28]

nitty nit: I'd prefer we make this protected and call it something like
BindInternal

[tibell, 2016/08/03 23:42:51]

Done.

+      ScopedMessagePipeHandle handle,
+      scoped_refptr<base::SingleThreadTaskRunner> runner,
+      const char* interface_name,
+      MessageFilter* request_validator,
+      bool has_sync_methods,
+      MessageReceiverWithResponderStatus* stub);
+
+  bool HasAssociatedInterfaces() const { return false; }
+
+  void PauseIncomingMethodCallProcessing();
+  void ResumeIncomingMethodCallProcessing();
+
+  bool WaitForIncomingMethodCall(
+      MojoDeadline deadline = MOJO_DEADLINE_INDEFINITE);
+
+  void Close();
+
+  void set_connection_error_handler(const base::Closure& error_handler) {
+    DCHECK(is_bound());
+    connection_error_handler_ = error_handler;
+  }
+
+  bool is_bound() const { return !!router_; }
+
+  MessagePipeHandle handle() const {
+    DCHECK(is_bound());
+    return router_->handle();
+  }
+
+  AssociatedGroup* associated_group() { return nullptr; }
+
+  void EnableTestingMode();
+
+ protected:
+  void DestroyRouter();
+
+  internal::Router* router_ = nullptr;
+  base::Closure connection_error_handler_;
+
+ private:
+  void RunConnectionErrorHandler();
+};
+
 template <typename Interface, bool use_multiplex_router>
 class BindingState;
 
@@ -39,7 +90,7 @@ class BindingState;
 // multiple interfaces running on the underlying message pipe. In that case, we
 // can use this specialization to reduce cost.
 template <typename Interface>
-class BindingState<Interface, false> {
+class BindingState<Interface, false> : public SimpleBindingStateBase {
  public:
   explicit BindingState(Interface* impl) : impl_(impl) {
     stub_.set_sink(impl_);
@@ -50,42 +101,10 @@ class BindingState<Interface, false> {
   void Bind(ScopedMessagePipeHandle handle,
             scoped_refptr<base::SingleThreadTaskRunner> runner) {
     DCHECK(!router_);
-    internal::FilterChain filters;
-    filters.Append<MessageHeaderValidator>(Interface::Name_);
-    filters.Append<typename Interface::RequestValidator_>();
-
-    router_ =
-        new internal::Router(std::move(handle), std::move(filters),
-                             Interface::HasSyncMethods_, std::move(runner));
-    router_->set_incoming_receiver(&stub_);
-    router_->set_connection_error_handler(
-        base::Bind(&BindingState::RunConnectionErrorHandler,
-                   base::Unretained(this)));
-  }
-
-  bool HasAssociatedInterfaces() const { return false; }
-
-  void PauseIncomingMethodCallProcessing() {
-    DCHECK(router_);
-    router_->PauseIncomingMethodCallProcessing();
-  }
-  void ResumeIncomingMethodCallProcessing() {
-    DCHECK(router_);
-    router_->ResumeIncomingMethodCallProcessing();
-  }
-
-  bool WaitForIncomingMethodCall(
-      MojoDeadline deadline = MOJO_DEADLINE_INDEFINITE) {
-    DCHECK(router_);
-    return router_->WaitForIncomingMessage(deadline);
-  }
-
-  void Close() {
-    if (!router_)
-      return;
-
-    router_->CloseMessagePipe();
-    DestroyRouter();
+    SimpleBindingStateBase::Bind(
+        std::move(handle), runner, Interface::Name_,
+        new typename Interface::RequestValidator_(), Interface::HasSyncMethods_,
+        &stub_);
   }
 
   InterfaceRequest<Interface> Unbind() {
@@ -95,13 +114,44 @@ class BindingState<Interface, false> {
     return std::move(request);
   }
 
+  Interface* impl() { return impl_; }
+
+ private:
+  typename Interface::Stub_ stub_;
+  Interface* impl_;
+
+  DISALLOW_COPY_AND_ASSIGN(BindingState);
+};
+
+// Non-templated base class to reduce the amount of code generated for every
+// mojom file.
+class MultiplexedBindingStateBase {

[Ken Rockot(use gerrit already), 2016/08/03 14:55:28]

nit: Roughly the same comments about "Base" and documentation here

[tibell, 2016/08/03 23:42:51]

Done.

+ public:
+  MultiplexedBindingStateBase();
+  ~MultiplexedBindingStateBase();
+
+  void Bind(ScopedMessagePipeHandle handle,
+            scoped_refptr<base::SingleThreadTaskRunner> runner,
+            const char* interface_name,
+            std::unique_ptr<MessageFilter> request_validator,
+            bool has_sync_methods,
+            MessageReceiverWithResponderStatus* stub);
+
+  bool HasAssociatedInterfaces() const;
+
+  void PauseIncomingMethodCallProcessing();
+  void ResumeIncomingMethodCallProcessing();
+
+  bool WaitForIncomingMethodCall(
+      MojoDeadline deadline = MOJO_DEADLINE_INDEFINITE);
+
+  void Close();
+
   void set_connection_error_handler(const base::Closure& error_handler) {
     DCHECK(is_bound());
     connection_error_handler_ = error_handler;
   }
 
-  Interface* impl() { return impl_; }
-
   bool is_bound() const { return !!router_; }
 
   MessagePipeHandle handle() const {
@@ -109,38 +159,25 @@ class BindingState<Interface, false> {
     return router_->handle();
   }
 
-  AssociatedGroup* associated_group() { return nullptr; }
-
-  void EnableTestingMode() {
-    DCHECK(is_bound());
-    router_->EnableTestingMode();
-  }
-
- private:
-  void DestroyRouter() {
-    router_->set_connection_error_handler(base::Closure());
-    delete router_;
-    router_ = nullptr;
-    connection_error_handler_.Reset();
+  AssociatedGroup* associated_group() {
+    return endpoint_client_ ? endpoint_client_->associated_group() : nullptr;
   }
 
-  void RunConnectionErrorHandler() {
-    if (!connection_error_handler_.is_null())
-      connection_error_handler_.Run();
-  }
+  void EnableTestingMode();
 
-  internal::Router* router_ = nullptr;
-  typename Interface::Stub_ stub_;
-  Interface* impl_;
+ protected:
+  scoped_refptr<internal::MultiplexRouter> router_;
+  std::unique_ptr<InterfaceEndpointClient> endpoint_client_;
   base::Closure connection_error_handler_;
 
-  DISALLOW_COPY_AND_ASSIGN(BindingState);
+ private:
+  void RunConnectionErrorHandler();
 };
 
 // Uses a multiplexing router. If |Interface| has methods to pass associated
 // interface pointers or requests, this specialization should be used.
 template <typename Interface>
-class BindingState<Interface, true> {
+class BindingState<Interface, true> : public MultiplexedBindingStateBase {
  public:
   explicit BindingState(Interface* impl) : impl_(impl) {
     stub_.set_sink(impl_);
@@ -150,49 +187,11 @@ class BindingState<Interface, true> {
 
   void Bind(ScopedMessagePipeHandle handle,
             scoped_refptr<base::SingleThreadTaskRunner> runner) {
-    DCHECK(!router_);
-
-    router_ = new internal::MultiplexRouter(false, std::move(handle), runner);
-    router_->SetMasterInterfaceName(Interface::Name_);
+    MultiplexedBindingStateBase::Bind(
+        std::move(handle), runner, Interface::Name_,
+        base::WrapUnique(new typename Interface::RequestValidator_()),
+        Interface::HasSyncMethods_, &stub_);
     stub_.serialization_context()->group_controller = router_;
-
-    endpoint_client_.reset(new InterfaceEndpointClient(
-        router_->CreateLocalEndpointHandle(kMasterInterfaceId),
-        &stub_, base::WrapUnique(new typename Interface::RequestValidator_()),
-        Interface::HasSyncMethods_, std::move(runner)));
-
-    endpoint_client_->set_connection_error_handler(
-        base::Bind(&BindingState::RunConnectionErrorHandler,
-                   base::Unretained(this)));
-  }
-
-  bool HasAssociatedInterfaces() const {
-    return router_ ? router_->HasAssociatedEndpoints() : false;
-  }
-
-  void PauseIncomingMethodCallProcessing() {
-    DCHECK(router_);
-    router_->PauseIncomingMethodCallProcessing();
-  }
-  void ResumeIncomingMethodCallProcessing() {
-    DCHECK(router_);
-    router_->ResumeIncomingMethodCallProcessing();
-  }
-
-  bool WaitForIncomingMethodCall(
-      MojoDeadline deadline = MOJO_DEADLINE_INDEFINITE) {
-    DCHECK(router_);
-    return router_->WaitForIncomingMessage(deadline);
-  }
-
-  void Close() {
-    if (!router_)
-      return;
-
-    endpoint_client_.reset();
-    router_->CloseMessagePipe();
-    router_ = nullptr;
-    connection_error_handler_.Reset();
   }
 
   InterfaceRequest<Interface> Unbind() {
@@ -204,41 +203,11 @@ class BindingState<Interface, true> {
     return request;
   }
 
-  void set_connection_error_handler(const base::Closure& error_handler) {
-    DCHECK(is_bound());
-    connection_error_handler_ = error_handler;
-  }
-
   Interface* impl() { return impl_; }
 
-  bool is_bound() const { return !!router_; }
-
-  MessagePipeHandle handle() const {
-    DCHECK(is_bound());
-    return router_->handle();
-  }
-
-  AssociatedGroup* associated_group() {
-    return endpoint_client_ ? endpoint_client_->associated_group() : nullptr;
-  }
-
-  void EnableTestingMode() {
-    DCHECK(is_bound());
-    router_->EnableTestingMode();
-  }
-
  private:
-  void RunConnectionErrorHandler() {
-    if (!connection_error_handler_.is_null())
-      connection_error_handler_.Run();
-  }
-
-  scoped_refptr<internal::MultiplexRouter> router_;
-  std::unique_ptr<InterfaceEndpointClient> endpoint_client_;
-
   typename Interface::Stub_ stub_;
   Interface* impl_;
-  base::Closure connection_error_handler_;
 
   DISALLOW_COPY_AND_ASSIGN(BindingState);
 };