BookmarkModel::ReorderChildren to take const BookmarkNodes.

chrome/browser/bookmarks/bookmark_model.cc

Index: chrome/browser/bookmarks/bookmark_model.cc
diff --git a/chrome/browser/bookmarks/bookmark_model.cc b/chrome/browser/bookmarks/bookmark_model.cc
index 493395a528b5f19fd26550d1c59635a5cfa25ce0..dd813334df75f4b73523e20aac3c1dd53dfb4c78 100644
--- a/chrome/browser/bookmarks/bookmark_model.cc
+++ b/chrome/browser/bookmarks/bookmark_model.cc
@@ -675,7 +675,7 @@ void BookmarkModel::SortChildren(const BookmarkNode* 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)
@@ -684,7 +684,13 @@ void BookmarkModel::ReorderChildren(
   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();