BookmarkModel::ReorderChildren to take const BookmarkNodes.

chrome/browser/bookmarks/bookmark_model.cc

 // Copyright (c) 2012 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 "chrome/browser/bookmarks/bookmark_model.h"
 
 #include <algorithm>
 #include <functional>
 
 #include "base/bind.h"
@@ skipping 657 matching lines @@
 
   if (store_.get())
     store_->ScheduleSave();
 
   FOR_EACH_OBSERVER(BookmarkModelObserver, observers_,
                     BookmarkNodeChildrenReordered(this, parent));
 }
 
 void BookmarkModel::ReorderChildren(
     const BookmarkNode* parent,
-    const std::vector<BookmarkNode*>& ordered_nodes) {
+    const std::vector<const BookmarkNode*>& ordered_nodes) {
   // Ensure that all children in |parent| are in |ordered_nodes|.
   DCHECK_EQ(static_cast<size_t>(parent->child_count()), ordered_nodes.size());
   for (size_t i = 0; i < ordered_nodes.size(); ++i)
     DCHECK_EQ(parent, ordered_nodes[i]->parent());
 
   FOR_EACH_OBSERVER(BookmarkModelObserver, observers_,
                     OnWillReorderBookmarkNode(this, parent));
 
-  AsMutable(parent)->SetChildren(ordered_nodes);
+  // SetChildren requires non-const nodes so |ordered_nodes| is reconstructed.
+  std::vector<BookmarkNode*> nodes;
+  nodes.reserve(ordered_nodes.size());
+  for (size_t i = 0; i < ordered_nodes.size(); ++i)
+    nodes.push_back(AsMutable(ordered_nodes[i]));
+
+  AsMutable(parent)->SetChildren(nodes);

[sky, 2013/08/05 14:53:52]

Can you use a const_cast here on ordered_nodes?

[Tom Cassiotis, 2013/08/05 19:34:49]

const_cast is not able to be used here because the vector<const BookmarkNode*>
is a different type to vector<BookmarkNode*> for const_cast.

I can however use reinterpret_cast to make this work:
AsMutable(parent)->SetChildren(*(reinterpret_cast<const
std::vector<BookmarkNode*>* >(&ordered_nodes)));

The style guide gives a warning but does not forbid the use of reinterpret_cast:
Use reinterpret_cast to do unsafe conversions of pointer types to and from
integer and other pointer types. Use this only if you know what you are doing
and you understand the aliasing issues.

Would you prefer with a cast over recreating the vector?

[sky, 2013/08/05 20:07:45]

I think reinterpret_cast is fine here.

 
   if (store_.get())
     store_->ScheduleSave();
 
   FOR_EACH_OBSERVER(BookmarkModelObserver, observers_,
                     BookmarkNodeChildrenReordered(this, parent));
 }
 
 void BookmarkModel::SetDateFolderModified(const BookmarkNode* parent,
                                           const Time time) {
@@ skipping 350 matching lines @@
   BookmarkPermanentNode* bb_node =
       CreatePermanentNode(BookmarkNode::BOOKMARK_BAR);
   BookmarkPermanentNode* other_node =
       CreatePermanentNode(BookmarkNode::OTHER_NODE);
   BookmarkPermanentNode* mobile_node =
       CreatePermanentNode(BookmarkNode::MOBILE);
   return new BookmarkLoadDetails(bb_node, other_node, mobile_node,
                                  new BookmarkIndex(profile_),
                                  next_node_id_);
 }