Mojo C++ bindings: refactor SyncHandleWatcher.

mojo/public/cpp/bindings/lib/router.cc

Index: mojo/public/cpp/bindings/lib/router.cc
diff --git a/mojo/public/cpp/bindings/lib/router.cc b/mojo/public/cpp/bindings/lib/router.cc
index 045fc6203fec20204ea20ec915ee5738fc5d1954..0eea525fcfdad3216772fd052132aacccefa6502 100644
--- a/mojo/public/cpp/bindings/lib/router.cc
+++ b/mojo/public/cpp/bindings/lib/router.cc
@@ -124,7 +124,7 @@ Router::Router(ScopedMessagePipeHandle message_pipe,
       weak_factory_(this) {
   filters_.SetSink(&thunk_);
   if (expects_sync_requests)
-    connector_.RegisterSyncHandleWatch();
+    connector_.AllowWokenUpBySyncWatchOnSameThread();
   connector_.set_incoming_receiver(filters_.GetHead());
   connector_.set_connection_error_handler([this]() { OnConnectionError(); });
 }
@@ -156,16 +156,13 @@ bool Router::AcceptWithResponder(Message* message, MessageReceiver* responder) {
     return true;
   }
 
-  if (!connector_.RegisterSyncHandleWatch())
-    return false;
-
   bool response_received = false;
   scoped_ptr<MessageReceiver> sync_responder(responder);
   sync_responses_.insert(std::make_pair(
       request_id, make_scoped_ptr(new SyncResponseInfo(&response_received))));
 
   base::WeakPtr<Router> weak_self = weak_factory_.GetWeakPtr();
-  bool result = connector_.RunSyncHandleWatch(&response_received);
+  bool result = connector_.SyncWatch(&response_received);
   // Make sure that this instance hasn't been destroyed.
   if (weak_self) {
     DCHECK(ContainsKey(sync_responses_, request_id));
@@ -176,8 +173,6 @@ bool Router::AcceptWithResponder(Message* message, MessageReceiver* responder) {
       ignore_result(sync_responder->Accept(response.get()));
     }
     sync_responses_.erase(iter);
-
-    connector_.UnregisterSyncHandleWatch();
   }
 
   // Return true means that we take ownership of |responder|.
@@ -288,6 +283,9 @@ bool Router::HandleMessageInternal(Message* message) {
 void Router::OnConnectionError() {
   DCHECK(!encountered_error_);

[Ken Rockot(use gerrit already), 2016/03/26 03:09:26]

If this DCHECK is appropriate then the function should always return immediately
by the condition below. One of these should be removed, I think probably the
DCHECK?

[yzshen1, 2016/03/26 06:33:17]

Oops! The DCHECK should be removed. Thanks!

 
+  if (encountered_error_)
+    return;
+
   if (!pending_messages_.empty()) {
     // After all the pending messages are processed, we will check whether an
     // error has been encountered and run the user's connection error handler
@@ -296,6 +294,14 @@ void Router::OnConnectionError() {
     return;
   }
 
+  if (connector_.during_sync_handle_watcher_callback()) {
+    // We don't want the error handler to reenter an ongoing sync call.
+    base::MessageLoop::current()->PostTask(
+        FROM_HERE,
+        base::Bind(&Router::OnConnectionError, weak_factory_.GetWeakPtr()));
+    return;
+  }
+
   encountered_error_ = true;
   error_handler_.Run();
 }