Add better handle tracking, and optimize Connector::Accept.

mojo/public/bindings/lib/connector.cc

 // Copyright 2013 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 "mojo/public/bindings/lib/connector.h"
 
 #include <assert.h>
 #include <stdlib.h>
 
 #include <algorithm>
@@ skipping 19 matching lines @@
   assert(!incoming_receiver_);
   incoming_receiver_ = receiver;
   if (incoming_receiver_)
     WaitToReadMore();
 }
 
 bool Connector::Accept(Message* message) {
   if (error_)
     return false;
 
-  write_queue_.Push(message);
-  WriteMore();
+  bool wait_to_write;
+  WriteOne(message, &wait_to_write);
+
+  if (wait_to_write) {

[viettrungluu, 2013/11/08 20:18:38]

I assume that wait_to_write should never be set on error?

+    WaitToWriteMore();
+    if (!error_)
+      write_queue_.Push(message);
+  }
+
   return !error_;
 }
 
 void Connector::OnHandleReady(Callback* callback, MojoResult result) {
   if (callback == &read_callback_)
     ReadMore();
   if (callback == &write_callback_)
     WriteMore();
 }
 
@@ skipping 52 matching lines @@
     if (rv != MOJO_RESULT_OK) {
       error_ = true;
       break;
     }
 
     incoming_receiver_->Accept(&message);
   }
 }
 
 void Connector::WriteMore() {
-  while (!write_queue_.IsEmpty()) {
-    const Message* message = write_queue_.Peek();
+  while (!error_ && !write_queue_.IsEmpty()) {
+    Message* message = write_queue_.Peek();
 
-    MojoResult rv = WriteMessage(message_pipe_,
-                                 message->data,
-                                 message->data->header.num_bytes,
-                                 message->handles.data(),
-                                 message->handles.size(),
-                                 MOJO_WRITE_MESSAGE_FLAG_NONE);
-    if (rv == MOJO_RESULT_OK) {
-      // TODO(darin): Handles were successfully transferred, and so we need
-      // to take care not to Close them here.
-      write_queue_.Pop();
-      continue;  // Write another message.
-    }
+    bool wait_to_write;
+    WriteOne(message, &wait_to_write);
+    if (wait_to_write)
+      break;
 
-    error_ = true;
-    break;
+    write_queue_.Pop();
   }
 }
 
+void Connector::WriteOne(Message* message, bool* wait_to_write) {
+  // TODO(darin): WriteMessage will eventually start generating an error that
+  // it cannot accept more data. In that case, we'll need to wait on the pipe
+  // to determine when we can try writing again. This flag will be set to true
+  // in that case.
+  *wait_to_write = false;
+
+  MojoResult rv = WriteMessage(message_pipe_,
+                               message->data,
+                               message->data->header.num_bytes,
+                               message->handles.data(),
+                               message->handles.size(),

[viettrungluu, 2013/11/08 20:18:38]

You need to static_cast this to uint32_t, or else jschuh will be unhappy (and
you'll break the win64 lkgr bot, apparently).

+                               MOJO_WRITE_MESSAGE_FLAG_NONE);
+  if (rv == MOJO_RESULT_OK) {
+    // The handles were successfully transferred, so we don't need the message
+    // to track their lifetime any longer.
+    message->handles.clear();
+  } else {
+    error_ = true;
+  }
+}
+
 // ----------------------------------------------------------------------------
 
 Connector::Callback::Callback()
     : owner_(NULL) {
 }
 
 void Connector::Callback::Cancel() {
   owner_ = NULL;
   BindingsSupport::Get()->CancelWait(this);
 }
 
 void Connector::Callback::SetOwnerToNotify(Connector* owner) {
   assert(!owner_);
   owner_ = owner;
 }
 
 bool Connector::Callback::IsPending() const {
   return owner_ != NULL;
 }
 
 void Connector::Callback::OnHandleReady(MojoResult result) {
   assert(owner_);
   Connector* owner = NULL;
   std::swap(owner, owner_);
   owner->OnHandleReady(this, result);
 }
 
 }  // namespace mojo