InterfacePtr: not setup proxy until actual read/write is needed.

A common usage of InterfacePtr is:
(1) Create a new InterfacePtr instance.
(2) BindToProxy() is called to bind the interface pointer to an implementation.
(3) Immediately, the message pipe handle underneath the interface pointer is detached and sent over another message pipe.

In step (2), currently we do all the setup work for the interface pointer: construct Router and Connector instance, post task to start watching the message pipe handle, etc. All the work is thrown away at step (3).

With this CL, the proxy setup work won't be done until actual read/write is needed.

BUG=394883
TEST=None

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@284034 0039d316-1c4b-4281-b951-d872f2087c98

mojo/apps/js/test/js_to_cpp_unittest.cc

@@ -394,7 +394,7 @@ class JsToCppTest : public testing::Test {
         MakeProxy<js_to_cpp::JsSide>(pipe.handle0.Pass());
     js_side.set_client(cpp_side);
 
-    js_side.internal_state()->router()->EnableTestingMode();
+    js_side.internal_state()->router_for_testing()->EnableTestingMode();
 
     cpp_side->set_js_side(js_side.get());
 

mojo/public/cpp/bindings/interface_ptr.h

@@ -60,15 +60,14 @@ class InterfacePtr {
   // The proxy is bound to the current thread, which means its methods may
   // only be called on the current thread.
   //
-  // To move a bound InterfacePtr<..> to another thread, call
-  // ResetAndReturnMessagePipe. Then create a new InterfacePtr<..> on another
-  // thread, and bind the new InterfacePtr<..> to the message pipe on that
-  // thread.
+  // To move a bound InterfacePtr<..> to another thread, call PassMessagePipe().
+  // Then create a new InterfacePtr<..> on another thread, and bind the new
+  // InterfacePtr<..> to the message pipe on that thread.
   void Bind(
       ScopedMessagePipeHandle handle,
       const MojoAsyncWaiter* waiter = Environment::GetDefaultAsyncWaiter()) {
     reset();
-    internal_state_.ConfigureProxy(handle.Pass(), waiter);
+    internal_state_.Bind(handle.Pass(), waiter);
   }
 
   // The client interface may only be set after this InterfacePtr<..> is bound.
@@ -80,16 +79,15 @@ class InterfacePtr {
   // an error. If true, this means method calls made on this interface will be
   // dropped (and may have already been dropped) on the floor.
   bool encountered_error() const {
-    assert(internal_state_.router());
-    return internal_state_.router()->encountered_error();
+    return internal_state_.encountered_error();
   }
 
   // This method may be called to register an ErrorHandler to observe a
-  // connection error on the underlying pipe. The callback runs asynchronously
-  // from the current message loop.
+  // connection error on the underlying pipe. It must only be called on a bound
+  // object.
+  // The callback runs asynchronously from the current message loop.
   void set_error_handler(ErrorHandler* error_handler) {
-    assert(internal_state_.router());
-    internal_state_.router()->set_error_handler(error_handler);
+    internal_state_.set_error_handler(error_handler);
   }
 
   // Returns the underlying message pipe handle (if any) and resets the
@@ -98,8 +96,7 @@ class InterfacePtr {
   ScopedMessagePipeHandle PassMessagePipe() {
     State state;
     internal_state_.Swap(&state);
-    return state.router() ?
-        state.router()->PassMessagePipe() : ScopedMessagePipeHandle();
+    return state.PassMessagePipe();
   }
 
   // DO NOT USE. Exposed only for internal use and for testing.
@@ -109,7 +106,7 @@ class InterfacePtr {
 
  private:
   typedef internal::InterfacePtrState<Interface> State;
-  State internal_state_;
+  mutable State internal_state_;
 };
 
 // Takes a handle to the proxy end-point of a pipe. On the other end is

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

@@ -10,7 +10,8 @@
 #include "mojo/public/cpp/bindings/lib/filter_chain.h"
 #include "mojo/public/cpp/bindings/lib/message_header_validator.h"
 #include "mojo/public/cpp/bindings/lib/router.h"
-#include "mojo/public/cpp/environment/environment.h"
+
+struct MojoAsyncWaiter;
 
 namespace mojo {
 namespace internal {
@@ -18,7 +19,7 @@ namespace internal {
 template <typename Interface>
 class InterfacePtrState {
  public:
-  InterfacePtrState() : proxy_(NULL), router_(NULL) {}
+  InterfacePtrState() : proxy_(NULL), router_(NULL), waiter_(NULL) {}
 
   ~InterfacePtrState() {
     // Destruction order matters here. We delete |proxy_| first, even though
@@ -28,43 +29,68 @@ class InterfacePtrState {
     delete router_;
   }
 
-  Interface* instance() const { return proxy_; }
+  Interface* instance() {
+    ConfigureProxyIfNecessary();
 
-  Router* router() const { return router_; }
+    // This will be NULL if the object is not bound.
+    return proxy_;
+  }
 
   void Swap(InterfacePtrState* other) {
     std::swap(other->proxy_, proxy_);
     std::swap(other->router_, router_);
+    handle_.swap(other->handle_);
+    std::swap(other->waiter_, waiter_);
   }
 
-  void ConfigureProxy(
-      ScopedMessagePipeHandle handle,
-      const MojoAsyncWaiter* waiter = Environment::GetDefaultAsyncWaiter()) {
+  void Bind(ScopedMessagePipeHandle handle, const MojoAsyncWaiter* waiter) {
     assert(!proxy_);
     assert(!router_);
+    assert(!handle_.is_valid());
+    assert(!waiter_);
 
-    FilterChain filters;
-    filters.Append<MessageHeaderValidator>();
-    filters.Append<typename Interface::Client::RequestValidator_>();
-    filters.Append<typename Interface::ResponseValidator_>();
-
-    router_ = new Router(handle.Pass(), filters.Pass(), waiter);
-    ProxyWithStub* proxy = new ProxyWithStub(router_);
-    router_->set_incoming_receiver(&proxy->stub);
-
-    proxy_ = proxy;
+    handle_ = handle.Pass();
+    waiter_ = waiter;
   }
 
   bool WaitForIncomingMethodCall() {
+    ConfigureProxyIfNecessary();
+
     assert(router_);
     return router_->WaitForIncomingMessage();
   }
 
+  ScopedMessagePipeHandle PassMessagePipe() {
+    if (router_)
+      return router_->PassMessagePipe();
+
+    waiter_ = NULL;
+    return handle_.Pass();
+  }
+
   void set_client(typename Interface::Client* client) {
+    ConfigureProxyIfNecessary();
+
     assert(proxy_);
     proxy_->stub.set_sink(client);
   }
 
+  bool encountered_error() const {
+    return router_ ? router_->encountered_error() : false;
+  }
+
+  void set_error_handler(ErrorHandler* error_handler) {
+    ConfigureProxyIfNecessary();
+
+    assert(router_);
+    router_->set_error_handler(error_handler);
+  }
+
+  Router* router_for_testing() {
+    ConfigureProxyIfNecessary();
+    return router_;
+  }
+
  private:
   class ProxyWithStub : public Interface::Proxy_ {
    public:
@@ -76,9 +102,41 @@ class InterfacePtrState {
     MOJO_DISALLOW_COPY_AND_ASSIGN(ProxyWithStub);
   };
 
+  void ConfigureProxyIfNecessary() {
+    // The proxy has been configured.
+    if (proxy_) {
+      assert(router_);
+      return;
+    }
+    // The object hasn't been bound.
+    if (!waiter_) {
+      assert(!handle_.is_valid());
+      return;
+    }
+
+    FilterChain filters;
+    filters.Append<MessageHeaderValidator>();
+    filters.Append<typename Interface::Client::RequestValidator_>();
+    filters.Append<typename Interface::ResponseValidator_>();
+
+    router_ = new Router(handle_.Pass(), filters.Pass(), waiter_);
+    waiter_ = NULL;
+
+    ProxyWithStub* proxy = new ProxyWithStub(router_);
+    router_->set_incoming_receiver(&proxy->stub);
+
+    proxy_ = proxy;
+  }
+
   ProxyWithStub* proxy_;
   Router* router_;
 
+  // |proxy_| and |router_| are not initialized until read/write with the
+  // message pipe handle is needed. Before that, |handle_| and |waiter_| store
+  // the arguments of Bind().
+  ScopedMessagePipeHandle handle_;
+  const MojoAsyncWaiter* waiter_;
+
   MOJO_DISALLOW_COPY_AND_ASSIGN(InterfacePtrState);
 };
 

mojo/public/cpp/bindings/tests/interface_ptr_unittest.cc

@@ -278,7 +278,7 @@ TEST_F(InterfacePtrTest, Resettable) {
   a.reset();
 
   EXPECT_TRUE(!a.get());
-  EXPECT_FALSE(a.internal_state()->router());
+  EXPECT_FALSE(a.internal_state()->router_for_testing());
 
   // Test that handle was closed.
   EXPECT_EQ(MOJO_RESULT_INVALID_ARGUMENT, CloseRaw(handle));

mojo/public/cpp/bindings/tests/validation_unittest.cc

@@ -406,7 +406,7 @@ TEST_F(ValidationIntegrationTest, InterfacePtr) {
   IntegrationTestInterface2Ptr interface2_ptr =
       MakeProxy<IntegrationTestInterface2>(testee_endpoint().Pass());
   interface2_ptr.set_client(&interface1_client);
-  interface2_ptr.internal_state()->router()->EnableTestingMode();
+  interface2_ptr.internal_state()->router_for_testing()->EnableTestingMode();
 
   RunValidationTests("integration_", test_message_receiver());
 }