First step at setroots

Still a work in progress, but some of the basics are there.

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

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

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

mojo/mojo_services.gypi

@@ -239,8 +239,6 @@
             'services/view_manager/root_node_manager.h',
             'services/view_manager/root_view_manager.cc',
             'services/view_manager/root_view_manager.h',
-            'services/view_manager/type_converters.cc',
-            'services/view_manager/type_converters.h',
             'services/view_manager/view.cc',
             'services/view_manager/view.h',
             'services/view_manager/view_manager_connection.cc',

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

@@ -39,6 +39,9 @@ interface IViewManager {
   // . |parent| or |child| does not identify a valid node.
   // . |child| is an ancestor of |parent|.
   // . |child| is already a child of |parent|.
+  //
+  // This may result in a connection getting OnNodeDeleted(). See
+  // RemoveNodeFromParent for details.
   AddNode(uint32 parent,
           uint32 child,
           uint32 server_change_id) => (bool success);
@@ -46,6 +49,12 @@ interface IViewManager {
   // Removes a view from its current parent. See description above class for
   // details of |change_id|. This fails if the node is not valid,
   // |server_change_id| doesn't match, or the node already has no parent.
+  //
+  // Removing a node from a parent may result in OnNodeDeleted() being sent to
+  // other connections. For example, connection A has nodes 1 and 2, with 2 a
+  // child of 1. Connection B has a root 1. If 2 is removed from 1 then B gets
+  // OnNodeDeleted(). This is done as node 2 is effectively no longer visible to
+  // connection B.
   RemoveNodeFromParent(uint32 node_id,
                        uint32 server_change_id) => (bool success);
 
@@ -72,6 +81,11 @@ interface IViewManager {
   SetViewContents(uint32 view_id,
                   handle<shared_buffer> buffer,
                   uint32 buffer_size);
+
+  // Sets the ids of the roots for the specified connection.
+  // TODO(sky): this is temporary for testing. This needs to be conveyed at
+  // creation time of a new connection.
+  SetRoots(uint16 connection_id, uint32[] nodes) => (bool success);
 };
 
 // Changes to nodes/views are not sent to the connection that originated the

mojo/services/view_manager/type_converters.cc

-// Copyright 2014 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/services/view_manager/type_converters.h"
-
-#include "mojo/public/cpp/bindings/buffer.h"
-#include "mojo/services/public/interfaces/view_manager/view_manager.mojom.h"
-#include "mojo/services/view_manager/ids.h"
-#include "mojo/services/view_manager/node.h"
-#include "mojo/services/view_manager/view.h"
-
-using mojo::view_manager::INode;
-using mojo::view_manager::service::Node;
-using mojo::view_manager::service::NodeId;
-using mojo::view_manager::service::ViewId;
-
-namespace mojo {
-
-// static
-INode TypeConverter<INode, const Node*>::ConvertFrom(const Node* node,
-                                                     Buffer* buf) {
-  DCHECK(node);
-
-  INode::Builder builder(buf);
-  const 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()));
-  return builder.Finish();
-}
-
-}  // namespace mojo

mojo/services/view_manager/type_converters.h

-// Copyright 2014 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.
-
-#ifndef MOJO_SERVICES_VIEW_MANAGER_TYPE_CONVERTERS_H_
-#define MOJO_SERVICES_VIEW_MANAGER_TYPE_CONVERTERS_H_
-
-#include "mojo/public/cpp/bindings/type_converter.h"
-
-namespace mojo {
-
-class Buffer;
-
-namespace view_manager {
-
-class INode;
-
-namespace service {
-class Node;
-}  // namespace service
-}  // namespace view_manager
-
-template <>
-class TypeConverter<view_manager::INode,
-                    const view_manager::service::Node*> {
- public:
-  static view_manager::INode ConvertFrom(
-      const view_manager::service::Node* node,
-      Buffer* buf);
-
-  MOJO_ALLOW_IMPLICIT_TYPE_CONVERSION();
-};
-
-}  // namespace mojo
-
-#endif  // MOJO_SERVICES_VIEW_MANAGER_TYPE_CONVERTERS_H_

mojo/services/view_manager/view_manager_connection.cc

@@ -8,7 +8,6 @@
 #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"
@@ -67,7 +66,7 @@ void ViewManagerConnection::OnConnectionEstablished() {
   client()->OnConnectionEstablished(
       id_,
       root_node_manager_->next_server_change_id(),
-      Array<INode>::From(to_send));
+      NodesToINodes(to_send));
 }
 
 const Node* ViewManagerConnection::GetNode(const NodeId& id) const {
@@ -92,10 +91,24 @@ void ViewManagerConnection::ProcessNodeHierarchyChanged(
     const Node* old_parent,
     TransportChangeId server_change_id,
     bool originated_change) {
+  if (known_nodes_.count(NodeIdToTransportId(node->id())) > 0) {
+    if (originated_change)
+      return;
+    if (node->id().connection_id != id_ && !IsNodeDescendantOfRoots(node)) {
+      // Node was a descendant of roots and is no longer, treat it as though the
+      // node was deleted.
+      RemoveFromKnown(node);
+      client()->OnNodeDeleted(NodeIdToTransportId(node->id()),
+                              server_change_id);
+      return;
+    }
+  }
+
   if (originated_change || root_node_manager_->is_processing_delete_node())
     return;
   std::vector<const Node*> to_send;
-  if (!ShouldNotifyOnHierarchyChange(node, new_parent, old_parent, &to_send)) {
+  if (!ShouldNotifyOnHierarchyChange(node, &new_parent, &old_parent,
+                                     &to_send)) {
     if (root_node_manager_->IsProcessingChange()) {
       client()->OnServerChangeIdAdvanced(
           root_node_manager_->next_server_change_id() + 1);
@@ -105,11 +118,15 @@ void ViewManagerConnection::ProcessNodeHierarchyChanged(
   AllocationScope allocation_scope;
   const NodeId new_parent_id(new_parent ? new_parent->id() : NodeId());
   const NodeId old_parent_id(old_parent ? old_parent->id() : NodeId());
+  DCHECK((node->id().connection_id == id_) ||
+         (roots_.count(NodeIdToTransportId(node->id())) > 0) ||
+         (new_parent && IsNodeDescendantOfRoots(new_parent)) ||
+         (old_parent && IsNodeDescendantOfRoots(old_parent)));
   client()->OnNodeHierarchyChanged(NodeIdToTransportId(node->id()),
                                    NodeIdToTransportId(new_parent_id),
                                    NodeIdToTransportId(old_parent_id),
                                    server_change_id,
-                                   Array<INode>::From(to_send));
+                                   NodesToINodes(to_send));
 }
 
 void ViewManagerConnection::ProcessNodeViewReplaced(
@@ -117,16 +134,14 @@ void ViewManagerConnection::ProcessNodeViewReplaced(
     const View* new_view,
     const View* old_view,
     bool originated_change) {
-  if (originated_change)
+  if (originated_change || !known_nodes_.count(NodeIdToTransportId(node->id())))
     return;
-  if (known_nodes_.count(NodeIdToTransportId(node->id())) > 0) {
   const TransportViewId new_view_id = new_view ?
       ViewIdToTransportId(new_view->id()) : 0;
   const TransportViewId old_view_id = old_view ?
       ViewIdToTransportId(old_view->id()) : 0;
   client()->OnNodeViewReplaced(NodeIdToTransportId(node->id()),
                                new_view_id, old_view_id);
-  }
 }
 
 void ViewManagerConnection::ProcessNodeDeleted(
@@ -231,29 +246,66 @@ void ViewManagerConnection::GetUnknownNodesFrom(
     GetUnknownNodesFrom(children[i], nodes);
 }
 
+void ViewManagerConnection::RemoveFromKnown(const Node* node) {
+  if (node->id().connection_id == id_)
+    return;
+  known_nodes_.erase(NodeIdToTransportId(node->id()));
+  std::vector<const Node*> children = node->GetChildren();
+  for (size_t i = 0; i < children.size(); ++i)
+    RemoveFromKnown(children[i]);
+}
+
+bool ViewManagerConnection::IsNodeDescendantOfRoots(const Node* node) const {
+  if (roots_.empty())
+    return true;
+  if (!node)
+    return false;
+  for (NodeIdSet::const_iterator i = roots_.begin(); i != roots_.end(); ++i) {
+    const Node* root = GetNode(NodeIdFromTransportId(*i));
+    DCHECK(root);
+    if (root->Contains(node))
+      return true;
+  }
+  return false;
+}
+
 bool ViewManagerConnection::ShouldNotifyOnHierarchyChange(
     const Node* node,
-    const Node* new_parent,
-    const Node* old_parent,
+    const Node** new_parent,
+    const Node** old_parent,
     std::vector<const Node*>* to_send) {
-  if (new_parent) {
+  // If the node is not in |roots_| or was never known to this connection then
+  // don't notify the client about it.
+  if (node->id().connection_id != id_ &&
+      known_nodes_.count(NodeIdToTransportId(node->id())) == 0 &&
+      !IsNodeDescendantOfRoots(node)) {
+    return false;
+  }
+  if (!IsNodeDescendantOfRoots(*new_parent))
+    *new_parent = NULL;
+  if (!IsNodeDescendantOfRoots(*old_parent))
+    *old_parent = NULL;
+
+  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.
+    // . New parent is a descendant of the roots. 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()))) {
+    if (root_node_manager_->root()->Contains(*new_parent) ||
+        known_nodes_.count(NodeIdToTransportId((*new_parent)->id()))) {
       GetUnknownNodesFrom(node, 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);
+      // No need to check against |roots_| as client should always know it's
+      // |roots_|.
+      GetUnknownNodesFrom((*new_parent)->GetRoot(), to_send);
       return true;
     }
   }
@@ -262,6 +314,62 @@ bool ViewManagerConnection::ShouldNotifyOnHierarchyChange(
   return known_nodes_.count(NodeIdToTransportId(node->id())) > 0;
 }
 
+bool ViewManagerConnection::ProcessSetRoots(
+    TransportConnectionId source_connection_id,
+    const Array<TransportNodeId>& transport_node_ids) {
+  // TODO(sky): these DCHECKs can go away once this is part of a real API. Also
+  // make sure that when roots are set nodes are communicate to client. Code in
+  // ProcessNodeHierarchyChanged() is depending on this.
+  DCHECK(node_map_.empty());
+  DCHECK(view_map_.empty());
+
+  NodeIdSet roots;
+  for (size_t i = 0; i < transport_node_ids.size(); ++i) {
+    const Node* node = GetNode(NodeIdFromTransportId(transport_node_ids[i]));
+    // Only allow setting roots that are owned by the source connection.
+    if (!node || node->id().connection_id != source_connection_id)
+      return false;
+    roots.insert(transport_node_ids[i]);
+  }
+  roots_.swap(roots);
+
+  // TODO(sky): remove |known_nodes_.clear()| temporary while this is done here
+  // instead of at creation time.
+  known_nodes_.clear();
+  std::vector<const Node*> to_send;
+  for (NodeIdSet::const_iterator i = roots_.begin(); i != roots_.end(); ++i)
+    GetUnknownNodesFrom(GetNode(NodeIdFromTransportId(*i)), &to_send);
+  AllocationScope allocation_scope;
+  client()->OnConnectionEstablished(
+      id_,
+      root_node_manager_->next_server_change_id(),
+      NodesToINodes(to_send));
+
+  return true;
+}
+
+Array<INode> ViewManagerConnection::NodesToINodes(
+    const std::vector<const Node*>& nodes) {
+  Array<INode>::Builder array_builder(nodes.size());
+  for (size_t i = 0; i < nodes.size(); ++i) {
+    INode::Builder node_builder;
+    const Node* node = nodes[i];
+    DCHECK(known_nodes_.count(NodeIdToTransportId(node->id())) > 0);
+    const Node* parent = node->GetParent();
+    // If the parent isn't known, it means the parent is not visible to us (not
+    // in roots), and should not be sent over.
+    if (parent && known_nodes_.count(NodeIdToTransportId(parent->id())) == 0)
+      parent = NULL;
+    node_builder.set_parent_id(NodeIdToTransportId(
+                                   parent ? parent->id() : NodeId()));
+    node_builder.set_node_id(NodeIdToTransportId(node->id()));
+    node_builder.set_view_id(ViewIdToTransportId(
+        node->view() ? node->view()->id() : ViewId()));
+    array_builder[i] = node_builder.Finish();
+  }
+  return array_builder.Finish();
+}
+
 void ViewManagerConnection::CreateNode(
     TransportNodeId transport_node_id,
     const Callback<void(bool)>& callback) {
@@ -338,7 +446,7 @@ void ViewManagerConnection::GetNodeTree(
   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));
+  callback.Run(NodesToINodes(nodes));
 }
 
 void ViewManagerConnection::CreateView(
@@ -393,6 +501,16 @@ void ViewManagerConnection::SetViewContents(
   UnmapBuffer(handle_data);
 }
 
+void ViewManagerConnection::SetRoots(
+    TransportConnectionId connection_id,
+    const Array<TransportNodeId>& transport_node_ids,
+    const Callback<void(bool)>& callback) {
+  ViewManagerConnection* connection =
+      root_node_manager_->GetConnection(connection_id);
+  callback.Run(connection &&
+               connection->ProcessSetRoots(id_, transport_node_ids));
+}
+
 void ViewManagerConnection::OnNodeHierarchyChanged(const Node* node,
                                                    const Node* new_parent,
                                                    const Node* old_parent) {

mojo/services/view_manager/view_manager_connection.h

@@ -96,14 +96,30 @@ class MOJO_VIEW_MANAGER_EXPORT ViewManagerConnection
   // to |nodes|, marks |node| as known and recurses.
   void GetUnknownNodesFrom(const Node* node, std::vector<const Node*>* nodes);
 
+  // Removes |node| and all its descendants from |known_nodes_|. This does not
+  // recurse through nodes that were created by this connection.
+  void RemoveFromKnown(const Node* node);
+
+  // Returns true if |node| is a non-null and a descendant of |roots_| (or
+  // |roots_| is empty).
+  bool IsNodeDescendantOfRoots(const Node* node) const;
+
   // Returns true if notification should be sent of a hierarchy change. If true
   // is returned, any nodes that need to be sent to the client are added to
   // |to_send|.
-  bool ShouldNotifyOnHierarchyChange(const Node* node_id,
-                                     const Node* new_parent_id,
-                                     const Node* old_parent_id,
+  bool ShouldNotifyOnHierarchyChange(const Node* node,
+                                     const Node** new_parent,
+                                     const Node** old_parent,
                                      std::vector<const Node*>* to_send);
 
+  bool ProcessSetRoots(TransportConnectionId source_connection_id,
+                       const Array<TransportNodeId>& transport_node_ids);
+
+  // Converts an array of Nodes to INodes. This assumes all the nodes are valid
+  // for the client. The parent of nodes the client is not allowed to see are
+  // set to NULL (in the returned INodes).
+  Array<INode> NodesToINodes(const std::vector<const Node*>& nodes);
+
   // Overridden from IViewManager:
   virtual void CreateNode(TransportNodeId transport_node_id,
                           const Callback<void(bool)>& callback) OVERRIDE;
@@ -130,6 +146,10 @@ class MOJO_VIEW_MANAGER_EXPORT ViewManagerConnection
   virtual void SetViewContents(TransportViewId view_id,
                                ScopedSharedBufferHandle buffer,
                                uint32_t buffer_size) OVERRIDE;
+  virtual void SetRoots(
+      TransportConnectionId connection_id,
+      const Array<TransportNodeId>& transport_node_ids,
+      const Callback<void(bool)>& callback) OVERRIDE;
 
   // Overridden from NodeDelegate:
   virtual void OnNodeHierarchyChanged(const Node* node,
@@ -152,6 +172,10 @@ class MOJO_VIEW_MANAGER_EXPORT ViewManagerConnection
   // The set of nodes that has been communicated to the client.
   NodeIdSet known_nodes_;
 
+  // This is the set of nodes the client can see. The client can not delete or
+  // move these.
+  NodeIdSet roots_;
+
   DISALLOW_COPY_AND_ASSIGN(ViewManagerConnection);
 };
 

mojo/services/view_manager/view_manager_connection_unittest.cc

@@ -10,6 +10,7 @@
 #include "base/message_loop/message_loop.h"
 #include "base/run_loop.h"
 #include "base/strings/stringprintf.h"
+#include "mojo/common/common_type_converters.h"
 #include "mojo/public/cpp/bindings/allocation_scope.h"
 #include "mojo/public/cpp/environment/environment.h"
 #include "mojo/public/cpp/shell/connect.h"
@@ -20,6 +21,21 @@
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace mojo {
+
+// TODO(sky): remove this when Darin is done with cleanup.
+template <typename T>
+class MOJO_COMMON_EXPORT TypeConverter<T, T> {
+ public:
+  static T ConvertFrom(T input, Buffer* buf) {
+    return input;
+  }
+  static T ConvertTo(T input) {
+    return input;
+  }
+
+  MOJO_ALLOW_IMPLICIT_TYPE_CONVERSION();
+};
+
 namespace view_manager {
 namespace service {
 
@@ -30,6 +46,8 @@ base::RunLoop* current_run_loop = NULL;
 // Sets |current_run_loop| and runs it. It is expected that someone else quits
 // the loop.
 void DoRunLoop() {
+  DCHECK(!current_run_loop);
+
   base::RunLoop run_loop;
   current_run_loop = &run_loop;
   current_run_loop->Run();
@@ -175,6 +193,17 @@ bool SetView(IViewManager* view_manager,
   return result;
 }
 
+bool SetRoots(IViewManager* view_manager,
+              TransportConnectionId connection_id,
+              const std::vector<uint32_t>& node_ids) {
+  bool result = false;
+  view_manager->SetRoots(connection_id,
+                         Array<uint32_t>::From(node_ids),
+                         base::Bind(&BooleanCallback, &result));
+  DoRunLoop();
+  return result;
+}
+
 }  // namespace
 
 typedef std::vector<std::string> Changes;
@@ -204,9 +233,13 @@ class ViewManagerClientImpl : public IViewManagerClient {
     return changes;
   }
 
+  void ClearId() {
+    id_ = 0;
+  }
+
   void WaitForId() {
-    if (id_ == 0)
-      DoRunLoop();
+    DCHECK_EQ(0, id_);
+    DoRunLoopUntilChangesCount(1);
   }
 
   void DoRunLoopUntilChangesCount(size_t count) {
@@ -224,9 +257,10 @@ class ViewManagerClientImpl : public IViewManagerClient {
       const mojo::Array<INode>& nodes) OVERRIDE {
     id_ = connection_id;
     next_server_change_id_ = next_server_change_id;
+    initial_nodes_.clear();
     INodesToTestNodes(nodes, &initial_nodes_);
-    if (current_run_loop)
-      current_run_loop->Quit();
+    changes_.push_back("OnConnectionEstablished");
+    QuitIfNecessary();
   }
   virtual void OnServerChangeIdAdvanced(
       uint32_t next_server_change_id) OVERRIDE {
@@ -314,6 +348,7 @@ class ViewManagerConnectionTest : public testing::Test {
     view_manager_->SetClient(&client_);
 
     client_.WaitForId();
+    client_.GetAndClearChanges();
   }
 
  protected:
@@ -323,6 +358,7 @@ class ViewManagerConnectionTest : public testing::Test {
     view_manager2_->SetClient(&client2_);
 
     client2_.WaitForId();
+    client2_.GetAndClearChanges();
   }
 
   void DestroySecondConnection() {
@@ -1092,6 +1128,95 @@ TEST_F(ViewManagerConnectionTest, GetNodeTree) {
   }
 }
 
+TEST_F(ViewManagerConnectionTest, SetRoots) {
+  // Create 1, 2, and 3 in the first connection.
+  ASSERT_TRUE(CreateNode(view_manager_.get(), 1, 1));
+  ASSERT_TRUE(CreateNode(view_manager_.get(), 1, 2));
+  ASSERT_TRUE(CreateNode(view_manager_.get(), 1, 3));
+
+  // Parent 1 to the root.
+  ASSERT_TRUE(AddNode(view_manager_.get(),
+                      CreateNodeId(0, 1),
+                      CreateNodeId(client_.id(), 1),
+                      1));
+
+  // Establish the second connection and give it the roots 1 and 3.
+  EstablishSecondConnection();
+  client2_.ClearId();
+  {
+    AllocationScope scope;
+    std::vector<uint32_t> roots;
+    roots.push_back(CreateNodeId(1, 1));
+    roots.push_back(CreateNodeId(1, 3));
+    ASSERT_TRUE(SetRoots(view_manager_.get(), 2, roots));
+    client2_.DoRunLoopUntilChangesCount(1);
+    Changes changes(client2_.GetAndClearChanges());
+    ASSERT_EQ(1u, changes.size());
+    EXPECT_EQ("OnConnectionEstablished", changes[0]);
+    ASSERT_NE(0u, client2_.id());
+    const std::vector<TestNode>& nodes(client2_.initial_nodes());
+    ASSERT_EQ(2u, nodes.size());
+    EXPECT_EQ("node=1,1 parent=null view=null", nodes[0].ToString());
+    EXPECT_EQ("node=1,3 parent=null view=null", nodes[1].ToString());
+  }
+
+  // Create 4 and add it to the root, connection 2 should only get id advanced.
+  {
+    ASSERT_TRUE(CreateNode(view_manager_.get(), 1, 4));
+    ASSERT_TRUE(AddNode(view_manager_.get(),
+                        CreateNodeId(0, 1),
+                        CreateNodeId(client_.id(), 4),
+                        2));
+    client2_.DoRunLoopUntilChangesCount(1);
+    Changes changes(client2_.GetAndClearChanges());
+    ASSERT_EQ(1u, changes.size());
+    EXPECT_EQ("ServerChangeIdAdvanced 3", changes[0]);
+  }
+
+  // Move 4 under 3, this should expose 4 to the client.
+  {
+    ASSERT_TRUE(AddNode(view_manager_.get(),
+                        CreateNodeId(1, 3),
+                        CreateNodeId(1, 4),
+                        3));
+    client2_.DoRunLoopUntilChangesCount(1);
+    Changes changes(client2_.GetAndClearChanges());
+    ASSERT_EQ(1u, changes.size());
+    EXPECT_EQ(
+        "HierarchyChanged change_id=3 node=1,4 new_parent=1,3 "
+        "old_parent=null", changes[0]);
+    const std::vector<TestNode>& nodes(client2_.hierarchy_changed_nodes());
+    ASSERT_EQ(1u, nodes.size());
+    EXPECT_EQ("node=1,4 parent=1,3 view=null", nodes[0].ToString());
+  }
+
+  // Move 4 under 2, since 2 isn't a root client should get a delete.
+  {
+    ASSERT_TRUE(AddNode(view_manager_.get(),
+                        CreateNodeId(1, 2),
+                        CreateNodeId(1, 4),
+                        4));
+    client2_.DoRunLoopUntilChangesCount(1);
+    Changes changes(client2_.GetAndClearChanges());
+    ASSERT_EQ(1u, changes.size());
+    EXPECT_EQ("NodeDeleted change_id=4 node=1,4", changes[0]);
+  }
+
+  // Delete 4, client shouldn't receive a delete since it should no longer know
+  // about 4.
+  {
+    ASSERT_TRUE(DeleteNode(view_manager_.get(), CreateNodeId(client_.id(), 4)));
+    ASSERT_TRUE(client_.GetAndClearChanges().empty());
+
+    client2_.DoRunLoopUntilChangesCount(1);
+    Changes changes(client2_.GetAndClearChanges());
+    ASSERT_EQ(1u, changes.size());
+    EXPECT_EQ("ServerChangeIdAdvanced 6", changes[0]);
+  }
+}
+
+// TODO: add tests that verify can't manipulate trees of uknown nodes.
+
 }  // namespace service
 }  // namespace view_manager
 }  // namespace mojo