Make sure to update the BookmarkIndex on a URL Change

components/bookmarks/browser/bookmark_model.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 "components/bookmarks/browser/bookmark_model.h"
 
 #include <algorithm>
 #include <functional>
 
 #include "base/bind.h"
@@ skipping 353 matching lines @@
 
   BookmarkNode* mutable_node = AsMutable(node);
   mutable_node->InvalidateFavicon();
   CancelPendingFaviconLoadRequests(mutable_node);
 
   FOR_EACH_OBSERVER(BookmarkModelObserver, observers_,
                     OnWillChangeBookmarkNode(this, node));
 
   {
     base::AutoLock url_lock(url_lock_);
     RemoveNodeFromURLSet(mutable_node);

[sky, 2014/11/03 20:44:46]

I think it would be less error prone if RemoveNodeFromURLSet did this. The name
would have to be updated then, maybe RemoveNodeFromInternalMaps or something.

A similar AddNodeToInternalMaps would be nice.

+    index_->Remove(node);
     mutable_node->set_url(url);
+    index_->Add(node);
     nodes_ordered_by_url_set_.insert(mutable_node);
   }
 
   if (store_.get())
     store_->ScheduleSave();
 
   FOR_EACH_OBSERVER(BookmarkModelObserver, observers_,
                     BookmarkNodeChanged(this, node));
 }
 
@@ skipping 618 matching lines @@
   BookmarkPermanentNode* mobile_node =
       CreatePermanentNode(BookmarkNode::MOBILE);
   return scoped_ptr<BookmarkLoadDetails>(new BookmarkLoadDetails(
       bb_node,
       other_node,
       mobile_node,
       client_->GetLoadExtraNodesCallback(),
       new BookmarkIndex(client_, accept_languages),
       next_node_id_));
 }