BookmarkModel::ReorderChildren to take const BookmarkNodes.

BookmarkModel::ReorderChildren to take const BookmarkNodes.

For consistency with other member functions ReorderChildren now takes
const BookmarkNode pointers.

BUG=126092
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=215845

[Tom Cassiotis, 2013-08-01 02:00:52]

sky@ please take a look.

Discussed here:
https://codereview.chromium.org/19287013/diff/8001/chrome/browser/undo/bookma...

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

https://codereview.chromium.org/20521005/diff/9001/chrome/browser/bookmarks/b...
File chrome/browser/bookmarks/bookmark_model.cc (right):

https://codereview.chromium.org/20521005/diff/9001/chrome/browser/bookmarks/b...
chrome/browser/bookmarks/bookmark_model.cc:693:
AsMutable(parent)->SetChildren(nodes);
Can you use a const_cast here on ordered_nodes?

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

sky@ PTAL

https://codereview.chromium.org/20521005/diff/9001/chrome/browser/bookmarks/b...
File chrome/browser/bookmarks/bookmark_model.cc (right):

https://codereview.chromium.org/20521005/diff/9001/chrome/browser/bookmarks/b...
chrome/browser/bookmarks/bookmark_model.cc:693:
AsMutable(parent)->SetChildren(nodes);
On 2013/08/05 14:53:52, sky wrote:
> Can you use a const_cast here on ordered_nodes?

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:44]

https://codereview.chromium.org/20521005/diff/9001/chrome/browser/bookmarks/b...
File chrome/browser/bookmarks/bookmark_model.cc (right):

https://codereview.chromium.org/20521005/diff/9001/chrome/browser/bookmarks/b...
chrome/browser/bookmarks/bookmark_model.cc:693:
AsMutable(parent)->SetChildren(nodes);
On 2013/08/05 19:34:49, Tom Cassiotis wrote:
> On 2013/08/05 14:53:52, sky wrote:
> > Can you use a const_cast here on ordered_nodes?
> 
> 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?

I think reinterpret_cast is fine here.

[Tom Cassiotis, 2013-08-05 21:15:36]

sky@ PTAL

[sky, 2013-08-05 23:53:50]

LGTM

[commit-bot: I haz the power, 2013-08-06 04:16:18]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/tom.cassiotis@gmail.com/20521005/21001

[commit-bot: I haz the power, 2013-08-06 07:46:47]

Change committed as 215845