mojo: Support sync calls through ThreadSafeInterfacePtr

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

 // Copyright 2016 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 <utility>
 
 #include "base/bind.h"
 #include "base/logging.h"
 #include "base/macros.h"
 #include "base/message_loop/message_loop.h"
@@ skipping 184 matching lines @@
 struct ImplTraits<TestSync> {
   using Type = TestSyncImpl;
 };
 
 template <>
 struct ImplTraits<TestSyncMaster> {
   using Type = TestSyncMasterImpl;
 };
 
 template <typename Interface>
+using ImplTypeFor = typename ImplTraits<Interface>::Type;
+
+// A wrapper for either an InterfacePtr or scoped_refptr<ThreadSafeInterfacePtr>
+// that exposes the InterfacePtr interface.
+template <typename Interface>
+class PtrWrapper {
+ public:
+  explicit PtrWrapper(InterfacePtr<Interface> ptr) : ptr_(std::move(ptr)) {}
+
+  explicit PtrWrapper(
+      scoped_refptr<ThreadSafeInterfacePtr<Interface>> thread_safe_ptr)
+      : thread_safe_ptr_(thread_safe_ptr) {}
+
+  PtrWrapper(PtrWrapper&& other) = default;
+
+  Interface* operator->() {
+    return thread_safe_ptr_ ? thread_safe_ptr_->get() : ptr_.get();
+  }
+
+  void set_connection_error_handler(const base::Closure& error_handler) {
+    DCHECK(!thread_safe_ptr_);
+    ptr_.set_connection_error_handler(error_handler);
+  }
+
+  void reset() {
+    ptr_ = nullptr;
+    thread_safe_ptr_ = nullptr;
+  }
+
+ private:
+  InterfacePtr<Interface> ptr_;
+  scoped_refptr<ThreadSafeInterfacePtr<Interface>> thread_safe_ptr_;
+
+  DISALLOW_COPY_AND_ASSIGN(PtrWrapper);
+};
+
+template <typename InterfaceT, bool use_thread_safe_ptr>
+struct TestParams {
+  using Interface = InterfaceT;
+  static const bool is_thread_safe_ptr_test = use_thread_safe_ptr;

[yzshen1, 2017/03/30 20:53:06]

Because this is a compile time constant, I think it should be
kIsThreadSafePtrTest?

[watk, 2017/03/31 00:27:27]

Done.

+
+  static PtrWrapper<InterfaceT> Wrap(InterfacePtr<Interface> ptr) {
+    if (is_thread_safe_ptr_test) {
+      return PtrWrapper<Interface>(
+          ThreadSafeInterfacePtr<Interface>::Create(std::move(ptr)));
+    } else {
+      return PtrWrapper<Interface>(std::move(ptr));
+    }
+  }
+};
+
+template <typename Interface>
 class TestSyncServiceThread {
  public:
   TestSyncServiceThread()
       : thread_("TestSyncServiceThread"), ping_called_(false) {
     thread_.Start();
   }
 
   void SetUp(InterfaceRequest<Interface> request) {
     CHECK(thread_.task_runner()->BelongsToCurrentThread());
-    impl_.reset(new typename ImplTraits<Interface>::Type(std::move(request)));
+    impl_.reset(new ImplTypeFor<Interface>(std::move(request)));
     impl_->set_ping_handler(
         [this](const typename Interface::PingCallback& callback) {
           {
             base::AutoLock locker(lock_);
             ping_called_ = true;
           }
           callback.Run();
         });
   }
 
   void TearDown() {
     CHECK(thread_.task_runner()->BelongsToCurrentThread());
     impl_.reset();
   }
 
   base::Thread* thread() { return &thread_; }
   bool ping_called() const {
     base::AutoLock locker(lock_);
     return ping_called_;
   }
 
  private:
   base::Thread thread_;
 
-  std::unique_ptr<typename ImplTraits<Interface>::Type> impl_;
+  std::unique_ptr<ImplTypeFor<Interface>> impl_;
 
   mutable base::Lock lock_;
   bool ping_called_;
 
   DISALLOW_COPY_AND_ASSIGN(TestSyncServiceThread);
 };
 
 class SyncMethodTest : public testing::Test {
  public:
   SyncMethodTest() {}
@@ skipping 77 matching lines @@
   func(value);
 }
 
 template <typename Func>
 TestSync::AsyncEchoCallback BindAsyncEchoCallback(Func func) {
   return base::Bind(&CallAsyncEchoCallback<Func>, func);
 }
 
 // TestSync (without associated interfaces) and TestSyncMaster (with associated
 // interfaces) exercise MultiplexRouter with different configurations.
-using InterfaceTypes = testing::Types<TestSync, TestSyncMaster>;
+using InterfaceTypes = testing::Types<TestParams<TestSync, true>,
+                                      TestParams<TestSync, false>,
+                                      TestParams<TestSyncMaster, true>,
+                                      TestParams<TestSyncMaster, false>>;
 TYPED_TEST_CASE(SyncMethodCommonTest, InterfaceTypes);
 
 TYPED_TEST(SyncMethodCommonTest, CallSyncMethodAsynchronously) {
-  InterfacePtr<TypeParam> ptr;
-  typename ImplTraits<TypeParam>::Type impl(MakeRequest(&ptr));
+  using Interface = typename TypeParam::Interface;
+  InterfacePtr<Interface> interface_ptr;
+  ImplTypeFor<Interface> impl(MakeRequest(&interface_ptr));
+  auto ptr = TypeParam::Wrap(std::move(interface_ptr));
 
   base::RunLoop run_loop;
   ptr->Echo(123, base::Bind(&ExpectValueAndRunClosure, 123,
                             run_loop.QuitClosure()));
   run_loop.Run();
 }
 
 TYPED_TEST(SyncMethodCommonTest, BasicSyncCalls) {
-  InterfacePtr<TypeParam> ptr;
+  using Interface = typename TypeParam::Interface;
+  InterfacePtr<Interface> interface_ptr;
+  InterfaceRequest<Interface> request = MakeRequest(&interface_ptr);
+  auto ptr = TypeParam::Wrap(std::move(interface_ptr));
 
-  TestSyncServiceThread<TypeParam> service_thread;
+  TestSyncServiceThread<Interface> service_thread;
   service_thread.thread()->task_runner()->PostTask(
-      FROM_HERE, base::Bind(&TestSyncServiceThread<TypeParam>::SetUp,
-                            base::Unretained(&service_thread),
-                            base::Passed(MakeRequest(&ptr))));
+      FROM_HERE,
+      base::Bind(&TestSyncServiceThread<Interface>::SetUp,
+                 base::Unretained(&service_thread), base::Passed(&request)));
   ASSERT_TRUE(ptr->Ping());
   ASSERT_TRUE(service_thread.ping_called());
 
   int32_t output_value = -1;
   ASSERT_TRUE(ptr->Echo(42, &output_value));
   ASSERT_EQ(42, output_value);
 
   base::RunLoop run_loop;
   service_thread.thread()->task_runner()->PostTaskAndReply(
-      FROM_HERE, base::Bind(&TestSyncServiceThread<TypeParam>::TearDown,
-                            base::Unretained(&service_thread)),
+      FROM_HERE,
+      base::Bind(&TestSyncServiceThread<Interface>::TearDown,
+                 base::Unretained(&service_thread)),
       run_loop.QuitClosure());
   run_loop.Run();
 }
 
 TYPED_TEST(SyncMethodCommonTest, ReenteredBySyncMethodBinding) {
   // Test that an interface pointer waiting for a sync call response can be
   // reentered by a binding serving sync methods on the same thread.
 
-  InterfacePtr<TypeParam> ptr;
+  using Interface = typename TypeParam::Interface;
+  InterfacePtr<Interface> interface_ptr;
   // The binding lives on the same thread as the interface pointer.
-  typename ImplTraits<TypeParam>::Type impl(MakeRequest(&ptr));
+  ImplTypeFor<Interface> impl(MakeRequest(&interface_ptr));
+  auto ptr = TypeParam::Wrap(std::move(interface_ptr));
   int32_t output_value = -1;
   ASSERT_TRUE(ptr->Echo(42, &output_value));
   EXPECT_EQ(42, output_value);
 }
 
 TYPED_TEST(SyncMethodCommonTest, InterfacePtrDestroyedDuringSyncCall) {
   // Test that it won't result in crash or hang if an interface pointer is
   // destroyed while it is waiting for a sync call response.
 
-  InterfacePtr<TypeParam> ptr;
-  typename ImplTraits<TypeParam>::Type impl(MakeRequest(&ptr));
+  using Interface = typename TypeParam::Interface;
+  InterfacePtr<Interface> interface_ptr;
+  ImplTypeFor<Interface> impl(MakeRequest(&interface_ptr));
+  auto ptr = TypeParam::Wrap(std::move(interface_ptr));
   impl.set_ping_handler([&ptr](const TestSync::PingCallback& callback) {
     ptr.reset();
     callback.Run();
   });
   ASSERT_FALSE(ptr->Ping());
 }
 
 TYPED_TEST(SyncMethodCommonTest, BindingDestroyedDuringSyncCall) {
   // Test that it won't result in crash or hang if a binding is
   // closed (and therefore the message pipe handle is closed) while the
   // corresponding interface pointer is waiting for a sync call response.
 
-  InterfacePtr<TypeParam> ptr;
-  typename ImplTraits<TypeParam>::Type impl(MakeRequest(&ptr));
+  using Interface = typename TypeParam::Interface;
+  InterfacePtr<Interface> interface_ptr;
+  ImplTypeFor<Interface> impl(MakeRequest(&interface_ptr));
+  auto ptr = TypeParam::Wrap(std::move(interface_ptr));
   impl.set_ping_handler([&impl](const TestSync::PingCallback& callback) {
     impl.binding()->Close();
     callback.Run();
   });
   ASSERT_FALSE(ptr->Ping());
 }
 
 TYPED_TEST(SyncMethodCommonTest, NestedSyncCallsWithInOrderResponses) {
   // Test that we can call a sync method on an interface ptr, while there is
   // already a sync call ongoing. The responses arrive in order.
 
-  InterfacePtr<TypeParam> ptr;
-  typename ImplTraits<TypeParam>::Type impl(MakeRequest(&ptr));
+  using Interface = typename TypeParam::Interface;
+  InterfacePtr<Interface> interface_ptr;
+  ImplTypeFor<Interface> impl(MakeRequest(&interface_ptr));
+  auto ptr = TypeParam::Wrap(std::move(interface_ptr));
 
   // The same variable is used to store the output of the two sync calls, in
   // order to test that responses are handled in the correct order.
   int32_t result_value = -1;
 
   bool first_call = true;
   impl.set_echo_handler([&first_call, &ptr, &result_value](
       int32_t value, const TestSync::EchoCallback& callback) {
     if (first_call) {
       first_call = false;
       ASSERT_TRUE(ptr->Echo(456, &result_value));
       EXPECT_EQ(456, result_value);
     }
     callback.Run(value);
   });
 
   ASSERT_TRUE(ptr->Echo(123, &result_value));
   EXPECT_EQ(123, result_value);
 }
 
 TYPED_TEST(SyncMethodCommonTest, NestedSyncCallsWithOutOfOrderResponses) {
   // Test that we can call a sync method on an interface ptr, while there is
   // already a sync call ongoing. The responses arrive out of order.
 
-  InterfacePtr<TypeParam> ptr;
-  typename ImplTraits<TypeParam>::Type impl(MakeRequest(&ptr));
+  using Interface = typename TypeParam::Interface;
+  InterfacePtr<Interface> interface_ptr;
+  ImplTypeFor<Interface> impl(MakeRequest(&interface_ptr));
+  auto ptr = TypeParam::Wrap(std::move(interface_ptr));
 
   // The same variable is used to store the output of the two sync calls, in
   // order to test that responses are handled in the correct order.
   int32_t result_value = -1;
 
   bool first_call = true;
   impl.set_echo_handler([&first_call, &ptr, &result_value](
       int32_t value, const TestSync::EchoCallback& callback) {
     callback.Run(value);
     if (first_call) {
       first_call = false;
       ASSERT_TRUE(ptr->Echo(456, &result_value));
       EXPECT_EQ(456, result_value);
     }
   });
 
   ASSERT_TRUE(ptr->Echo(123, &result_value));
   EXPECT_EQ(123, result_value);
 }
 
 TYPED_TEST(SyncMethodCommonTest, AsyncResponseQueuedDuringSyncCall) {
   // Test that while an interface pointer is waiting for the response to a sync
   // call, async responses are queued until the sync call completes.
 
-  InterfacePtr<TypeParam> ptr;
-  typename ImplTraits<TypeParam>::Type impl(MakeRequest(&ptr));
+  using Interface = typename TypeParam::Interface;
+  InterfacePtr<Interface> interface_ptr;
+  ImplTypeFor<Interface> impl(MakeRequest(&interface_ptr));
+  auto ptr = TypeParam::Wrap(std::move(interface_ptr));
 
   int32_t async_echo_request_value = -1;
   TestSync::AsyncEchoCallback async_echo_request_callback;
   base::RunLoop run_loop1;
   impl.set_async_echo_handler(
       [&async_echo_request_value, &async_echo_request_callback, &run_loop1](
           int32_t value, const TestSync::AsyncEchoCallback& callback) {
         async_echo_request_value = value;
         async_echo_request_callback = callback;
         run_loop1.Quit();
@@ skipping 34 matching lines @@
   run_loop2.Run();
 
   EXPECT_TRUE(async_echo_response_dispatched);
 }
 
 TYPED_TEST(SyncMethodCommonTest, AsyncRequestQueuedDuringSyncCall) {
   // Test that while an interface pointer is waiting for the response to a sync
   // call, async requests for a binding running on the same thread are queued
   // until the sync call completes.
 
-  InterfacePtr<TypeParam> ptr;
-  typename ImplTraits<TypeParam>::Type impl(MakeRequest(&ptr));
+  using Interface = typename TypeParam::Interface;
+  InterfacePtr<Interface> interface_ptr;
+  ImplTypeFor<Interface> impl(MakeRequest(&interface_ptr));
+  auto ptr = TypeParam::Wrap(std::move(interface_ptr));
 
   bool async_echo_request_dispatched = false;
   impl.set_async_echo_handler([&async_echo_request_dispatched](
       int32_t value, const TestSync::AsyncEchoCallback& callback) {
     async_echo_request_dispatched = true;
     callback.Run(value);
   });
 
   bool async_echo_response_dispatched = false;
   base::RunLoop run_loop;
@@ skipping 29 matching lines @@
   EXPECT_TRUE(async_echo_response_dispatched);
 }
 
 TYPED_TEST(SyncMethodCommonTest,
            QueuedMessagesProcessedBeforeErrorNotification) {
   // Test that while an interface pointer is waiting for the response to a sync
   // call, async responses are queued. If the message pipe is disconnected
   // before the queued messages are processed, the connection error
   // notification is delayed until all the queued messages are processed.
 
-  InterfacePtr<TypeParam> ptr;
-  typename ImplTraits<TypeParam>::Type impl(MakeRequest(&ptr));
+  // ThreadSafeInterfacePtr doesn't make guarantees about error notifications.

[yzshen1, 2017/03/30 20:53:06]

nit: it would be nice to clarify that "doesn't make guarantees that messages are
delivered before the error notification".

[watk, 2017/03/31 00:27:27]

Done.

+  if (TypeParam::is_thread_safe_ptr_test)
+    return;
+
+  using Interface = typename TypeParam::Interface;
+  InterfacePtr<Interface> ptr;
+  ImplTypeFor<Interface> impl(MakeRequest(&ptr));
 
   int32_t async_echo_request_value = -1;
   TestSync::AsyncEchoCallback async_echo_request_callback;
   base::RunLoop run_loop1;
   impl.set_async_echo_handler(
       [&async_echo_request_value, &async_echo_request_callback, &run_loop1](
           int32_t value, const TestSync::AsyncEchoCallback& callback) {
         async_echo_request_value = value;
         async_echo_request_callback = callback;
         run_loop1.Quit();
@@ skipping 54 matching lines @@
   ASSERT_TRUE(connection_error_dispatched);
   EXPECT_TRUE(ptr.encountered_error());
 }
 
 TYPED_TEST(SyncMethodCommonTest, InvalidMessageDuringSyncCall) {
   // Test that while an interface pointer is waiting for the response to a sync
   // call, an invalid incoming message will disconnect the message pipe, cause
   // the sync call to return false, and run the connection error handler
   // asynchronously.
 
+  using Interface = typename TypeParam::Interface;
   MessagePipe pipe;
 
-  InterfacePtr<TypeParam> ptr;
-  ptr.Bind(InterfacePtrInfo<TypeParam>(std::move(pipe.handle0), 0u));
+  InterfacePtr<Interface> interface_ptr;
+  interface_ptr.Bind(InterfacePtrInfo<Interface>(std::move(pipe.handle0), 0u));
+  auto ptr = TypeParam::Wrap(std::move(interface_ptr));
 
   MessagePipeHandle raw_binding_handle = pipe.handle1.get();
-  typename ImplTraits<TypeParam>::Type impl(
-      MakeRequest<TypeParam>(std::move(pipe.handle1)));
+  ImplTypeFor<Interface> impl(MakeRequest<Interface>(std::move(pipe.handle1)));
 
   impl.set_echo_handler([&raw_binding_handle](
       int32_t value, const TestSync::EchoCallback& callback) {
     // Write a 1-byte message, which is considered invalid.
     char invalid_message = 0;
     MojoResult result =
         WriteMessageRaw(raw_binding_handle, &invalid_message, 1u, nullptr, 0u,
                         MOJO_WRITE_MESSAGE_FLAG_NONE);
     ASSERT_EQ(MOJO_RESULT_OK, result);
     callback.Run(value);
   });
 
   bool connection_error_dispatched = false;
   base::RunLoop run_loop;
-  ptr.set_connection_error_handler(
-      base::Bind(&SetFlagAndRunClosure, &connection_error_dispatched,
-                 run_loop.QuitClosure()));
+  // ThreadSafeInterfacePtr doesn't make guarantees about error notifications.
+  if (!TypeParam::is_thread_safe_ptr_test) {

[yzshen1, 2017/03/30 20:53:06]

I guess this test case works for TSIP because it doesn't rely on the order of
messages and error notification. Right?

[watk, 2017/03/31 00:27:27]

Well for TSIP the value of this test is that invalid messages result in sync
calls returning false. Since TSIP doesn't let you set a connection error handler
I didn't bother running that bit for TSIP.

+    ptr.set_connection_error_handler(base::Bind(&SetFlagAndRunClosure,
+                                                &connection_error_dispatched,
+                                                run_loop.QuitClosure()));
+  }
 
   int32_t result_value = -1;
   ASSERT_FALSE(ptr->Echo(456, &result_value));
   EXPECT_EQ(-1, result_value);
   ASSERT_FALSE(connection_error_dispatched);
 
-  run_loop.Run();
-  ASSERT_TRUE(connection_error_dispatched);
+  if (!TypeParam::is_thread_safe_ptr_test) {
+    run_loop.Run();
+    ASSERT_TRUE(connection_error_dispatched);
+  }
 }
 
 TEST_F(SyncMethodAssociatedTest, ReenteredBySyncMethodAssoBindingOfSameRouter) {
   // Test that an interface pointer waiting for a sync call response can be
   // reentered by an associated binding serving sync methods on the same thread.
   // The associated binding belongs to the same MultiplexRouter as the waiting
   // interface pointer.
 
   TestSyncAssociatedImpl opposite_asso_impl(std::move(opposite_asso_request_));
   TestSyncAssociatedPtr opposite_asso_ptr;
@@ skipping 36 matching lines @@
   int32_t result_value = -1;
   ASSERT_TRUE(master_ptr_->Echo(456, &result_value));
   EXPECT_EQ(456, result_value);
 }
 
 // TODO(yzshen): Add more tests related to associated interfaces.
 
 }  // namespace
 }  // namespace test
 }  // namespace mojo