Tweaks to ViewManager:

mojo/services/view_manager/view_manager_connection.cc

Index: mojo/services/view_manager/view_manager_connection.cc
diff --git a/mojo/services/view_manager/view_manager_connection.cc b/mojo/services/view_manager/view_manager_connection.cc
index 342bf9e2ab461575aeb5a457d55c40fb14f4bc91..7dbbb456ac7527f996b4dd9d4d970e75d443006e 100644
--- a/mojo/services/view_manager/view_manager_connection.cc
+++ b/mojo/services/view_manager/view_manager_connection.cc
@@ -8,6 +8,7 @@
 #include "mojo/public/cpp/bindings/allocation_scope.h"
 #include "mojo/services/view_manager/node.h"
 #include "mojo/services/view_manager/root_node_manager.h"
+#include "mojo/services/view_manager/type_converters.h"
 #include "mojo/services/view_manager/view.h"
 #include "third_party/skia/include/core/SkBitmap.h"
 #include "ui/aura/window.h"
@@ -18,43 +19,16 @@ namespace view_manager {
 namespace service {
 namespace {
 
-// Implementation of NodeCount(). |count| is the current count.
-void NodeCountImpl(Node* node, size_t* count) {
-  (*count)++;
-  std::vector<Node*> children(node->GetChildren());
-  for (size_t i = 0 ; i < children.size(); ++i)
-    NodeCountImpl(children[i], count);
-}
-
-// Returns the number of descendants of |node|.
-size_t NodeCount(Node* node) {
-  size_t count = 0;
-  if (node)
-    NodeCountImpl(node, &count);
-  return count;
-}
-
-// Converts a Node to an INode, putting the result at |index| in
-// |array_builder|. This then recurses through the children.
-void NodeToINode(Node* node,
-                 Array<INode>::Builder* array_builder,
-                 size_t* index) {
+// Places |node| in |nodes| and recurses through the children.
+void GetDescendants(Node* node, std::vector<Node*>* nodes) {
   if (!node)
     return;
 
-  INode::Builder builder;
-  Node* parent = node->GetParent();
-  builder.set_parent_id(NodeIdToTransportId(parent ? parent->id() : NodeId()));
-  builder.set_node_id(NodeIdToTransportId(node->id()));
-  builder.set_view_id(ViewIdToTransportId(
-                          node->view() ? node->view()->id() : ViewId()));
-  (*array_builder)[*index] = builder.Finish();
+  nodes->push_back(node);
 
   std::vector<Node*> children(node->GetChildren());
-  for (size_t i = 0 ; i < children.size(); ++i) {
-    (*index)++;
-    NodeToINode(children[i], array_builder, index);
-  }
+  for (size_t i = 0 ; i < children.size(); ++i)
+    GetDescendants(children[i], nodes);
 }
 
 }  // namespace
@@ -84,11 +58,16 @@ ViewManagerConnection::~ViewManagerConnection() {
 }
 
 void ViewManagerConnection::OnConnectionEstablished() {
-  DCHECK_EQ(0, id_);  // Should only get SetClient() once.
+  DCHECK_EQ(0, id_);  // Should only get OnConnectionEstablished() once.
   id_ = root_node_manager_->GetAndAdvanceNextConnectionId();
   root_node_manager_->AddConnection(this);
+  std::vector<Node*> to_send;
+  GetUnknownNodesFrom(root_node_manager_->root(), &to_send);
+  AllocationScope allocation_scope;
   client()->OnConnectionEstablished(
-      id_, root_node_manager_->next_server_change_id());
+      id_,
+      root_node_manager_->next_server_change_id(),
+      Array<INode>::From(to_send));
 }
 
 void ViewManagerConnection::OnConnectionError() {}
@@ -109,33 +88,64 @@ View* ViewManagerConnection::GetView(const ViewId& id) {
   return root_node_manager_->GetView(id);
 }
 
-void ViewManagerConnection::NotifyNodeHierarchyChanged(
-    const NodeId& node,
-    const NodeId& new_parent,
-    const NodeId& old_parent,
-    TransportChangeId server_change_id) {
-  client()->OnNodeHierarchyChanged(NodeIdToTransportId(node),
-                                   NodeIdToTransportId(new_parent),
-                                   NodeIdToTransportId(old_parent),
-                                   server_change_id);
+void ViewManagerConnection::ProcessNodeHierarchyChanged(
+    const NodeId& node_id,
+    const NodeId& new_parent_id,
+    const NodeId& old_parent_id,
+    TransportChangeId server_change_id,
+    bool originated_change) {
+  if (originated_change || root_node_manager_->is_processing_delete_node())
+    return;
+  std::vector<Node*> to_send;
+  if (!ShouldNotifyOnHierarchyChange(node_id, new_parent_id, old_parent_id,
+                                     &to_send)) {
+    if (root_node_manager_->IsProcessingChange()) {
+      client()->OnServerChangeIdAdvanced(
+          root_node_manager_->next_server_change_id() + 1);
+    }
+    return;
+  }
+  AllocationScope allocation_scope;
+  client()->OnNodeHierarchyChanged(NodeIdToTransportId(node_id),
+                                   NodeIdToTransportId(new_parent_id),
+                                   NodeIdToTransportId(old_parent_id),
+                                   server_change_id,
+                                   Array<INode>::From(to_send));
 }
 
-void ViewManagerConnection::NotifyNodeViewReplaced(
+void ViewManagerConnection::ProcessNodeViewReplaced(
     const NodeId& node,
     const ViewId& new_view_id,
-    const ViewId& old_view_id) {
+    const ViewId& old_view_id,
+    bool originated_change) {
+  if (originated_change)
+    return;
   client()->OnNodeViewReplaced(NodeIdToTransportId(node),
                                ViewIdToTransportId(new_view_id),
                                ViewIdToTransportId(old_view_id));
 }
 
-void ViewManagerConnection::NotifyNodeDeleted(
+void ViewManagerConnection::ProcessNodeDeleted(
     const NodeId& node,
-    TransportChangeId server_change_id) {
-  client()->OnNodeDeleted(NodeIdToTransportId(node), server_change_id);
+    TransportChangeId server_change_id,
+    bool originated_change) {
+  const bool in_known = known_nodes_.erase(NodeIdToTransportId(node)) > 0;
+
+  if (originated_change)
+    return;
+
+  if (in_known) {
+    client()->OnNodeDeleted(NodeIdToTransportId(node), server_change_id);
+  } else if (root_node_manager_->IsProcessingChange()) {
+    client()->OnServerChangeIdAdvanced(
+        root_node_manager_->next_server_change_id() + 1);
+  }
 }
 
-void ViewManagerConnection::NotifyViewDeleted(const ViewId& view) {
+void ViewManagerConnection::ProcessViewDeleted(const ViewId& view,
+                                               bool originated_change) {
+  if (originated_change)
+    return;
   client()->OnViewDeleted(ViewIdToTransportId(view));
 }
 
@@ -147,7 +157,7 @@ bool ViewManagerConnection::DeleteNodeImpl(ViewManagerConnection* source,
     return false;
   RootNodeManager::ScopedChange change(
       source, root_node_manager_,
-      RootNodeManager::CHANGE_TYPE_ADVANCE_SERVER_CHANGE_ID);
+      RootNodeManager::CHANGE_TYPE_ADVANCE_SERVER_CHANGE_ID, true);
   if (node->GetParent())
     node->GetParent()->Remove(node);
   std::vector<Node*> children(node->GetChildren());
@@ -157,7 +167,7 @@ bool ViewManagerConnection::DeleteNodeImpl(ViewManagerConnection* source,
   node_map_.erase(node_id.node_id);
   delete node;
   node = NULL;
-  root_node_manager_->NotifyNodeDeleted(node_id);
+  root_node_manager_->ProcessNodeDeleted(node_id);
   return true;
 }
 
@@ -169,7 +179,7 @@ bool ViewManagerConnection::DeleteViewImpl(ViewManagerConnection* source,
     return false;
   RootNodeManager::ScopedChange change(
       source, root_node_manager_,
-      RootNodeManager::CHANGE_TYPE_DONT_ADVANCE_SERVER_CHANGE_ID);
+      RootNodeManager::CHANGE_TYPE_DONT_ADVANCE_SERVER_CHANGE_ID, false);
   if (view->node())
     view->node()->SetView(NULL);
   view_map_.erase(view_id.view_id);
@@ -177,7 +187,7 @@ bool ViewManagerConnection::DeleteViewImpl(ViewManagerConnection* source,
   // valid.
   const ViewId view_id_copy(view_id);
   delete view;
-  root_node_manager_->NotifyViewDeleted(view_id_copy);
+  root_node_manager_->ProcessViewDeleted(view_id_copy);
   return true;
 }
 
@@ -191,11 +201,56 @@ bool ViewManagerConnection::SetViewImpl(const NodeId& node_id,
     return false;
   RootNodeManager::ScopedChange change(
       this, root_node_manager_,
-      RootNodeManager::CHANGE_TYPE_DONT_ADVANCE_SERVER_CHANGE_ID);
+      RootNodeManager::CHANGE_TYPE_DONT_ADVANCE_SERVER_CHANGE_ID, false);
   node->SetView(view);
   return true;
 }
 
+void ViewManagerConnection::GetUnknownNodesFrom(
+    Node* node,
+    std::vector<Node*>* nodes) {
+  const TransportNodeId transport_id = NodeIdToTransportId(node->id());
+  if (known_nodes_.count(transport_id) == 1)
+    return;
+  nodes->push_back(node);
+  known_nodes_.insert(transport_id);
+  std::vector<Node*> children(node->GetChildren());
+  for (size_t i = 0 ; i < children.size(); ++i)
+    GetUnknownNodesFrom(children[i], nodes);
+}
+
+bool ViewManagerConnection::ShouldNotifyOnHierarchyChange(
+    const NodeId& node_id,
+    const NodeId& new_parent_id,
+    const NodeId& old_parent_id,
+    std::vector<Node*>* to_send) {
+  Node* new_parent = GetNode(new_parent_id);
+  if (new_parent) {
+    // On getting a new parent we may need to communicate new nodes to the
+    // client. We do that in the following cases:
+    // . New parent is a descendant of the root. In this case the client already
+    //   knows all ancestors, so we only have to communicate descendants of node
+    //   the client doesn't know about.
+    // . If the client knew about the parent, we have to do the same.
+    // . If the client knows about the node and is added to a tree the client
+    //   doesn't know about we have to communicate from the root down (the
+    //   client is learning about a new root).
+    if (root_node_manager_->root()->Contains(new_parent) ||
+        known_nodes_.count(NodeIdToTransportId(new_parent_id))) {
+      GetUnknownNodesFrom(GetNode(node_id), to_send);
+      return true;
+    }
+    // If parent wasn't known we have to communicate from the root down.
+    if (known_nodes_.count(NodeIdToTransportId(node_id))) {
+      GetUnknownNodesFrom(new_parent->GetRoot(), to_send);
+      return true;
+    }
+  }
+  // Otherwise only communicate the change if the node was known. We shouldn't
+  // need to communicate any nodes on a remove.
+  return known_nodes_.count(NodeIdToTransportId(node_id)) > 0;
+}
+
 void ViewManagerConnection::CreateNode(
     TransportNodeId transport_node_id,
     const Callback<void(bool)>& callback) {
@@ -212,6 +267,7 @@ void ViewManagerConnection::CreateNode(
 void ViewManagerConnection::DeleteNode(
     TransportNodeId transport_node_id,
     const Callback<void(bool)>& callback) {
+  AllocationScope allocation_scope;
   const NodeId node_id(NodeIdFromTransportId(transport_node_id));
   ViewManagerConnection* connection = root_node_manager_->GetConnection(
       node_id.connection_id);
@@ -233,7 +289,7 @@ void ViewManagerConnection::AddNode(
       success = true;
       RootNodeManager::ScopedChange change(
           this, root_node_manager_,
-          RootNodeManager::CHANGE_TYPE_ADVANCE_SERVER_CHANGE_ID);
+          RootNodeManager::CHANGE_TYPE_ADVANCE_SERVER_CHANGE_ID, false);
       parent->Add(child);
     }
   }
@@ -251,7 +307,7 @@ void ViewManagerConnection::RemoveNodeFromParent(
       success = true;
       RootNodeManager::ScopedChange change(
           this, root_node_manager_,
-          RootNodeManager::CHANGE_TYPE_ADVANCE_SERVER_CHANGE_ID);
+          RootNodeManager::CHANGE_TYPE_ADVANCE_SERVER_CHANGE_ID, false);
       node->GetParent()->Remove(node);
     }
   }
@@ -263,12 +319,11 @@ void ViewManagerConnection::GetNodeTree(
     const Callback<void(Array<INode>)>& callback) {
   AllocationScope allocation_scope;
   Node* node = GetNode(NodeIdFromTransportId(node_id));
-  Array<INode>::Builder array_builder(NodeCount(node));
-  {
-    size_t index = 0;
-    NodeToINode(node, &array_builder, &index);
-  }
-  callback.Run(array_builder.Finish());
+  std::vector<Node*> nodes;
+  GetDescendants(node, &nodes);
+  for (size_t i = 0; i < nodes.size(); ++i)
+    known_nodes_.insert(NodeIdToTransportId(nodes[i]->id()));
+  callback.Run(Array<INode>::From(nodes));
 }
 
 void ViewManagerConnection::CreateView(
@@ -322,13 +377,13 @@ void ViewManagerConnection::SetViewContents(
 void ViewManagerConnection::OnNodeHierarchyChanged(const NodeId& node,
                                                    const NodeId& new_parent,
                                                    const NodeId& old_parent) {
-  root_node_manager_->NotifyNodeHierarchyChanged(node, new_parent, old_parent);
+  root_node_manager_->ProcessNodeHierarchyChanged(node, new_parent, old_parent);
 }
 
 void ViewManagerConnection::OnNodeViewReplaced(const NodeId& node,
                                                const ViewId& new_view_id,
                                                const ViewId& old_view_id) {
-  root_node_manager_->NotifyNodeViewReplaced(node, new_view_id, old_view_id);
+  root_node_manager_->ProcessNodeViewReplaced(node, new_view_id, old_view_id);
 }
 
 }  // namespace service