mojo: Support sync calls through ThreadSafeInterfacePtr

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

Index: mojo/public/cpp/bindings/tests/sync_method_unittest.cc
diff --git a/mojo/public/cpp/bindings/tests/sync_method_unittest.cc b/mojo/public/cpp/bindings/tests/sync_method_unittest.cc
index d0e5f108880aa2bfe3a7e8018a59a496e8be3022..264e3484ffec871cec9de17c220e695de9aad61c 100644
--- a/mojo/public/cpp/bindings/tests/sync_method_unittest.cc
+++ b/mojo/public/cpp/bindings/tests/sync_method_unittest.cc
@@ -349,17 +349,21 @@ TYPED_TEST(SyncMethodCommonTest, CallSyncMethodAsynchronously) {
 
 TYPED_TEST(SyncMethodCommonTest, BasicSyncCalls) {
   InterfacePtr<TypeParam> ptr;
-
+  InterfaceRequest<TypeParam> request(&ptr);
   TestSyncServiceThread<TypeParam> service_thread;
+  scoped_refptr<ThreadSafeInterfacePtr<TypeParam>> tsip =
+      ThreadSafeInterfacePtr<TypeParam>::Create(
+          ptr.PassInterface(), service_thread.thread()->task_runner());
+
   service_thread.thread()->task_runner()->PostTask(
-      FROM_HERE, base::Bind(&TestSyncServiceThread<TypeParam>::SetUp,
-                            base::Unretained(&service_thread),
-                            base::Passed(MakeRequest(&ptr))));
-  ASSERT_TRUE(ptr->Ping());
+      FROM_HERE,
+      base::Bind(&TestSyncServiceThread<TypeParam>::SetUp,
+                 base::Unretained(&service_thread), base::Passed(&request)));
+  ASSERT_TRUE((*tsip)->Ping());
   ASSERT_TRUE(service_thread.ping_called());
 
   int32_t output_value = -1;
-  ASSERT_TRUE(ptr->Echo(42, &output_value));
+  ASSERT_TRUE((*tsip)->Echo(42, &output_value));
   ASSERT_EQ(42, output_value);
 
   base::RunLoop run_loop;
@@ -395,6 +399,23 @@ TYPED_TEST(SyncMethodCommonTest, InterfacePtrDestroyedDuringSyncCall) {
   ASSERT_FALSE(ptr->Ping());
 }
 
+TYPED_TEST(SyncMethodCommonTest, TSIPInterfacePtrDestroyedDuringSyncCall) {

[watk, 2017/03/24 02:40:52]

I just added these for local testing. 

I'm thinking it's probably possible to make ThreadSafeInterfacePtr another
dimension of the tests params, so we'd run all these with both IP and TSIP.
WDYT?

+  // 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));
+  scoped_refptr<ThreadSafeInterfacePtr<TypeParam>> thread_safe_ptr =
+      ThreadSafeInterfacePtr<TypeParam>::Create(std::move(ptr));
+  // The binding lives on the same thread as the interface pointer.
+  impl.set_ping_handler(
+      [&thread_safe_ptr](const TestSync::PingCallback& callback) {
+        thread_safe_ptr = nullptr;
+        callback.Run();
+      });
+  ASSERT_FALSE((*thread_safe_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
@@ -435,6 +456,35 @@ TYPED_TEST(SyncMethodCommonTest, NestedSyncCallsWithInOrderResponses) {
   EXPECT_EQ(123, result_value);
 }
 
+TYPED_TEST(SyncMethodCommonTest, TSIPNestedSyncCallsWithInOrderResponses) {
+  // 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));
+  scoped_refptr<ThreadSafeInterfacePtr<TypeParam>> tsip =
+      ThreadSafeInterfacePtr<TypeParam>::Create(std::move(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, &tsip, &result_value](
+          int32_t value, const TestSync::EchoCallback& callback) {
+        if (first_call) {
+          first_call = false;
+          ASSERT_TRUE((*tsip)->Echo(456, &result_value));
+          EXPECT_EQ(456, result_value);
+        }
+        callback.Run(value);
+      });
+
+  ASSERT_TRUE((*tsip)->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.
@@ -461,6 +511,35 @@ TYPED_TEST(SyncMethodCommonTest, NestedSyncCallsWithOutOfOrderResponses) {
   EXPECT_EQ(123, result_value);
 }
 
+TYPED_TEST(SyncMethodCommonTest, TSIPNestedSyncCallsWithOutOfOrderResponses) {
+  // 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));
+  scoped_refptr<ThreadSafeInterfacePtr<TypeParam>> tsip =
+      ThreadSafeInterfacePtr<TypeParam>::Create(std::move(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, &tsip, &result_value](
+          int32_t value, const TestSync::EchoCallback& callback) {
+        callback.Run(value);
+        if (first_call) {
+          first_call = false;
+          ASSERT_TRUE((*tsip)->Echo(456, &result_value));
+          EXPECT_EQ(456, result_value);
+        }
+      });
+
+  ASSERT_TRUE((*tsip)->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.