Add CommandBroker::OnTransaction to handle incoming transactions

chromeos/binder/command_broker.cc

Index: chromeos/binder/command_broker.cc
diff --git a/chromeos/binder/command_broker.cc b/chromeos/binder/command_broker.cc
index 81b2f93ad3e056c10b63828ef2fb4ddd0cafe957..afdf25bb4c8e0ac0073b74b6390c0b4567f2a3d3 100644
--- a/chromeos/binder/command_broker.cc
+++ b/chromeos/binder/command_broker.cc
@@ -13,6 +13,7 @@
 #include "chromeos/binder/driver.h"
 #include "chromeos/binder/local_object.h"
 #include "chromeos/binder/transaction_data.h"
+#include "chromeos/binder/transaction_status.h"
 
 namespace binder {
 
@@ -47,6 +48,37 @@ CommandBroker::~CommandBroker() {
   DCHECK(thread_checker_.CalledOnValidThread());
 }
 
+bool CommandBroker::EnterLooper() {
+  command_stream_.AppendOutgoingCommand(BC_ENTER_LOOPER, nullptr, 0);
+  return command_stream_.Flush();
+}
+
+bool CommandBroker::ExitLooper() {
+  command_stream_.AppendOutgoingCommand(BC_EXIT_LOOPER, nullptr, 0);
+  return command_stream_.Flush();
+}
+
+bool CommandBroker::PollCommands() {
+  DCHECK(!command_stream_.CanProcessIncomingCommand());

[satorux1, 2016/01/13 04:33:20]

so it's not ok to call this function while CanProcessIncomingCommand() is true.
maybe we should document when to call this function in the function comment?

[hashimoto, 2016/01/13 05:46:46]

I put this DCHECK here to ensure that all previously-fetched commands were
processed in the last PollCommands() call, as leaving fetched commands
unprocessed can result in letting the transaction sender process wait forever.

However, I realized that this DCHECK is unnecessary as there is while
(command_stream_.CanProcessIncomingCommand()) loop below.
So removed this DCHECK.

+  // Fetch and process commands.
+  if (!command_stream_.Fetch()) {
+    LOG(ERROR) << "Failed to fetch commands.";
+    return false;
+  }
+  while (command_stream_.CanProcessIncomingCommand()) {
+    if (!command_stream_.ProcessIncomingCommand()) {
+      LOG(ERROR) << "Failed to process command.";
+      return false;
+    }
+  }
+  // Flush outgoing commands.
+  if (!command_stream_.Flush()) {
+    LOG(ERROR) << "Failed to flush commands.";
+    return false;
+  }
+  return true;
+}
+
 bool CommandBroker::Transact(int32_t handle,
                              const TransactionData& request,
                              scoped_ptr<TransactionData>* reply) {
@@ -100,6 +132,33 @@ base::Closure CommandBroker::GetReleaseReferenceClosure(int32_t handle) {
                     weak_ptr_factory_.GetWeakPtr(), handle);
 }
 
+bool CommandBroker::OnTransaction(const TransactionData& data) {
+  LocalObject* object = reinterpret_cast<LocalObject*>(data.GetCookie());
+  scoped_ptr<TransactionData> reply;
+  if (!object->Transact(this, data, &reply)) {
+    LOG(ERROR) << "Failed to transact.";
+    return false;
+  }
+  if (!data.IsOneWay()) {
+    // Send reply.
+    if (!reply) {
+      reply.reset(new TransactionStatus(Status::FAILED_TRANSACTION));
+    }
+    binder_transaction_data tr = ConvertTransactionDataToStruct(*reply);
+    tr.target.handle = -1;

[satorux1, 2016/01/13 04:33:20]

maybe document why -1? 

[hashimoto, 2016/01/13 05:46:46]

Done.

+    command_stream_.AppendOutgoingCommand(BC_REPLY, &tr, sizeof(tr));
+    if (!command_stream_.Flush()) {
+      LOG(ERROR) << "Failed to write";
+      return false;
+    }
+    scoped_ptr<TransactionData> response_data;
+    ResponseType response_type = WaitForResponse(&response_data);
+    LOG_IF(ERROR, response_type != RESPONSE_TYPE_TRANSACTION_COMPLETE)
+        << "Error on the other end when sending reply: " << response_type;

[satorux1, 2016/01/13 04:33:20]

in this case, should this return true?

[hashimoto, 2016/01/13 05:46:46]

Added a comment.

+  }
+  return true;
+}
+
 void CommandBroker::OnReply(scoped_ptr<TransactionData> data) {
   DCHECK(thread_checker_.CalledOnValidThread());
   DCHECK_EQ(response_type_, RESPONSE_TYPE_NONE);