Start of adding enums for error codes

I started with CreateNode so that you can get a feel for how this
would look. I may need a couple of other error codes, but not many
more.

BUG=384437
TEST=covered by tests
R=ben@chromium.org

Review URL: https://codereview.chromium.org/383293002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@282760 0039d316-1c4b-4281-b951-d872f2087c98

mojo/services/public/cpp/view_manager/lib/view_manager_client_impl.cc

@@ -137,11 +137,18 @@ class ViewManagerTransaction {
     return client_->next_server_change_id_++;
   }
 
+  // TODO(sky): nuke this and covert all to new one, then rename
+  // ActionCompletedCallbackWithErrorCode to ActionCompletedCallback.
   base::Callback<void(bool)> ActionCompletedCallback() {
     return base::Bind(&ViewManagerTransaction::OnActionCompleted,
                       base::Unretained(this));
   }
 
+  base::Callback<void(ErrorCode)> ActionCompletedCallbackWithErrorCode() {
+    return base::Bind(&ViewManagerTransaction::OnActionCompletedWithErrorCode,
+                      base::Unretained(this));
+  }
+
  private:
   // General callback to be used for commits to the service.
   void OnActionCompleted(bool success) {
@@ -149,6 +156,11 @@ class ViewManagerTransaction {
     client_->RemoveFromPendingQueue(this);
   }
 
+  void OnActionCompletedWithErrorCode(ErrorCode error_code) {
+    DoActionCompleted(error_code == ERROR_CODE_NONE);
+    client_->RemoveFromPendingQueue(this);
+  }
+
   bool committed_;
   ViewManagerClientImpl* client_;
 
@@ -209,7 +221,7 @@ class CreateNodeTransaction : public ViewManagerTransaction {
  private:
   // Overridden from ViewManagerTransaction:
   virtual void DoCommit() OVERRIDE {
-    service()->CreateNode(node_id_, ActionCompletedCallback());
+    service()->CreateNode(node_id_, ActionCompletedCallbackWithErrorCode());
   }
   virtual void DoActionCompleted(bool success) OVERRIDE {
     // TODO(beng): Failure means we tried to create with an extant id for this

mojo/services/public/interfaces/view_manager/view_manager.mojom

@@ -16,6 +16,13 @@ struct NodeData {
   // TODO(sky): add visible.
 };
 
+enum ErrorCode {
+  ERROR_CODE_NONE,
+  ERROR_CODE_VALUE_IN_USE,
+  ERROR_CODE_ILLEGAL_ARGUMENT,
+  ERROR_CODE_UNEXPECTED_CHANGE_ID,
+};
+
 // ViewManagerInitService is responsible for launching the client that controls
 // the root node. mojo::view_manager returns an instance of this. All other
 // connections are established by the client this creates.
@@ -39,7 +46,11 @@ interface ViewManagerService {
   // the id is unique to the connection (the id need not be globally unique).
   // Additionally the connection id (embedded in |node_id|) must match that of
   // the connection.
-  CreateNode(uint32 node_id) => (bool success);
+  // Errors:
+  //   ERROR_CODE_VALUE_IN_USE: a node already exists with the specified id.
+  //   ERROR_CODE_ILLEGAL_ARGUMENT: The connection part of |node_id| does not
+  //     match the connection id of the client.
+  CreateNode(uint32 node_id) => (ErrorCode error_code);
 
   // Deletes a node. This does not recurse. No hierarchy change notifications
   // are sent as a result of this. Only the connection that created the node can

mojo/services/view_manager/view_manager_service_impl.cc

@@ -542,16 +542,18 @@ Array<NodeDataPtr> ViewManagerServiceImpl::NodesToNodeDatas(
 
 void ViewManagerServiceImpl::CreateNode(
     Id transport_node_id,
-    const Callback<void(bool)>& callback) {
+    const Callback<void(ErrorCode)>& callback) {
   const NodeId node_id(NodeIdFromTransportId(transport_node_id));
-  if (node_id.connection_id != id_ ||
-      node_map_.find(node_id.node_id) != node_map_.end()) {
-    callback.Run(false);
-    return;
+  ErrorCode error_code = ERROR_CODE_NONE;
+  if (node_id.connection_id != id_) {
+    error_code = ERROR_CODE_ILLEGAL_ARGUMENT;
+  } else if (node_map_.find(node_id.node_id) != node_map_.end()) {
+    error_code = ERROR_CODE_VALUE_IN_USE;
+  } else {
+    node_map_[node_id.node_id] = new Node(root_node_manager_, node_id);
+    known_nodes_.insert(transport_node_id);
   }
-  node_map_[node_id.node_id] = new Node(root_node_manager_, node_id);
-  known_nodes_.insert(transport_node_id);
-  callback.Run(true);
+  callback.Run(error_code);
 }
 
 void ViewManagerServiceImpl::DeleteNode(

mojo/services/view_manager/view_manager_service_impl.h

@@ -169,7 +169,7 @@ class MOJO_VIEW_MANAGER_EXPORT ViewManagerServiceImpl
 
   // Overridden from ViewManagerService:
   virtual void CreateNode(Id transport_node_id,
-                          const Callback<void(bool)>& callback) OVERRIDE;
+                          const Callback<void(ErrorCode)>& callback) OVERRIDE;
   virtual void DeleteNode(Id transport_node_id,
                           Id server_change_id,
                           const Callback<void(bool)>& callback) OVERRIDE;

mojo/services/view_manager/view_manager_unittest.cc

@@ -93,10 +93,21 @@ class ViewManagerProxy : public TestChangeTracker::Delegate {
   // block until the result is received.
   bool CreateNode(Id node_id) {
     changes_.clear();
-    bool result = false;
-    view_manager_->CreateNode(node_id,
-                              base::Bind(&ViewManagerProxy::GotResult,
-                                         base::Unretained(this), &result));
+    ErrorCode result = ERROR_CODE_NONE;
+    view_manager_->CreateNode(
+        node_id,
+        base::Bind(&ViewManagerProxy::GotResultWithErrorCode,
+                   base::Unretained(this), &result));
+    RunMainLoop();
+    return result == ERROR_CODE_NONE;
+  }
+  ErrorCode CreateNodeWithErrorCode(Id node_id) {
+    changes_.clear();
+    ErrorCode result = ERROR_CODE_NONE;
+    view_manager_->CreateNode(
+        node_id,
+        base::Bind(&ViewManagerProxy::GotResultWithErrorCode,
+                   base::Unretained(this), &result));
     RunMainLoop();
     return result;
   }
@@ -241,6 +252,13 @@ class ViewManagerProxy : public TestChangeTracker::Delegate {
     main_run_loop_->Quit();
   }
 
+  void GotResultWithErrorCode(ErrorCode* error_code_cache,
+                              ErrorCode error_code) {
+    *error_code_cache = error_code;
+    DCHECK(main_run_loop_);
+    main_run_loop_->Quit();
+  }
+
   void GotNodeTree(std::vector<TestNode>* nodes, Array<NodeDataPtr> results) {
     NodeDatasToTestNodes(results, nodes);
     DCHECK(main_run_loop_);
@@ -541,11 +559,14 @@ TEST_F(ViewManagerTest, CreateNode) {
   EXPECT_TRUE(connection_->changes().empty());
 
   // Can't create a node with the same id.
-  ASSERT_FALSE(connection_->CreateNode(BuildNodeId(1, 1)));
+  ASSERT_EQ(ERROR_CODE_VALUE_IN_USE,
+            connection_->CreateNodeWithErrorCode(BuildNodeId(1, 1)));
   EXPECT_TRUE(connection_->changes().empty());
 
   // Can't create a node with a bogus connection id.
-  EXPECT_FALSE(connection_->CreateNode(BuildNodeId(2, 1)));
+  EXPECT_EQ(
+      ERROR_CODE_ILLEGAL_ARGUMENT,
+      connection_->CreateNodeWithErrorCode(BuildNodeId(2, 1)));
   EXPECT_TRUE(connection_->changes().empty());
 }