Cleans up usage of ViewManagerServiceImpl::roots_

mojo/services/public/cpp/view_manager/lib/view_manager_client_impl.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/public/cpp/view_manager/lib/view_manager_client_impl.h"
 
 #include "base/bind.h"
 #include "base/message_loop/message_loop.h"
+#include "base/stl_util.h"
 #include "mojo/public/cpp/application/application_connection.h"
 #include "mojo/public/cpp/application/connect.h"
 #include "mojo/public/interfaces/service_provider/service_provider.mojom.h"
 #include "mojo/services/public/cpp/view_manager/lib/node_private.h"
 #include "mojo/services/public/cpp/view_manager/lib/view_private.h"
 #include "mojo/services/public/cpp/view_manager/node_observer.h"
 #include "mojo/services/public/cpp/view_manager/util.h"
 #include "mojo/services/public/cpp/view_manager/view_manager_delegate.h"
 #include "mojo/services/public/cpp/view_manager/view_observer.h"
 #include "mojo/services/public/cpp/view_manager/window_manager_delegate.h"
@@ skipping 33 matching lines @@
     private_view.set_node(node);
     // TODO(beng): this broadcasts notifications locally... do we want this? I
     //             don't think so. same story for LocalAddChild above!
     private_node.LocalSetActiveView(view);
     client->AddView(view);
   }
   return node;
 }
 
 Node* BuildNodeTree(ViewManagerClientImpl* client,
-                    const Array<NodeDataPtr>& nodes) {
+                    const Array<NodeDataPtr>& nodes,
+                    Node* initial_parent) {
   std::vector<Node*> parents;
   Node* root = NULL;
   Node* last_node = NULL;
+  if (initial_parent)
+    parents.push_back(initial_parent);
   for (size_t i = 0; i < nodes.size(); ++i) {
     if (last_node && nodes[i]->parent_id == last_node->id()) {
       parents.push_back(last_node);
     } else if (!parents.empty()) {
       while (parents.back()->id() != nodes[i]->parent_id)
         parents.pop_back();
     }
     Node* node = AddNodeToViewManager(
         client,
         !parents.empty() ? parents.back() : NULL,
@@ skipping 54 matching lines @@
 
 ViewManagerClientImpl::ViewManagerClientImpl(ViewManagerDelegate* delegate)
     : connected_(false),
       connection_id_(0),
       next_id_(1),
       delegate_(delegate),
       window_manager_delegate_(NULL) {
 }
 
 ViewManagerClientImpl::~ViewManagerClientImpl() {
+  std::vector<Node*> non_owned;
   while (!nodes_.empty()) {
     IdToNodeMap::iterator it = nodes_.begin();
-    if (OwnsNode(it->second->id()))
+    if (OwnsNode(it->second->id())) {
       it->second->Destroy();
-    else
+    } else {
+      non_owned.push_back(it->second);
       nodes_.erase(it);
+    }
   }
   while (!views_.empty()) {
     IdToViewMap::iterator it = views_.begin();
     if (OwnsView(it->second->id()))
       it->second->Destroy();
     else
       views_.erase(it);
   }
+  // Delete the non-owned nodes last. In the typical case these are roots. The
+  // exception is the window manager, which may know aboutother random nodes
+  // that it doesn't own.
+  // NOTE: we manually delete as we're a friend.
+  for (size_t i = 0; i < non_owned.size(); ++i)
+    delete non_owned[i];
   delegate_->OnViewManagerDisconnected(this);
 }
 
 Id ViewManagerClientImpl::CreateNode() {
   DCHECK(connected_);
   const Id node_id = MakeTransportId(connection_id_, ++next_id_);
   service_->CreateNode(node_id, ActionCompletedCallbackWithErrorCode());
   return node_id;
 }
 
@@ skipping 153 matching lines @@
 ////////////////////////////////////////////////////////////////////////////////
 // ViewManagerClientImpl, ViewManagerClient implementation:
 
 void ViewManagerClientImpl::OnViewManagerConnectionEstablished(
     ConnectionSpecificId connection_id,
     const String& creator_url,
     Array<NodeDataPtr> nodes) {
   connected_ = true;
   connection_id_ = connection_id;
   creator_url_ = TypeConverter<String, std::string>::ConvertFrom(creator_url);
-  AddRoot(BuildNodeTree(this, nodes));
+  AddRoot(BuildNodeTree(this, nodes, NULL));
 }
 
 void ViewManagerClientImpl::OnRootAdded(Array<NodeDataPtr> nodes) {
-  AddRoot(BuildNodeTree(this, nodes));
+  AddRoot(BuildNodeTree(this, nodes, NULL));
 }
 
 void ViewManagerClientImpl::OnNodeBoundsChanged(Id node_id,
                                                 RectPtr old_bounds,
                                                 RectPtr new_bounds) {
   Node* node = GetNodeById(node_id);
   NodePrivate(node).LocalSetBounds(old_bounds.To<gfx::Rect>(),
                                    new_bounds.To<gfx::Rect>());
 }
 
 void ViewManagerClientImpl::OnNodeHierarchyChanged(
     Id node_id,
     Id new_parent_id,
     Id old_parent_id,
     mojo::Array<NodeDataPtr> nodes) {
-  BuildNodeTree(this, nodes);
+  Node* initial_parent = nodes.size() ?
+      GetNodeById(nodes[0]->parent_id) : NULL;
+
+  BuildNodeTree(this, nodes, initial_parent);
 
   Node* new_parent = GetNodeById(new_parent_id);
   Node* old_parent = GetNodeById(old_parent_id);
   Node* node = GetNodeById(node_id);
   if (new_parent)
     NodePrivate(new_parent).LocalAddChild(node);
   else
     NodePrivate(old_parent).LocalRemoveChild(node);
 }
 
@@ skipping 112 matching lines @@
                     base::Unretained(this));
 }
 
 base::Callback<void(ErrorCode)>
     ViewManagerClientImpl::ActionCompletedCallbackWithErrorCode() {
   return base::Bind(&ViewManagerClientImpl::OnActionCompletedWithErrorCode,
                     base::Unretained(this));
 }
 
 }  // namespace mojo