Changes the bookmark model to allow more than one bookmark to

reference the same url. Clicking the star button edits the most
recently added bookmark for the URL. Dragging a button/star always
does a move, otherwise drops on the bookmark bar create a new
bookmark.

Also changed the add page context menu for the bookmark bar to
remember where you invoked it from.

BUG=1173228 1678

Review URL: http://codereview.chromium.org/3203

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@2413 0039d316-1c4b-4281-b951-d872f2087c98

22 files changed, 510 insertions, 316 deletions

diff --git a/chrome/browser/bookmark_bar_context_menu_controller.cc b/chrome/browser/bookmark_bar_context_menu_controller.cc
index 0ba0348..cb195ef 100644
--- a/chrome/browser/bookmark_bar_context_menu_controller.cc
+++ b/chrome/browser/bookmark_bar_context_menu_controller.cc
@@ -297,8 +297,7 @@ void BookmarkBarContextMenuController::ExecuteCommand(int id) {
 
       if (node_->GetType() == history::StarredEntry::URL) {
         BookmarkEditorView::Show(view_->GetViewContainer()->GetHWND(),
-                                 view_->GetProfile(), node_->GetURL(),
-                                 node_->GetTitle());
+                                 view_->GetProfile(), NULL, node_);
       } else {
         // Controller deletes itself when done.
         EditFolderController* controller = new EditFolderController(
@@ -319,7 +318,7 @@ void BookmarkBarContextMenuController::ExecuteCommand(int id) {
       UserMetrics::RecordAction(L"BookmarkBar_ContextMenu_Add", profile);
 
       BookmarkEditorView::Show(view_->GetViewContainer()->GetHWND(),
-                               view_->GetProfile(), GURL(), std::wstring());
+                               view_->GetProfile(), node_, NULL);
       break;
     }
 
@@ -373,4 +372,3 @@ BookmarkNode* BookmarkBarContextMenuController::
     return node_->GetParent();
   }
 }
-
diff --git a/chrome/browser/bookmark_bar_context_menu_controller_test.cc b/chrome/browser/bookmark_bar_context_menu_controller_test.cc
index dd70601..fd3f5cc 100644
--- a/chrome/browser/bookmark_bar_context_menu_controller_test.cc
+++ b/chrome/browser/bookmark_bar_context_menu_controller_test.cc
@@ -96,7 +96,7 @@ TEST_F(BookmarkBarContextMenuControllerTest, DeleteURL) {
   controller.ExecuteCommand(
       BookmarkBarContextMenuController::delete_bookmark_id);
   // Model shouldn't have URL anymore.
-  ASSERT_TRUE(model_->GetNodeByURL(url) == NULL);
+  ASSERT_FALSE(model_->IsBookmarked(url));
 }
 
 // Tests openning from the menu.
diff --git a/chrome/browser/bookmarks/bookmark_drag_data.cc b/chrome/browser/bookmarks/bookmark_drag_data.cc
index 996470c..d2ff684 100644
--- a/chrome/browser/bookmarks/bookmark_drag_data.cc
+++ b/chrome/browser/bookmarks/bookmark_drag_data.cc
@@ -6,6 +6,7 @@
 
 #include "base/pickle.h"
 #include "chrome/browser/bookmarks/bookmark_model.h"
+#include "chrome/browser/profile.h"
 #include "chrome/common/os_exchange_data.h"
 
 static CLIPFORMAT clipboard_format = 0;
@@ -30,15 +31,16 @@ BookmarkDragData::BookmarkDragData(BookmarkNode* node)
     AddChildren(node);
 }
 
-void BookmarkDragData::Write(OSExchangeData* data) const {
+void BookmarkDragData::Write(Profile* profile, OSExchangeData* data) const {
   RegisterFormat();
 
   DCHECK(data);
 
-  if (is_url) {
+  if (is_url)
     data->SetURL(url, title);
-  }
   Pickle data_pickle;
+  data_pickle.WriteWString(profile->GetPath());
+  data_pickle.WriteInt(id_);
   WriteToPickle(&data_pickle);
   data->SetPickledData(clipboard_format, data_pickle);
 }
@@ -48,13 +50,15 @@ bool BookmarkDragData::Read(const OSExchangeData& data) {
 
   is_valid = data.GetURLAndTitle(&url, &title) && url.is_valid();
   is_url = is_valid;
-  profile_id.clear();
+  profile_path_.clear();
 
   if (data.HasFormat(clipboard_format)) {
     Pickle drag_data_pickle;
     if (data.GetPickledData(clipboard_format, &drag_data_pickle)) {
       void* data_iterator = NULL;
-      if (ReadFromPickle(&drag_data_pickle, &data_iterator)) {
+      if (drag_data_pickle.ReadWString(&data_iterator, &profile_path_) &&
+          drag_data_pickle.ReadInt(&data_iterator, &id_) &&
+          ReadFromPickle(&drag_data_pickle, &data_iterator)) {
         is_valid = true;
       }
     }
@@ -62,18 +66,17 @@ bool BookmarkDragData::Read(const OSExchangeData& data) {
   return is_valid;
 }
 
-BookmarkNode* BookmarkDragData::GetNode(BookmarkModel* model) const {
-  DCHECK(!is_url && id_ && is_valid);
-  return model->GetNodeByID(id_);
+BookmarkNode* BookmarkDragData::GetNode(Profile* profile) const {
+  DCHECK(is_valid);
+  return (profile->GetPath() == profile_path_) ?
+      profile->GetBookmarkModel()->GetNodeByID(id_) : NULL;
 }
 
 void BookmarkDragData::WriteToPickle(Pickle* pickle) const {
   pickle->WriteBool(is_url);
-  pickle->WriteWString(profile_id);
   pickle->WriteString(url.spec());
   pickle->WriteWString(title);
   if (!is_url) {
-    pickle->WriteInt(id_);
     pickle->WriteInt(static_cast<int>(children.size()));
     for (std::vector<BookmarkDragData>::const_iterator i = children.begin();
          i != children.end(); ++i) {
@@ -86,17 +89,13 @@ bool BookmarkDragData::ReadFromPickle(Pickle* pickle, void** iterator) {
   std::string url_spec;
   is_valid = false;
   if (!pickle->ReadBool(iterator, &is_url) ||
-      !pickle->ReadWString(iterator, &profile_id) ||
       !pickle->ReadString(iterator, &url_spec) ||
       !pickle->ReadWString(iterator, &title)) {
     return false;
   }
   url = GURL(url_spec);
   if (!is_url) {
-    id_ = 0;
     children.clear();
-    if (!pickle->ReadInt(iterator, &id_))
-      return false;
     int children_count;
     if (!pickle->ReadInt(iterator, &children_count))
       return false;
@@ -115,4 +114,3 @@ void BookmarkDragData::AddChildren(BookmarkNode* node) {
   for (int i = 0, max = node->GetChildCount(); i < max; ++i)
     children.push_back(BookmarkDragData(node->GetChild(i)));
 }
-
diff --git a/chrome/browser/bookmarks/bookmark_drag_data.h b/chrome/browser/bookmarks/bookmark_drag_data.h
index c7e1fed..564393a 100644
--- a/chrome/browser/bookmarks/bookmark_drag_data.h
+++ b/chrome/browser/bookmarks/bookmark_drag_data.h
@@ -14,6 +14,7 @@ class BookmarkModel;
 class BookmarkNode;
 class OSExchangeData;
 class Pickle;
+class Profile;
 
 // BookmarkDragData is used by the bookmark bar to represent a dragged
 // URL or starred group on the clipboard during drag and drop.
@@ -36,23 +37,19 @@ struct BookmarkDragData {
 
   // Writes this BookmarkDragData to data. If BookmarkDragData is a URL,
   // this writes out the URL and URL title clipboard data as well.
-  void Write(OSExchangeData* data) const;
+  void Write(Profile* profile, OSExchangeData* data) const;
 
   // Restores this data from the clipboard, returning true on success.
   bool Read(const OSExchangeData& data);
 
-  // Returns the node represented by this drag data from root. If the
-  // path can not be found, NULL is returned.
-  //
-  // This is only valid for groups.
-  BookmarkNode* BookmarkDragData::GetNode(BookmarkModel* model) const;
+  // Returns the node represented by this DragData. If this DragData was created
+  // from the same profile then the node from the model is returned. If the
+  // node can't be found (may have been deleted), NULL is returned.
+  BookmarkNode* BookmarkDragData::GetNode(Profile* profile) const;
 
   // If true, this entry represents a StarredEntry of type URL.
   bool is_url;
 
-  // ID of the profile we originated from.
-  std::wstring profile_id;
-
   // The URL, only valid if is_url is true.
   GURL url;
 
@@ -74,7 +71,12 @@ struct BookmarkDragData {
   // Adds to children an entry for each child of node.
   void AddChildren(BookmarkNode* node);
 
+  // Path of the profile we originated from.
+  // This is only saved for the root node.
+  std::wstring profile_path_;
+
   // ID (node->id()) of the node this BookmarkDragData was created from.
+  // This is only saved for the root node.
   int id_;
 };
 
diff --git a/chrome/browser/bookmarks/bookmark_drag_data_unittest.cc b/chrome/browser/bookmarks/bookmark_drag_data_unittest.cc
index 76f0f67..a3022f8 100644
--- a/chrome/browser/bookmarks/bookmark_drag_data_unittest.cc
+++ b/chrome/browser/bookmarks/bookmark_drag_data_unittest.cc
@@ -6,6 +6,7 @@
 #include "chrome/browser/bookmarks/bookmark_drag_data.h"
 #include "chrome/browser/bookmarks/bookmark_model.h"
 #include "chrome/common/os_exchange_data.h"
+#include "chrome/test/testing_profile.h"
 #include "googleurl/src/gurl.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -25,20 +26,21 @@ TEST_F(BookmarkDragDataTest, BogusRead) {
 }
 
 TEST_F(BookmarkDragDataTest, URL) {
-  BookmarkModel model(NULL);
-  BookmarkNode* root = model.GetBookmarkBarNode();
+  TestingProfile profile;
+  profile.CreateBookmarkModel(false);
+  profile.SetID(L"id");
+  BookmarkModel* model = profile.GetBookmarkModel();
+  BookmarkNode* root = model->GetBookmarkBarNode();
   GURL url(GURL("http://foo.com"));
-  const std::wstring profile_id(L"blah");
   const std::wstring title(L"blah");
-  BookmarkNode* node = model.AddURL(root, 0, title, url);
+  BookmarkNode* node = model->AddURL(root, 0, title, url);
   BookmarkDragData drag_data(node);
-  drag_data.profile_id = profile_id;
   EXPECT_TRUE(drag_data.url == url);
   EXPECT_EQ(title, drag_data.title);
   EXPECT_TRUE(drag_data.is_url);
 
   scoped_refptr<OSExchangeData> data(new OSExchangeData());
-  drag_data.Write(data.get());
+  drag_data.Write(&profile, data.get());
 
   // Now read the data back in.
   scoped_refptr<OSExchangeData> data2(new OSExchangeData(data.get()));
@@ -48,6 +50,10 @@ TEST_F(BookmarkDragDataTest, URL) {
   EXPECT_EQ(title, read_data.title);
   EXPECT_TRUE(read_data.is_valid);
   EXPECT_TRUE(read_data.is_url);
+  EXPECT_TRUE(read_data.GetNode(&profile) == node);
+
+  TestingProfile profile2(1);
+  EXPECT_TRUE(read_data.GetNode(&profile2) == NULL);
 
   // Writing should also put the URL and title on the clipboard.
   GURL read_url;
@@ -58,50 +64,54 @@ TEST_F(BookmarkDragDataTest, URL) {
 }
 
 TEST_F(BookmarkDragDataTest, Group) {
-  BookmarkModel model(NULL);
-  BookmarkNode* root = model.GetBookmarkBarNode();
-  BookmarkNode* g1 = model.AddGroup(root, 0, L"g1");
-  BookmarkNode* g11 = model.AddGroup(g1, 0, L"g11");
-  BookmarkNode* g12 = model.AddGroup(g1, 0, L"g12");
+  TestingProfile profile;
+  profile.CreateBookmarkModel(false);
+  profile.SetID(L"id");
+  BookmarkModel* model = profile.GetBookmarkModel();
+  BookmarkNode* root = model->GetBookmarkBarNode();
+  BookmarkNode* g1 = model->AddGroup(root, 0, L"g1");
+  BookmarkNode* g11 = model->AddGroup(g1, 0, L"g11");
+  BookmarkNode* g12 = model->AddGroup(g1, 0, L"g12");
 
   BookmarkDragData drag_data(g12);
-  const std::wstring profile_id(L"blah");
-  drag_data.profile_id = profile_id;
   EXPECT_EQ(g12->GetTitle(), drag_data.title);
   EXPECT_FALSE(drag_data.is_url);
 
   scoped_refptr<OSExchangeData> data(new OSExchangeData());
-  drag_data.Write(data.get());
+  drag_data.Write(&profile, data.get());
 
   // Now read the data back in.
   scoped_refptr<OSExchangeData> data2(new OSExchangeData(data.get()));
   BookmarkDragData read_data;
   EXPECT_TRUE(read_data.Read(*data2));
   EXPECT_EQ(g12->GetTitle(), read_data.title);
-  EXPECT_EQ(profile_id, read_data.profile_id);
   EXPECT_TRUE(read_data.is_valid);
   EXPECT_FALSE(read_data.is_url);
 
-  BookmarkNode* r_g12 = read_data.GetNode(&model);
+  BookmarkNode* r_g12 = read_data.GetNode(&profile);
   EXPECT_TRUE(g12 == r_g12);
+
+  TestingProfile profile2(1);
+  EXPECT_TRUE(read_data.GetNode(&profile2) == NULL);
 }
 
 TEST_F(BookmarkDragDataTest, GroupWithChild) {
-  BookmarkModel model(NULL);
-  BookmarkNode* root = model.GetBookmarkBarNode();
-  BookmarkNode* group = model.AddGroup(root, 0, L"g1");
+  TestingProfile profile;
+  profile.SetID(L"id");
+  profile.CreateBookmarkModel(false);
+  BookmarkModel* model = profile.GetBookmarkModel();
+  BookmarkNode* root = model->GetBookmarkBarNode();
+  BookmarkNode* group = model->AddGroup(root, 0, L"g1");
 
   GURL url(GURL("http://foo.com"));
-  const std::wstring profile_id(L"blah");
   const std::wstring title(L"blah2");
 
-  model.AddURL(group, 0, title, url);
+  model->AddURL(group, 0, title, url);
 
   BookmarkDragData drag_data(group);
-  drag_data.profile_id = profile_id;
 
   scoped_refptr<OSExchangeData> data(new OSExchangeData());
-  drag_data.Write(data.get());
+  drag_data.Write(&profile, data.get());
 
   // Now read the data back in.
   scoped_refptr<OSExchangeData> data2(new OSExchangeData(data.get()));
@@ -115,6 +125,6 @@ TEST_F(BookmarkDragDataTest, GroupWithChild) {
   EXPECT_TRUE(url == read_data.children[0].url);
   EXPECT_TRUE(read_data.children[0].is_url);
 
-  BookmarkNode* r_group = read_data.GetNode(&model);
+  BookmarkNode* r_group = read_data.GetNode(&profile);
   EXPECT_TRUE(group == r_group);
 }
diff --git a/chrome/browser/bookmarks/bookmark_model.cc b/chrome/browser/bookmarks/bookmark_model.cc
index af5a43f..0b7a212 100644
--- a/chrome/browser/bookmarks/bookmark_model.cc
+++ b/chrome/browser/bookmarks/bookmark_model.cc
@@ -265,21 +265,47 @@ void BookmarkModel::SetTitle(BookmarkNode* node,
                     BookmarkNodeChanged(this, node));
 }
 
-BookmarkNode* BookmarkModel::GetNodeByURL(const GURL& url) {
+void BookmarkModel::GetNodesByURL(const GURL& url,
+                                  std::vector<BookmarkNode*>* nodes) {
   AutoLock url_lock(url_lock_);
   BookmarkNode tmp_node(this, url);
   NodesOrderedByURLSet::iterator i = nodes_ordered_by_url_set_.find(&tmp_node);
-  return (i != nodes_ordered_by_url_set_.end()) ? *i : NULL;
+  while (i != nodes_ordered_by_url_set_.end() && (*i)->GetURL() == url) {
+    nodes->push_back(*i);
+    ++i;
+  }
+}
+
+BookmarkNode* BookmarkModel::GetMostRecentlyAddedNodeForURL(const GURL& url) {
+  std::vector<BookmarkNode*> nodes;
+  GetNodesByURL(url, &nodes);
+  if (nodes.empty())
+    return NULL;
+
+  std::sort(nodes.begin(), nodes.end(), &MoreRecentlyAdded);
+  return nodes.front();
 }
 
 void BookmarkModel::GetBookmarks(std::vector<GURL>* urls) {
   AutoLock url_lock(url_lock_);
+  const GURL* last_url = NULL;
   for (NodesOrderedByURLSet::iterator i = nodes_ordered_by_url_set_.begin();
        i != nodes_ordered_by_url_set_.end(); ++i) {
-    urls->push_back((*i)->GetURL());
+    const GURL* url = &((*i)->url_);
+    // Only add unique URLs.
+    if (!last_url || *url != *last_url)
+      urls->push_back(*url);
+    last_url = url;
   }
 }
 
+bool BookmarkModel::IsBookmarked(const GURL& url) {
+  AutoLock url_lock(url_lock_);
+  BookmarkNode tmp_node(this, url);
+  return (nodes_ordered_by_url_set_.find(&tmp_node) !=
+          nodes_ordered_by_url_set_.end());
+}
+
 BookmarkNode* BookmarkModel::GetNodeByID(int id) {
   // TODO(sky): TreeNode needs a method that visits all nodes using a predicate.
   return GetNodeByID(&root_, id);
@@ -299,7 +325,7 @@ BookmarkNode* BookmarkModel::AddGroup(
   new_node->SetTitle(title);
   new_node->type_ = history::StarredEntry::USER_GROUP;
 
-  return AddNode(parent, index, new_node);
+  return AddNode(parent, index, new_node, false);
 }
 
 BookmarkNode* BookmarkModel::AddURL(BookmarkNode* parent,
@@ -321,12 +347,7 @@ BookmarkNode* BookmarkModel::AddURLWithCreationTime(
     return NULL;
   }
 
-  BookmarkNode* existing_node = GetNodeByURL(url);
-  if (existing_node) {
-    Move(existing_node, parent, index);
-    SetTitle(existing_node, title);
-    return existing_node;
-  }
+  bool was_bookmarked = IsBookmarked(url);
 
   SetDateGroupModified(parent, creation_time);
 
@@ -338,19 +359,28 @@ BookmarkNode* BookmarkModel::AddURLWithCreationTime(
   AutoLock url_lock(url_lock_);
   nodes_ordered_by_url_set_.insert(new_node);
 
-  return AddNode(parent, index, new_node);
+  return AddNode(parent, index, new_node, was_bookmarked);
 }
 
 void BookmarkModel::SetURLStarred(const GURL& url,
                                   const std::wstring& title,
                                   bool is_starred) {
-  BookmarkNode* node = GetNodeByURL(url);
-  if (is_starred && !node) {
-    // Add the url.
+  std::vector<BookmarkNode*> bookmarks;
+  GetNodesByURL(url, &bookmarks);
+  bool bookmarks_exist = !bookmarks.empty();
+  if (is_starred == bookmarks_exist)
+    return;  // Nothing to do, state already matches.
+
+  if (is_starred) {
+    // Create a bookmark.
     BookmarkNode* parent = GetParentForNewNodes();
     AddURL(parent, parent->GetChildCount(), title, url);
-  } else if (!is_starred && node) {
-    Remove(node->GetParent(), node->GetParent()->IndexOfChild(node));
+  } else {
+    // Remove all the bookmarks.
+    for (size_t i = 0; i < bookmarks.size(); ++i) {
+      BookmarkNode* node = bookmarks[i];
+      Remove(node->GetParent(), node->GetParent()->IndexOfChild(node));
+    }
   }
 }
 
@@ -377,6 +407,10 @@ void BookmarkModel::RemoveNode(BookmarkNode* node,
     // such, this doesn't explicitly grab the lock.
     NodesOrderedByURLSet::iterator i = nodes_ordered_by_url_set_.find(node);
     DCHECK(i != nodes_ordered_by_url_set_.end());
+    // i points to the first node with the URL, advance until we find the
+    // node we're removing.
+    while (*i != node)
+      ++i;
     nodes_ordered_by_url_set_.erase(i);
     removed_urls->insert(node->GetURL());
   }
@@ -480,6 +514,16 @@ void BookmarkModel::RemoveAndDeleteNode(BookmarkNode* delete_me) {
   {
     AutoLock url_lock(url_lock_);
     RemoveNode(node.get(), &details.changed_urls);
+
+    // RemoveNode adds an entry to changed_urls for each node of type URL. As we
+    // allow duplicates we need to remove any entries that are still bookmarked.
+    for (std::set<GURL>::iterator i = details.changed_urls.begin();
+         i != details.changed_urls.end(); ){
+      if (IsBookmarked(*i))
+        i = details.changed_urls.erase(i);
+      else
+        ++i;
+    }
   }
 
   if (store_.get())
@@ -488,6 +532,11 @@ void BookmarkModel::RemoveAndDeleteNode(BookmarkNode* delete_me) {
   FOR_EACH_OBSERVER(BookmarkModelObserver, observers_,
                     BookmarkNodeRemoved(this, parent, index));
 
+  if (details.changed_urls.empty()) {
+    // No point in sending out notification if the starred state didn't change.
+    return;
+  }
+
   if (profile_) {
     HistoryService* history =
         profile_->GetHistoryService(Profile::EXPLICIT_ACCESS);
@@ -502,7 +551,8 @@ void BookmarkModel::RemoveAndDeleteNode(BookmarkNode* delete_me) {
 
 BookmarkNode* BookmarkModel::AddNode(BookmarkNode* parent,
                                      int index,
-                                     BookmarkNode* node) {
+                                     BookmarkNode* node,
+                                     bool was_bookmarked) {
   parent->Add(index, node);
 
   if (store_.get())
@@ -511,7 +561,7 @@ BookmarkNode* BookmarkModel::AddNode(BookmarkNode* parent,
   FOR_EACH_OBSERVER(BookmarkModelObserver, observers_,
                     BookmarkNodeAdded(this, parent, index));
 
-  if (node->GetType() == history::StarredEntry::URL) {
+  if (node->GetType() == history::StarredEntry::URL && !was_bookmarked) {
     history::URLsStarredDetails details(true);
     details.changed_urls.insert(node->GetURL());
     NotificationService::current()->Notify(NOTIFY_URLS_STARRED,
@@ -662,9 +712,11 @@ void BookmarkModel::Observe(NotificationType type,
       Details<history::FavIconChangeDetails> favicon_details(details);
       for (std::set<GURL>::const_iterator i = favicon_details->urls.begin();
            i != favicon_details->urls.end(); ++i) {
-        BookmarkNode* node = GetNodeByURL(*i);
-        if (node) {
+        std::vector<BookmarkNode*> nodes;
+        GetNodesByURL(*i, &nodes);
+        for (size_t i = 0; i < nodes.size(); ++i) {
           // Got an updated favicon, for a URL, do a new request.
+          BookmarkNode* node = nodes[i];
           node->InvalidateFavicon();
           CancelPendingFavIconLoadRequests(node);
           FOR_EACH_OBSERVER(BookmarkModelObserver, observers_,
diff --git a/chrome/browser/bookmarks/bookmark_model.h b/chrome/browser/bookmarks/bookmark_model.h
index e20349d..a9f6308 100644
--- a/chrome/browser/bookmarks/bookmark_model.h
+++ b/chrome/browser/bookmarks/bookmark_model.h
@@ -18,6 +18,7 @@
 #include "googleurl/src/gurl.h"
 #include "skia/include/SkBitmap.h"
 
+class BookmarkEditorView;
 class BookmarkModel;
 class BookmarkCodec;
 class Profile;
@@ -32,10 +33,14 @@ class StarredURLDatabase;
 // star id and type. BookmarkNodes are returned from a BookmarkModel.
 //
 class BookmarkNode : public ChromeViews::TreeNode<BookmarkNode> {
+  friend class BookmarkEditorView;
   friend class BookmarkModel;
   friend class BookmarkCodec;
   friend class history::StarredURLDatabase;
+  FRIEND_TEST(BookmarkEditorViewTest, ChangeParentAndURL);
+  FRIEND_TEST(BookmarkEditorViewTest, EditURLKeepsPosition);
   FRIEND_TEST(BookmarkModelTest, MostRecentlyAddedEntries);
+  FRIEND_TEST(BookmarkModelTest, GetMostRecentlyAddedNodeForURL);
 
  public:
   BookmarkNode(BookmarkModel* model, const GURL& url);
@@ -114,7 +119,7 @@ class BookmarkNode : public ChromeViews::TreeNode<BookmarkNode> {
   // Time last modified. Only used for groups.
   Time date_group_modified_;
 
-  DISALLOW_EVIL_CONSTRUCTORS(BookmarkNode);
+  DISALLOW_COPY_AND_ASSIGN(BookmarkNode);
 };
 
 // BookmarkModelObserver ------------------------------------------------------
@@ -161,15 +166,11 @@ class BookmarkModelObserver {
 // and groups. Two graphs are provided for the two entry points: those on
 // the bookmark bar, and those in the other folder.
 //
-// The methods of BookmarkModel update the internal structure immediately
-// and update the backend in the background.
-//
 // An observer may be attached to observer relevant events.
 //
 // You should NOT directly create a BookmarkModel, instead go through the
 // Profile.
 
-// TODO(sky): rename to BookmarkModel.
 class BookmarkModel : public NotificationObserver, public BookmarkService {
   friend class BookmarkNode;
   friend class BookmarkModelTest;
@@ -241,18 +242,19 @@ class BookmarkModel : public NotificationObserver, public BookmarkService {
   // Returns true if the model finished loading.
   bool IsLoaded() { return loaded_; }
 
-  // Returns the node with the specified URL, or NULL if there is no node with
-  // the specified URL. This method is thread safe.
-  BookmarkNode* GetNodeByURL(const GURL& url);
+  // Returns the set of nodes with the specified URL.
+  void GetNodesByURL(const GURL& url, std::vector<BookmarkNode*>* nodes);
+
+  // Returns the most recently added node for the url. Returns NULL if url is
+  // not bookmarked.
+  BookmarkNode* GetMostRecentlyAddedNodeForURL(const GURL& url);
 
   // Returns all the bookmarked urls. This method is thread safe.
   virtual void GetBookmarks(std::vector<GURL>* urls);
 
   // Returns true if there is a bookmark for the specified URL. This method is
   // thread safe. See BookmarkService for more details on this.
-  virtual bool IsBookmarked(const GURL& url) {
-    return GetNodeByURL(url) != NULL;
-  }
+  virtual bool IsBookmarked(const GURL& url);
 
   // Blocks until loaded; this is NOT invoked on the main thread. See
   // BookmarkService for more details on this.
@@ -267,8 +269,7 @@ class BookmarkModel : public NotificationObserver, public BookmarkService {
                          int index,
                          const std::wstring& title);
 
-  // Adds a url at the specified position. If there is already a node with the
-  // specified URL, it is moved to the new position.
+  // Adds a url at the specified position.
   BookmarkNode* AddURL(BookmarkNode* parent,
                        int index,
                        const std::wstring& title,
@@ -281,9 +282,9 @@ class BookmarkModel : public NotificationObserver, public BookmarkService {
                                        const GURL& url,
                                        const Time& creation_time);
 
-  // This is the convenience that makes sure the url is starred or not
-  // starred. If the URL is not currently starred, it is added to the
-  // most recent parent.
+  // This is the convenience that makes sure the url is starred or not starred.
+  // If is_starred is false, all bookmarks for URL are removed. If is_starred is
+  // true and there are no bookmarks for url, a bookmark is created.
   void SetURLStarred(const GURL& url,
                      const std::wstring& title,
                      bool is_starred);
@@ -305,7 +306,7 @@ class BookmarkModel : public NotificationObserver, public BookmarkService {
   // Overriden to notify the observer the favicon has been loaded.
   void FavIconLoaded(BookmarkNode* node);
 
-  // Removes the node from internal maps and recurces through all children. If
+  // Removes the node from internal maps and recurses through all children. If
   // the node is a url, its url is added to removed_urls.
   //
   // This does NOT delete the node.
@@ -339,10 +340,13 @@ class BookmarkModel : public NotificationObserver, public BookmarkService {
   // type specifies how the node should be removed.
   void RemoveAndDeleteNode(BookmarkNode* delete_me);
 
-  // Adds the node at the specified position, and sends notification.
+  // Adds the node at the specified position and sends notification. If
+  // was_bookmarked is true, it indicates a bookmark already existed for the
+  // URL.
   BookmarkNode* AddNode(BookmarkNode* parent,
                         int index,
-                        BookmarkNode* node);
+                        BookmarkNode* node,
+                        bool was_bookmarked);
 
   // Implementation of GetNodeByID.
   BookmarkNode* GetNodeByID(BookmarkNode* node, int id);
@@ -409,7 +413,7 @@ class BookmarkModel : public NotificationObserver, public BookmarkService {
   // urls.
   // WARNING: nodes_ordered_by_url_set_ is accessed on multiple threads. As
   // such, be sure and wrap all usage of it around url_lock_.
-  typedef std::set<BookmarkNode*,NodeURLComparator> NodesOrderedByURLSet;
+  typedef std::multiset<BookmarkNode*,NodeURLComparator> NodesOrderedByURLSet;
   NodesOrderedByURLSet nodes_ordered_by_url_set_;
   Lock url_lock_;
 
diff --git a/chrome/browser/bookmarks/bookmark_model_unittest.cc b/chrome/browser/bookmarks/bookmark_model_unittest.cc
index 04c08d3..a6cab2a 100644
--- a/chrome/browser/bookmarks/bookmark_model_unittest.cc
+++ b/chrome/browser/bookmarks/bookmark_model_unittest.cc
@@ -7,6 +7,7 @@
 #include "chrome/browser/bookmarks/bookmark_codec.h"
 #include "chrome/common/chrome_constants.h"
 #include "chrome/common/chrome_paths.h"
+#include "chrome/common/notification_registrar.h"
 #include "chrome/test/testing_profile.h"
 #include "chrome/views/tree_node_model.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -169,7 +170,7 @@ TEST_F(BookmarkModelTest, AddURL) {
   ASSERT_EQ(title, new_node->GetTitle());
   ASSERT_TRUE(url == new_node->GetURL());
   ASSERT_EQ(history::StarredEntry::URL, new_node->GetType());
-  ASSERT_TRUE(new_node == model.GetNodeByURL(url));
+  ASSERT_TRUE(new_node == model.GetMostRecentlyAddedNodeForURL(url));
 
   EXPECT_TRUE(new_node->id() != root->id() &&
               new_node->id() != model.other_node()->id());
@@ -210,7 +211,7 @@ TEST_F(BookmarkModelTest, RemoveURL) {
   observer_details.AssertEquals(root, NULL, 0, -1);
 
   // Make sure there is no mapping for the URL.
-  ASSERT_TRUE(model.GetNodeByURL(url) == NULL);
+  ASSERT_TRUE(model.GetMostRecentlyAddedNodeForURL(url) == NULL);
 }
 
 TEST_F(BookmarkModelTest, RemoveGroup) {
@@ -233,7 +234,7 @@ TEST_F(BookmarkModelTest, RemoveGroup) {
   observer_details.AssertEquals(root, NULL, 0, -1);
 
   // Make sure there is no mapping for the URL.
-  ASSERT_TRUE(model.GetNodeByURL(url) == NULL);
+  ASSERT_TRUE(model.GetMostRecentlyAddedNodeForURL(url) == NULL);
 }
 
 TEST_F(BookmarkModelTest, SetTitle) {
@@ -274,7 +275,7 @@ TEST_F(BookmarkModelTest, Move) {
   model.Remove(root, 0);
   AssertObserverCount(0, 0, 1, 0);
   observer_details.AssertEquals(root, NULL, 0, -1);
-  EXPECT_TRUE(model.GetNodeByURL(url) == NULL);
+  EXPECT_TRUE(model.GetMostRecentlyAddedNodeForURL(url) == NULL);
   EXPECT_EQ(0, root->GetChildCount());
 }
 
@@ -369,6 +370,110 @@ TEST_F(BookmarkModelTest, GetBookmarksMatchingText) {
   EXPECT_EQ(n2, results[0].node);
 }
 
+// Makes sure GetMostRecentlyAddedNodeForURL stays in sync.
+TEST_F(BookmarkModelTest, GetMostRecentlyAddedNodeForURL) {
+  // Add a couple of nodes such that the following holds for the time of the
+  // nodes: n1 > n2
+  Time base_time = Time::Now();
+  const GURL url("http://foo.com/0");
+  BookmarkNode* n1 = model.AddURL(model.GetBookmarkBarNode(), 0, L"blah", url);
+  BookmarkNode* n2 = model.AddURL(model.GetBookmarkBarNode(), 1, L"blah", url);
+  n1->date_added_ = base_time + TimeDelta::FromDays(4);
+  n2->date_added_ = base_time + TimeDelta::FromDays(3);
+
+  // Make sure order is honored.
+  ASSERT_EQ(n1, model.GetMostRecentlyAddedNodeForURL(url));
+
+  // swap 1 and 2, then check again.
+  std::swap(n1->date_added_, n2->date_added_);
+  ASSERT_EQ(n2, model.GetMostRecentlyAddedNodeForURL(url));
+}
+
+// Makes sure GetBookmarks removes duplicates.
+TEST_F(BookmarkModelTest, GetBookmarksWithDups) {
+  const GURL url("http://foo.com/0");
+  model.AddURL(model.GetBookmarkBarNode(), 0, L"blah", url);
+  model.AddURL(model.GetBookmarkBarNode(), 1, L"blah", url);
+
+  std::vector<GURL> urls;
+  model.GetBookmarks(&urls);
+  EXPECT_EQ(1, urls.size());
+  ASSERT_TRUE(urls[0] == url);
+}
+
+namespace {
+
+// NotificationObserver implementation used in verifying we've received the
+// NOTIFY_URLS_STARRED method correctly.
+class StarredListener : public NotificationObserver {
+ public:
+  StarredListener() : notification_count_(0), details_(false) {
+    registrar_.Add(this, NOTIFY_URLS_STARRED, Source<Profile>(NULL));
+  }
+
+  virtual void Observe(NotificationType type,
+                       const NotificationSource& source,
+                       const NotificationDetails& details) {
+    if (type == NOTIFY_URLS_STARRED) {
+      notification_count_++;
+      details_ = *(Details<history::URLsStarredDetails>(details).ptr());
+    }
+  }
+
+  // Number of times NOTIFY_URLS_STARRED has been observed.
+  int notification_count_;
+
+  // Details from the last NOTIFY_URLS_STARRED.
+  history::URLsStarredDetails details_;
+
+ private:
+  NotificationRegistrar registrar_;
+
+  DISALLOW_COPY_AND_ASSIGN(StarredListener);
+};
+
+}  // namespace
+
+// Makes sure NOTIFY_URLS_STARRED is sent correctly.
+TEST_F(BookmarkModelTest, NotifyURLsStarred) {
+  StarredListener listener;
+  const GURL url("http://foo.com/0");
+  BookmarkNode* n1 = model.AddURL(model.GetBookmarkBarNode(), 0, L"blah", url);
+
+  // Starred notification should be sent.
+  EXPECT_EQ(1, listener.notification_count_);
+  ASSERT_TRUE(listener.details_.starred);
+  ASSERT_EQ(1, listener.details_.changed_urls.size());
+  EXPECT_TRUE(url == *(listener.details_.changed_urls.begin()));
+  listener.notification_count_ = 0;
+  listener.details_.changed_urls.clear();
+
+  // Add another bookmark for the same URL. This should not send any
+  // notification.
+  BookmarkNode* n2 = model.AddURL(model.GetBookmarkBarNode(), 1, L"blah", url);
+
+  EXPECT_EQ(0, listener.notification_count_);
+
+  // Remove n2.
+  model.Remove(n2->GetParent(), 1);
+  n2 = NULL;
+
+  // Shouldn't have received any notification as n1 still exists with the same
+  // URL.
+  EXPECT_EQ(0, listener.notification_count_);
+
+  EXPECT_TRUE(model.GetMostRecentlyAddedNodeForURL(url) == n1);
+
+  // Remove n1.
+  model.Remove(n1->GetParent(), 0);
+
+  // Now we should get the notification.
+  EXPECT_EQ(1, listener.notification_count_);
+  ASSERT_FALSE(listener.details_.starred);
+  ASSERT_EQ(1, listener.details_.changed_urls.size());
+  EXPECT_TRUE(url == *(listener.details_.changed_urls.begin()));
+}
+
 namespace {
 
 // See comment in PopulateNodeFromString.
@@ -650,7 +755,7 @@ class BookmarkModelTestWithProfile2 : public BookmarkModelTestWithProfile {
     ASSERT_TRUE(child->GetURL() ==
                 GURL("http://www.google.com/intl/en/about.html"));
 
-    ASSERT_TRUE(bb_model_->GetNodeByURL(GURL("http://www.google.com")) != NULL);
+    ASSERT_TRUE(bb_model_->IsBookmarked(GURL("http://www.google.com")));
   }
 };
 
diff --git a/chrome/browser/bookmarks/bookmark_service.h b/chrome/browser/bookmarks/bookmark_service.h
index a758382..30387a3 100644
--- a/chrome/browser/bookmarks/bookmark_service.h
+++ b/chrome/browser/bookmarks/bookmark_service.h
@@ -48,7 +48,9 @@ class BookmarkService {
   // If not on the main thread you *must* invoke BlockTillLoaded first.
   virtual bool IsBookmarked(const GURL& url) = 0;
 
-  // Returns, by reference in urls, the set of bookmarked urls.
+  // Returns, by reference in urls, the set of bookmarked urls. This returns
+  // the unique set of URLs. For example, if two bookmarks reference the same
+  // URL only one entry is added.
   //
   // If not on the main thread you *must* invoke BlockTillLoaded first.
   virtual void GetBookmarks(std::vector<GURL>* urls) = 0;
diff --git a/chrome/browser/browser_commands.cc b/chrome/browser/browser_commands.cc
index d2cb3fe..cf8630b 100644
--- a/chrome/browser/browser_commands.cc
+++ b/chrome/browser/browser_commands.cc
@@ -823,10 +823,10 @@ void Browser::StarCurrentTabContents() {
 
   if (window_->GetStarButton()) {
     if (!window_->GetStarButton()->is_bubble_showing()) {
-      const bool newly_bookmarked = (model->GetNodeByURL(url) == NULL);
+      const bool newly_bookmarked = !model->IsBookmarked(url);
       if (newly_bookmarked) {
         model->SetURLStarred(url, entry->title(), true);
-        if (!model->GetNodeByURL(url)) {
+        if (!model->IsBookmarked(url)) {
           // Starring failed. This shouldn't happen.
           NOTREACHED();
           return;
@@ -834,7 +834,7 @@ void Browser::StarCurrentTabContents() {
       }
       window_->GetStarButton()->ShowStarBubble(url, newly_bookmarked);
     }
-  } else if (model->GetNodeByURL(url)) {
+  } else if (model->IsBookmarked(url)) {
     // If we can't find the star button and the user wanted to unstar it,
     // go ahead and unstar it without showing the bubble.
     model->SetURLStarred(url, std::wstring(), false);
diff --git a/chrome/browser/importer/importer.cc b/chrome/browser/importer/importer.cc
index 45d317f..4618f50 100644
--- a/chrome/browser/importer/importer.cc
+++ b/chrome/browser/importer/importer.cc
@@ -83,8 +83,8 @@ void ProfileWriter::AddBookmarkEntry(
   std::set<BookmarkNode*> groups_added_to;
   for (std::vector<BookmarkEntry>::const_iterator it = bookmark.begin();
        it != bookmark.end(); ++it) {
-    // Don't insert this url if it exists in model or url is not valid.
-    if (model->GetNodeByURL(it->url) != NULL || !it->url.is_valid())
+    // Don't insert this url if it isn't valid.
+    if (!it->url.is_valid())
       continue;
 
     // Set up groups in BookmarkModel in such a way that path[i] is
diff --git a/chrome/browser/views/bookmark_bar_view.cc b/chrome/browser/views/bookmark_bar_view.cc
index b7475ee..8d2531a 100644
--- a/chrome/browser/views/bookmark_bar_view.cc
+++ b/chrome/browser/views/bookmark_bar_view.cc
@@ -145,6 +145,21 @@ static const SkColor kInstructionsColor = SkColorSetRGB(128, 128, 142);
 
 namespace {
 
+// Calculates the drop operation given the event and supported set of
+// operations.
+int PreferredDropOperation(const DropTargetEvent& event, int operation) {
+  int common_ops = (event.GetSourceOperations() & operation);
+  if (!common_ops)
+    return 0;
+  if (DragDropTypes::DRAG_COPY & common_ops)
+    return DragDropTypes::DRAG_COPY;
+  if (DragDropTypes::DRAG_LINK & common_ops)
+    return DragDropTypes::DRAG_LINK;
+  if (DragDropTypes::DRAG_MOVE & common_ops)
+    return DragDropTypes::DRAG_MOVE;
+  return DragDropTypes::DRAG_NONE;
+}
+
 // Returns the tooltip text for the specified url and title. The returned
 // text is clipped to fit within the bounds of the monitor.
 //
@@ -420,22 +435,15 @@ class MenuRunner : public ChromeViews::MenuDelegate,
     if (drop_data_.is_url)
       return true;
 
-    if (drop_data_.profile_id != view_->GetProfile()->GetID()) {
-      // Always accept drags of bookmark groups from other profiles.
+    BookmarkNode* drag_node = drop_data_.GetNode(view_->GetProfile());
+    if (!drag_node) {
+      // Dragging a group from another profile, always accept.
       return true;
     }
     // Drag originated from same profile and is not a URL. Only accept it if
     // the dragged node is not a parent of the node menu represents.
     BookmarkNode* drop_node = menu_id_to_node_map_[menu->GetCommand()];
     DCHECK(drop_node);
-    BookmarkNode* drag_node =
-        drop_data_.GetNode(view_->GetProfile()->GetBookmarkModel());
-    if (!drag_node) {
-      // Hmmm, can't find the dragged node. This is generally an error
-      // condition and we won't try and do anything fancy.
-      NOTREACHED();
-      return false;
-    }
     BookmarkNode* node = drop_node;
     while (drop_node && drop_node != drag_node)
       drop_node = drop_node->GetParent();
@@ -456,7 +464,7 @@ class MenuRunner : public ChromeViews::MenuDelegate,
       index_to_drop_at = node->GetChildCount();
     }
     DCHECK(drop_parent);
-    return view_->CalculateDropOperation(drop_data_, drop_parent,
+    return view_->CalculateDropOperation(event, drop_data_, drop_parent,
                                          index_to_drop_at);
   }
 
@@ -929,6 +937,7 @@ int BookmarkBarView::OnDragUpdated(const DropTargetEvent& event) {
 
   if (drop_info_->valid &&
       (drop_info_->x == event.x() && drop_info_->y == event.y())) {
+    // The location of the mouse didn't change, return the last operation.
     return drop_info_->drag_operation;
   }
 
@@ -948,6 +957,8 @@ int BookmarkBarView::OnDragUpdated(const DropTargetEvent& event) {
       drop_info_->drop_on == drop_on &&
       drop_info_->is_over_overflow == is_over_overflow &&
       drop_info_->is_over_other == is_over_other) {
+    // The position we're going to drop didn't change, return the last drag
+    // operation we calculated.
     return drop_info_->drag_operation;
   }
 
@@ -1290,8 +1301,7 @@ void BookmarkBarView::WriteDragData(BookmarkNode* node,
                                     OSExchangeData* data) {
   DCHECK(node && data);
   BookmarkDragData drag_data(node);
-  drag_data.profile_id = GetProfile()->GetID();
-  drag_data.Write(data);
+  drag_data.Write(profile_, data);
 }
 
 int BookmarkBarView::GetDragOperations(View* sender, int x, int y) {
@@ -1591,7 +1601,10 @@ int BookmarkBarView::CalculateDropOperation(const DropTargetEvent& event,
   } else if (!GetBookmarkButtonCount()) {
     // No bookmarks, accept the drop.
     *index = 0;
-    return DragDropTypes::DRAG_COPY;
+    int ops = data.GetNode(profile_)
+        ? DragDropTypes::DRAG_MOVE
+        : DragDropTypes::DRAG_COPY | DragDropTypes::DRAG_LINK;
+    return PreferredDropOperation(event, ops);
   }
 
   for (int i = 0; i < GetBookmarkButtonCount() &&
@@ -1650,49 +1663,33 @@ int BookmarkBarView::CalculateDropOperation(const DropTargetEvent& event,
         *is_over_other ? model_->other_node() :
                          model_->GetBookmarkBarNode()->GetChild(*index);
     int operation =
-        CalculateDropOperation(data, parent, parent->GetChildCount());
-    if (!operation && !data.is_url &&
-        data.profile_id == GetProfile()->GetID()) {
-      if (data.GetNode(model_) == parent) {
-        // Don't open a menu if the node being dragged is the the menu to
-        // open.
-        *drop_on = false;
-      }
+        CalculateDropOperation(event, data, parent, parent->GetChildCount());
+    if (!operation && !data.is_url && data.GetNode(profile_) == parent) {
+      // Don't open a menu if the node being dragged is the the menu to
+      // open.
+      *drop_on = false;
     }
     return operation;
   } else {
-    return CalculateDropOperation(data, model_->GetBookmarkBarNode(), *index);
+    return CalculateDropOperation(event, data, model_->GetBookmarkBarNode(),
+                                  *index);
   }
 }
 
-int BookmarkBarView::CalculateDropOperation(const BookmarkDragData& data,
+int BookmarkBarView::CalculateDropOperation(const DropTargetEvent& event,
+                                            const BookmarkDragData& data,
                                             BookmarkNode* parent,
                                             int index) {
   if (!CanDropAt(data, parent, index))
     return DragDropTypes::DRAG_NONE;
 
-  if (data.is_url) {
-    // User is dragging a URL.
-    BookmarkNode* node = model_->GetNodeByURL(data.url);
-    if (!node) {
-      // We don't have a node with this url.
-      return DragDropTypes::DRAG_COPY;
-    }
-    // Technically we're going to move, but most sources export as copy so that
-    // if we don't accept copy we won't accept the drop.
-    return DragDropTypes::DRAG_MOVE | DragDropTypes::DRAG_COPY;
-  } else if (data.profile_id == GetProfile()->GetID()) {
-    // Dropping a group from the same profile results in a move.
-    BookmarkNode* node = data.GetNode(model_);
-    if (!node) {
-      // Generally shouldn't get here, we originated the drag but couldn't
-      // find the node.
-      return DragDropTypes::DRAG_NONE;
-    }
+  if (data.GetNode(profile_)) {
+    // User is dragging from this profile: move.
     return DragDropTypes::DRAG_MOVE;
   } else {
-    // Dropping a group from different profile. Always accept.
-    return DragDropTypes::DRAG_COPY;
+    // User is dragging from another app, copy.
+    return PreferredDropOperation(
+        event, DragDropTypes::DRAG_COPY | DragDropTypes::DRAG_LINK);
   }
 }
 
@@ -1700,29 +1697,19 @@ bool BookmarkBarView::CanDropAt(const BookmarkDragData& data,
                                 BookmarkNode* parent,
                                 int index) {
   DCHECK(data.is_valid);
-  if (data.is_url) {
-    BookmarkNode* existing_node = model_->GetNodeByURL(data.url);
-    if (existing_node && existing_node->GetParent() == parent) {
-      const int existing_index = parent->IndexOfChild(existing_node);
+  BookmarkNode* dragged_node = data.GetNode(profile_);
+  if (dragged_node) {
+    if (dragged_node->GetParent() == parent) {
+      const int existing_index = parent->IndexOfChild(dragged_node);
       if (index == existing_index || existing_index + 1 == index)
         return false;
     }
-    return true;
-  } else if (data.profile_id == profile_->GetID()) {
-    BookmarkNode* existing_node = data.GetNode(model_);
-    if (existing_node) {
-      if (existing_node->GetParent() == parent) {
-        const int existing_index = parent->IndexOfChild(existing_node);
-        if (index == existing_index || existing_index + 1 == index)
-          return false;
-      }
-      // Allow the drop only if the node we're going to drop on isn't a
-      // descendant of the dragged node.
-      BookmarkNode* test_node = parent;
-      while (test_node && test_node != existing_node)
-        test_node = test_node->GetParent();
-      return (test_node == NULL);
-    }
+    // Allow the drop only if the node we're going to drop on isn't a
+    // descendant of the dragged node.
+    BookmarkNode* test_node = parent;
+    while (test_node && test_node != dragged_node)
+      test_node = test_node->GetParent();
+    return (test_node == NULL);
   }  // else case clones, always allow.
   return true;
 }
@@ -1731,31 +1718,22 @@ bool BookmarkBarView::CanDropAt(const BookmarkDragData& data,
 int BookmarkBarView::PerformDropImpl(const BookmarkDragData& data,
                                      BookmarkNode* parent_node,
                                      int index) {
-  if (data.is_url) {
-    // User is dragging a URL.
-    BookmarkNode* node = model_->GetNodeByURL(data.url);
-    if (!node) {
-      std::wstring title = data.title;
-      if (title.empty()) {
-        // No title, use the host.
-        title = UTF8ToWide(data.url.host());
-        if (title.empty())
-          title = l10n_util::GetString(IDS_BOOMARK_BAR_UNKNOWN_DRAG_TITLE);
-      }
-      model_->AddURL(parent_node, index, title, data.url);
-      return DragDropTypes::DRAG_COPY;
-    }
-    model_->Move(node, parent_node, index);
+  BookmarkNode* dragged_node = data.GetNode(profile_);
+  if (dragged_node) {
+    // Drag from same profile, do a move.
+    model_->Move(dragged_node, parent_node, index);
     return DragDropTypes::DRAG_MOVE;
-  } else if (data.profile_id == GetProfile()->GetID()) {
-    BookmarkNode* node = data.GetNode(model_);
-    if (!node) {
-      // Generally shouldn't get here, we originated the drag but couldn't
-      // find the node. Do nothing.
-      return DragDropTypes::DRAG_COPY;
+  } else if (data.is_url) {
+    // New URL, add it at the specified location.
+    std::wstring title = data.title;
+    if (title.empty()) {
+      // No title, use the host.
+      title = UTF8ToWide(data.url.host());
+      if (title.empty())
+        title = l10n_util::GetString(IDS_BOOMARK_BAR_UNKNOWN_DRAG_TITLE);
     }
-    model_->Move(node, parent_node, index);
-    return DragDropTypes::DRAG_MOVE;
+    model_->AddURL(parent_node, index, title, data.url);
+    return DragDropTypes::DRAG_COPY | DragDropTypes::DRAG_LINK;
   } else {
     // Dropping a group from different profile. Always accept.
     CloneDragData(data, parent_node, index);
@@ -1768,12 +1746,7 @@ void BookmarkBarView::CloneDragData(const BookmarkDragData& data,
                                     int index_to_add_at) {
   DCHECK(data.is_valid && model_);
   if (data.is_url) {
-    BookmarkNode* node = model_->GetNodeByURL(data.url);
-    if (node) {
-      model_->Move(node, parent, index_to_add_at);
-    } else {
-      model_->AddURL(parent, index_to_add_at, data.title, data.url);
-    }
+    model_->AddURL(parent, index_to_add_at, data.title, data.url);
   } else {
     BookmarkNode* new_folder = model_->AddGroup(parent, index_to_add_at,
                                                 data.title);
@@ -1800,7 +1773,7 @@ void BookmarkBarView::StartThrobbing() {
   if (!GetViewContainer())
     return;  // We're not showing, don't do anything.
 
-  BookmarkNode* node = model_->GetNodeByURL(bubble_url_);
+  BookmarkNode* node = model_->GetMostRecentlyAddedNodeForURL(bubble_url_);
   if (!node)
     return;  // Generally shouldn't happen.
 
diff --git a/chrome/browser/views/bookmark_bar_view.h b/chrome/browser/views/bookmark_bar_view.h
index 552dbaf..605322b 100644
--- a/chrome/browser/views/bookmark_bar_view.h
+++ b/chrome/browser/views/bookmark_bar_view.h
@@ -346,7 +346,8 @@ class BookmarkBarView : public ChromeViews::View,
 
   // Invokes CanDropAt to determine if this is a valid location for the data,
   // then returns the appropriate drag operation based on the data.
-  int CalculateDropOperation(const BookmarkDragData& data,
+  int CalculateDropOperation(const ChromeViews::DropTargetEvent& event,
+                             const BookmarkDragData& data,
                              BookmarkNode* parent,
                              int index);
 
diff --git a/chrome/browser/views/bookmark_bubble_view.cc b/chrome/browser/views/bookmark_bubble_view.cc
index 186defe..b70416c 100644
--- a/chrome/browser/views/bookmark_bubble_view.cc
+++ b/chrome/browser/views/bookmark_bubble_view.cc
@@ -163,8 +163,9 @@ BookmarkBubbleView::BookmarkBubbleView(InfoBubbleDelegate* delegate,
       profile_(profile),
       url_(url),
       newly_bookmarked_(newly_bookmarked),
-      parent_model_(profile_->GetBookmarkModel(),
-                    profile_->GetBookmarkModel()->GetNodeByURL(url)) {
+      parent_model_(
+          profile_->GetBookmarkModel(),
+          profile_->GetBookmarkModel()->GetMostRecentlyAddedNodeForURL(url)) {
   Init();
 }
 
@@ -255,7 +256,7 @@ void BookmarkBubbleView::Init() {
 
 std::wstring BookmarkBubbleView::GetTitle() {
   BookmarkModel* bookmark_model= profile_->GetBookmarkModel();
-  BookmarkNode* node = bookmark_model->GetNodeByURL(url_);
+  BookmarkNode* node = bookmark_model->GetMostRecentlyAddedNodeForURL(url_);
   if (node)
     return node->GetTitle();
   else
@@ -289,7 +290,7 @@ void BookmarkBubbleView::ItemChanged(ComboBox* combo_box,
     return;
   }
   BookmarkModel* model = profile_->GetBookmarkModel();
-  BookmarkNode* node = model->GetNodeByURL(url_);
+  BookmarkNode* node = model->GetMostRecentlyAddedNodeForURL(url_);
   if (node) {
     BookmarkNode* new_parent = parent_model_.GetNodeAt(new_index);
     if (new_parent != node->GetParent()) {
@@ -331,6 +332,9 @@ void BookmarkBubbleView::RemoveBookmark() {
 }
 
 void BookmarkBubbleView::ShowEditor() {
+  BookmarkNode* node =
+      profile_->GetBookmarkModel()->GetMostRecentlyAddedNodeForURL(url_);
+
   // The user may have edited the title, commit it now.
   SetNodeTitleFromTextField();
 
@@ -351,12 +355,13 @@ void BookmarkBubbleView::ShowEditor() {
   // the delete and all that.
   Close();
 
-  BookmarkEditorView::Show(parent, profile_, url_, title_);
+  if (node)
+    BookmarkEditorView::Show(parent, profile_, NULL, node);
 }
 
 void BookmarkBubbleView::SetNodeTitleFromTextField() {
   BookmarkModel* model = profile_->GetBookmarkModel();
-  BookmarkNode* node = model->GetNodeByURL(url_);
+  BookmarkNode* node = model->GetMostRecentlyAddedNodeForURL(url_);
   if (node) {
     const std::wstring new_title = title_tf_->GetText();
     if (new_title != node->GetTitle()) {
diff --git a/chrome/browser/views/bookmark_editor_view.cc b/chrome/browser/views/bookmark_editor_view.cc
index 2d777b3..e963b6b 100644
--- a/chrome/browser/views/bookmark_editor_view.cc
+++ b/chrome/browser/views/bookmark_editor_view.cc
@@ -42,22 +42,22 @@ static const int kNewGroupButtonID             = 1002;
 // static
 void BookmarkEditorView::Show(HWND parent_hwnd,
                               Profile* profile,
-                              const GURL& url,
-                              const std::wstring& title) {
+                              BookmarkNode* parent,
+                              BookmarkNode* node) {
   DCHECK(profile);
-  BookmarkEditorView* editor = new BookmarkEditorView(profile, url, title);
+  BookmarkEditorView* editor = new BookmarkEditorView(profile, parent, node);
   editor->Show(parent_hwnd);
 }
 
 BookmarkEditorView::BookmarkEditorView(Profile* profile,
-                                       const GURL& url,
-                                       const std::wstring& title)
+                                       BookmarkNode* parent,
+                                       BookmarkNode* node)
     : profile_(profile),
 #pragma warning(suppress: 4355)  // Okay to pass "this" here.
       new_group_button_(
           l10n_util::GetString(IDS_BOOMARK_EDITOR_NEW_FOLDER_BUTTON)),
-      url_(url),
-      title_(title),
+      parent_(parent),
+      node_(node),
       running_menu_for_root_(false) {
   DCHECK(profile);
   Init();
@@ -238,10 +238,10 @@ void BookmarkEditorView::Init() {
   new_group_button_.SetListener(this);
   new_group_button_.SetID(kNewGroupButtonID);
 
-  title_tf_.SetText(title_);
+  title_tf_.SetText(node_ ? node_->GetTitle() : std::wstring());
   title_tf_.SetController(this);
 
-  url_tf_.SetText(UTF8ToWide(url_.spec()));
+  url_tf_.SetText(node_ ? UTF8ToWide(node_->GetURL().spec()) : std::wstring());
   url_tf_.SetController(this);
 
   // Yummy layout code.
@@ -292,11 +292,7 @@ void BookmarkEditorView::Init() {
   layout->AddPaddingRow(0, kRelatedControlVerticalSpacing);
 
   if (bb_model_->IsLoaded())
-    Loaded(bb_model_);
-}
-
-void BookmarkEditorView::Loaded(BookmarkModel* model) {
-  Reset(true);
+    Reset();
 }
 
 void BookmarkEditorView::BookmarkNodeMoved(BookmarkModel* model,
@@ -304,32 +300,27 @@ void BookmarkEditorView::BookmarkNodeMoved(BookmarkModel* model,
                                            int old_index,
                                            BookmarkNode* new_parent,
                                            int new_index) {
-  Reset(false);
+  Reset();
 }
 
 void BookmarkEditorView::BookmarkNodeAdded(BookmarkModel* model,
                                            BookmarkNode* parent,
                                            int index) {
-  Reset(false);
+  Reset();
 }
 
 void BookmarkEditorView::BookmarkNodeRemoved(BookmarkModel* model,
                                              BookmarkNode* parent,
                                              int index) {
-  Reset(false);
-}
-
-void BookmarkEditorView::Reset(bool first_time) {
-  BookmarkNode* node_editing = bb_model_->GetNodeByURL(url_);
-
-  // If the title is empty we need to fetch it from the node.
-  if (first_time && title_.empty()) {
-    if (node_editing) {
-      title_ = node_editing->GetTitle();
-      title_tf_.SetText(title_);
-    }
+  if ((node_ && !node_->GetParent()) || (parent_ && !parent_->GetParent())) {
+    // The node, or its parent was removed. Close the dialog.
+    window()->Close();
+  } else {
+    Reset();
   }
+}
 
+void BookmarkEditorView::Reset() {
   // Do this first, otherwise when we invoke SetModel with the real one
   // tree_view will try to invoke something on the model we just deleted.
   tree_view_.SetModel(NULL);
@@ -346,9 +337,7 @@ void BookmarkEditorView::Reset(bool first_time) {
 
   if (GetParent()) {
     ExpandAndSelect();
-
-    if(!first_time)
-      UserInputChanged();
+    UserInputChanged();
   } else if (GetParent()) {
     tree_view_.ExpandAll();
   }
@@ -396,11 +385,8 @@ BookmarkEditorView::EditorNode* BookmarkEditorView::AddNewGroup(
 void BookmarkEditorView::ExpandAndSelect() {
   tree_view_.ExpandAll();
 
-  BookmarkNode* to_select = bb_model_->GetNodeByURL(url_);
-  int group_id_to_select =
-      to_select ? to_select->GetParent()->id() :
-                  bb_model_->GetParentForNewNodes()->id();
-
+  BookmarkNode* to_select = node_ ? node_->GetParent() : parent_;
+  int group_id_to_select = to_select->id();
   DCHECK(group_id_to_select);  // GetMostRecentParent should never return NULL.
   EditorNode* b_node =
       FindNodeWithID(tree_model_->GetRoot(), group_id_to_select);
@@ -468,14 +454,8 @@ void BookmarkEditorView::ApplyEdits(EditorNode* parent) {
   GURL new_url(GetInputURL());
   std::wstring new_title(GetInputTitle());
 
-  BookmarkNode* old_node = bb_model_->GetNodeByURL(url_);
-  BookmarkNode* old_parent = old_node ? old_node->GetParent() : NULL;
-  const int old_index = old_parent ? old_parent->IndexOfChild(old_node) : -1;
-
-  if (url_ != new_url) {
-    // The URL has changed, unstar the old url.
-    bb_model_->SetURLStarred(url_, std::wstring(), false);
-  }
+  BookmarkNode* old_parent = node_ ? node_->GetParent() : NULL;
+  const int old_index = old_parent ? old_parent->IndexOfChild(node_) : -1;
 
   // Create the new groups and update the titles.
   BookmarkNode* new_parent = NULL;
@@ -488,29 +468,34 @@ void BookmarkEditorView::ApplyEdits(EditorNode* parent) {
     return;
   }
 
-  BookmarkNode* current_node = bb_model_->GetNodeByURL(new_url);
-
-  if (current_node) {
-    // There's already a node with the URL.
-    bb_model_->SetTitle(current_node, new_title);
-    if (new_parent == old_parent) {
-      // Parent hasn't changed.
-      bb_model_->Move(current_node, new_parent, old_index);
+  if (node_) {
+    Time date_added = node_->date_added();
+    if (new_parent == node_->GetParent()) {
+      // The parent is the same.
+      if (new_url != node_->GetURL()) {
+        bb_model_->Remove(old_parent, old_index);
+        BookmarkNode* new_node =
+            bb_model_->AddURL(old_parent, old_index, new_title, new_url);
+        new_node->date_added_ = date_added;
+      } else {
+        bb_model_->SetTitle(node_, new_title);
+      }
+    } else if (new_url != node_->GetURL()) {
+      // The parent and URL changed.
+      bb_model_->Remove(old_parent, old_index);
+      BookmarkNode* new_node =
+          bb_model_->AddURL(new_parent, new_parent->GetChildCount(), new_title,
+                            new_url);
+      new_node->date_added_ = date_added;
     } else {
-      // Parent changed, move to end of new parent.
-      bb_model_->Move(current_node, new_parent, new_parent->GetChildCount());
+      // The parent and title changed. Move the node and change the title.
+      bb_model_->Move(node_, new_parent, new_parent->GetChildCount());
+      bb_model_->SetTitle(node_, new_title);
     }
   } else {
-    // Adding a new URL.
-    if (new_parent == old_parent) {
-      // Parent hasn't changed. Place newly created bookmark at the same
-      // location as last bookmark.
-      bb_model_->AddURL(new_parent, old_index, new_title, new_url);
-    } else {
-      // Parent changed, put bookmark at end of new parent.
-      bb_model_->AddURL(new_parent, new_parent->GetChildCount(), new_title,
-                        new_url);
-    }
+    // We're adding a new URL.
+    bb_model_->AddURL(new_parent, new_parent->GetChildCount(), new_title,
+                      new_url);
   }
 }
 
diff --git a/chrome/browser/views/bookmark_editor_view.h b/chrome/browser/views/bookmark_editor_view.h
index b928dc9..1f2f3fe 100644
--- a/chrome/browser/views/bookmark_editor_view.h
+++ b/chrome/browser/views/bookmark_editor_view.h
@@ -7,13 +7,13 @@
 
 #include <set>
 
-#include "chrome/views/tree_node_model.h"
 #include "chrome/browser/bookmarks/bookmark_model.h"
 #include "chrome/views/checkbox.h"
 #include "chrome/views/dialog_delegate.h"
 #include "chrome/views/menu.h"
 #include "chrome/views/native_button.h"
 #include "chrome/views/text_field.h"
+#include "chrome/views/tree_node_model.h"
 
 namespace ChromeViews {
 class Window;
@@ -40,21 +40,24 @@ class BookmarkEditorView : public ChromeViews::View,
                            public Menu::Delegate,
                            public BookmarkModelObserver {
   FRIEND_TEST(BookmarkEditorViewTest, ChangeParent);
+  FRIEND_TEST(BookmarkEditorViewTest, ChangeParentAndURL);
   FRIEND_TEST(BookmarkEditorViewTest, ChangeURLToExistingURL);
   FRIEND_TEST(BookmarkEditorViewTest, EditTitleKeepsPosition);
   FRIEND_TEST(BookmarkEditorViewTest, EditURLKeepsPosition);
   FRIEND_TEST(BookmarkEditorViewTest, ModelsMatch);
   FRIEND_TEST(BookmarkEditorViewTest, MoveToNewParent);
+  FRIEND_TEST(BookmarkEditorViewTest, NewURL);
  public:
-  // Shows the BookmarkEditorView editing the specified entry.
+  // Shows the BookmarkEditorView editing |node|. If |node| is NULL a new entry
+  // is created initially parented to |parent|.
   static void Show(HWND parent_window,
                    Profile* profile,
-                   const GURL& url,
-                   const std::wstring& title);
+                   BookmarkNode* parent,
+                   BookmarkNode* node);
 
   BookmarkEditorView(Profile* profile,
-                     const GURL& url,
-                     const std::wstring& title);
+                     BookmarkNode* parent,
+                     BookmarkNode* node);
 
   virtual ~BookmarkEditorView();
 
@@ -135,7 +138,7 @@ class BookmarkEditorView : public ChromeViews::View,
 
   // BookmarkModel observer methods. Any structural change results in
   // resetting the tree model.
-  virtual void Loaded(BookmarkModel* model);
+  virtual void Loaded(BookmarkModel* model) { }
   virtual void BookmarkNodeMoved(BookmarkModel* model,
                                  BookmarkNode* old_parent,
                                  int old_index,
@@ -153,9 +156,7 @@ class BookmarkEditorView : public ChromeViews::View,
                                          BookmarkNode* node) {}
 
   // Resets the model of the tree and updates the various buttons appropriately.
-  // If first_time is true, Reset is being invoked from the constructor or
-  // once the bookmark bar has finished loading.
-  void Reset(bool first_time);
+  void Reset();
 
   // Expands all the nodes in the tree and selects the parent node of the
   // url we're editing or the most recent parent if the url being editted isn't
@@ -233,15 +234,15 @@ class BookmarkEditorView : public ChromeViews::View,
   // Used for editing the title.
   ChromeViews::TextField title_tf_;
 
-  // URL we were created with.
-  const GURL url_;
+  // Initial parent to select. Is only used if node_ is NULL.
+  BookmarkNode* parent_;
+
+  // Node being edited. Is NULL if creating a new node.
+  BookmarkNode* node_;
 
   // The context menu.
   scoped_ptr<Menu> context_menu_;
 
-  // Title of the url to display.
-  std::wstring title_;
-
   // Mode used to create nodes from.
   BookmarkModel* bb_model_;
 
diff --git a/chrome/browser/views/bookmark_editor_view_unittest.cc b/chrome/browser/views/bookmark_editor_view_unittest.cc
index 74baaf5..767d501 100644
--- a/chrome/browser/views/bookmark_editor_view_unittest.cc
+++ b/chrome/browser/views/bookmark_editor_view_unittest.cc
@@ -38,6 +38,10 @@ class BookmarkEditorViewTest : public testing::Test {
 
   std::string base_path() const { return "file:///c:/tmp/"; }
 
+  BookmarkNode* GetNode(const std::string& name) {
+    return model_->GetMostRecentlyAddedNodeForURL(GURL(base_path() + name));
+  }
+
  private:
   // Creates the following structure:
   // bookmark bar node
@@ -72,8 +76,7 @@ class BookmarkEditorViewTest : public testing::Test {
 
 // Makes sure the tree model matches that of the bookmark bar model.
 TEST_F(BookmarkEditorViewTest, ModelsMatch) {
-  BookmarkEditorView editor(profile_.get(), GURL(base_path() + "xxx"),
-                            L"xxx");
+  BookmarkEditorView editor(profile_.get(), NULL, NULL);
   BookmarkEditorView::EditorNode* editor_root = editor.tree_model_->GetRoot();
   // The root should have two children, one for the bookmark bar node,
   // the other for the 'other bookmarks' folder.
@@ -97,7 +100,8 @@ TEST_F(BookmarkEditorViewTest, ModelsMatch) {
 
 // Changes the title and makes sure parent/visual order doesn't change.
 TEST_F(BookmarkEditorViewTest, EditTitleKeepsPosition) {
-  BookmarkEditorView editor(profile_.get(), GURL(base_path() + "a"), L"new_a");
+  BookmarkEditorView editor(profile_.get(), NULL, GetNode("a"));
+  editor.title_tf_.SetText(L"new_a");
 
   editor.ApplyEdits(editor.tree_model_->GetRoot()->GetChild(0));
 
@@ -109,7 +113,9 @@ TEST_F(BookmarkEditorViewTest, EditTitleKeepsPosition) {
 
 // Changes the url and makes sure parent/visual order doesn't change.
 TEST_F(BookmarkEditorViewTest, EditURLKeepsPosition) {
-  BookmarkEditorView editor(profile_.get(), GURL(base_path() + "a"), L"a");
+  Time node_time = Time::Now() + TimeDelta::FromDays(2);
+  GetNode("a")->date_added_ = node_time;
+  BookmarkEditorView editor(profile_.get(), NULL, GetNode("a"));
 
   editor.url_tf_.SetText(UTF8ToWide(GURL(base_path() + "new_a").spec()));
 
@@ -119,11 +125,12 @@ TEST_F(BookmarkEditorViewTest, EditURLKeepsPosition) {
   ASSERT_EQ(L"a", bb_node->GetChild(0)->GetTitle());
   // The URL should have changed.
   ASSERT_TRUE(GURL(base_path() + "new_a") == bb_node->GetChild(0)->GetURL());
+  ASSERT_TRUE(node_time == bb_node->GetChild(0)->date_added());
 }
 
 // Moves 'a' to be a child of the other node.
 TEST_F(BookmarkEditorViewTest, ChangeParent) {
-  BookmarkEditorView editor(profile_.get(), GURL(base_path() + "a"), L"a");
+  BookmarkEditorView editor(profile_.get(), NULL, GetNode("a"));
 
   editor.ApplyEdits(editor.tree_model_->GetRoot()->GetChild(1));
 
@@ -132,28 +139,25 @@ TEST_F(BookmarkEditorViewTest, ChangeParent) {
   ASSERT_TRUE(GURL(base_path() + "a") == other_node->GetChild(2)->GetURL());
 }
 
-// Changes the URL to a URL that is already starred.
-TEST_F(BookmarkEditorViewTest, ChangeURLToExistingURL) {
-  BookmarkEditorView editor(profile_.get(), GURL(base_path() + "a"), L"a");
-
-  editor.url_tf_.SetText(UTF8ToWide(GURL(base_path() + "f1a").spec()));
+// Moves 'a' to be a child of the other node and changes its url to new_a.
+TEST_F(BookmarkEditorViewTest, ChangeParentAndURL) {
+  Time node_time = Time::Now() + TimeDelta::FromDays(2);
+  GetNode("a")->date_added_ = node_time;
+  BookmarkEditorView editor(profile_.get(), NULL, GetNode("a"));
 
-  editor.ApplyEdits(editor.tree_model_->GetRoot()->GetChild(0));
+  editor.url_tf_.SetText(UTF8ToWide(GURL(base_path() + "new_a").spec()));
 
-  // Position shouldn't have changed.
-  BookmarkNode* bb_node = profile_->GetBookmarkModel()->GetBookmarkBarNode();
-  ASSERT_EQ(L"a", bb_node->GetChild(0)->GetTitle());
-  // The URL should have changed.
-  ASSERT_TRUE(GURL(base_path() + "f1a") == bb_node->GetChild(0)->GetURL());
+  editor.ApplyEdits(editor.tree_model_->GetRoot()->GetChild(1));
 
-  // And F1 should have one last child (f1a was removed from it).
-  ASSERT_EQ(1, bb_node->GetChild(1)->GetChildCount());
-  ASSERT_NE(L"f1a", bb_node->GetChild(1)->GetChild(0)->GetTitle());
+  BookmarkNode* other_node = profile_->GetBookmarkModel()->other_node();
+  ASSERT_EQ(L"a", other_node->GetChild(2)->GetTitle());
+  ASSERT_TRUE(GURL(base_path() + "new_a") == other_node->GetChild(2)->GetURL());
+  ASSERT_TRUE(node_time == other_node->GetChild(2)->date_added());
 }
 
 // Creates a new folder and moves a node to it.
 TEST_F(BookmarkEditorViewTest, MoveToNewParent) {
-  BookmarkEditorView editor(profile_.get(), GURL(base_path() + "a"), L"a");
+  BookmarkEditorView editor(profile_.get(), NULL, GetNode("a"));
 
   // Create two nodes: "F21" as a child of "F2" and "F211" as a child of "F21".
   BookmarkEditorView::EditorNode* f2 =
@@ -182,3 +186,20 @@ TEST_F(BookmarkEditorViewTest, MoveToNewParent) {
   ASSERT_EQ(L"F211", mf21->GetChild(0)->GetTitle());
 }
 
+// Brings up the editor, creating a new URL on the bookmark bar.
+TEST_F(BookmarkEditorViewTest, NewURL) {
+  BookmarkEditorView editor(profile_.get(), NULL, NULL);
+
+  editor.url_tf_.SetText(UTF8ToWide(GURL(base_path() + "a").spec()));
+  editor.title_tf_.SetText(L"new_a");
+
+  editor.ApplyEdits(editor.tree_model_->GetRoot()->GetChild(0));
+
+  BookmarkNode* bb_node = profile_->GetBookmarkModel()->GetBookmarkBarNode();
+  ASSERT_EQ(4, bb_node->GetChildCount());
+
+  BookmarkNode* new_node = bb_node->GetChild(3);
+
+  EXPECT_EQ(L"new_a", new_node->GetTitle());
+  EXPECT_TRUE(GURL(base_path() + "a") == new_node->GetURL());
+}
diff --git a/chrome/browser/views/toolbar_view.cc b/chrome/browser/views/toolbar_view.cc
index abbad21..8d7ce07 100644
--- a/chrome/browser/views/toolbar_view.cc
+++ b/chrome/browser/views/toolbar_view.cc
@@ -10,6 +10,8 @@
 #include "base/path_service.h"
 #include "chrome/app/chrome_dll_resource.h"
 #include "chrome/app/theme/theme_resources.h"
+#include "chrome/browser/bookmarks/bookmark_drag_data.h"
+#include "chrome/browser/bookmarks/bookmark_model.h"
 #include "chrome/browser/browser.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/character_encoding.h"
@@ -668,6 +670,11 @@ int BrowserToolbarView::GetDragOperations(ChromeViews::View* sender,
       !tab_->GetURL().is_valid()) {
     return DragDropTypes::DRAG_NONE;
   }
+  if (profile_ && profile_->GetBookmarkModel() &&
+      profile_->GetBookmarkModel()->IsBookmarked(tab_->GetURL())) {
+    return DragDropTypes::DRAG_MOVE | DragDropTypes::DRAG_COPY |
+           DragDropTypes::DRAG_LINK;
+  }
   return DragDropTypes::DRAG_COPY | DragDropTypes::DRAG_LINK;
 }
 
@@ -680,6 +687,18 @@ void BrowserToolbarView::WriteDragData(ChromeViews::View* sender,
 
   UserMetrics::RecordAction(L"Toolbar_DragStar", profile_);
 
+  // If there is a bookmark for the URL, add the bookmark drag data for it. We
+  // do this to ensure the bookmark is moved, rather than creating an new
+  // bookmark.
+  if (profile_ && profile_->GetBookmarkModel()) {
+    BookmarkNode* node = profile_->GetBookmarkModel()->
+        GetMostRecentlyAddedNodeForURL(tab_->GetURL());
+    if (node) {
+      BookmarkDragData bookmark_data(node);
+      bookmark_data.Write(profile_, data);
+    }
+  }
+
   drag_utils::SetURLAndDragImage(tab_->GetURL(), tab_->GetTitle(),
                                  tab_->GetFavIcon(), data);
 }
diff --git a/chrome/browser/web_contents.cc b/chrome/browser/web_contents.cc
index 72f8d82..09be89a 100644
--- a/chrome/browser/web_contents.cc
+++ b/chrome/browser/web_contents.cc
@@ -1374,7 +1374,7 @@ void WebContents::WebAppImagesChanged(WebApp* web_app) {
 void WebContents::UpdateStarredStateForCurrentURL() {
   BookmarkModel* model = profile()->GetBookmarkModel();
   const bool old_state = is_starred_;
-  is_starred_ = (model && model->GetNodeByURL(GetURL()));
+  is_starred_ = (model && model->IsBookmarked(GetURL()));
 
   if (is_starred_ != old_state && delegate())
     delegate()->URLStarredChanged(this, is_starred_);
diff --git a/chrome/common/notification_types.h b/chrome/common/notification_types.h
index e2a0352..50e3b5b 100644
--- a/chrome/common/notification_types.h
+++ b/chrome/common/notification_types.h
@@ -355,7 +355,7 @@ enum NotificationType {
   // No details are expected.
   NOTIFY_AUTH_SUPPLIED,
 
-  // History, bookmarks --------------------------------------------------------
+  // History -------------------------------------------------------------------
 
   // Sent when a history service is created on the main thread. This is sent
   // after history is created, but before it has finished loading. Use
@@ -389,21 +389,23 @@ enum NotificationType {
   // the details is history::URLsDeletedDetails that lists the deleted URLs.
   NOTIFY_HISTORY_URLS_DELETED,
 
-  // This is sent when the "starred" button is clicked, toggling the
-  // starredness of a tab's current URL. The source is a Profile and the
-  // details is history::URLsStarredDetails that contains the list of URLs and
-  // whether they were starred or unstarred.
+  // Sent by history when the favicon of a URL changes.
+  // The source is the profile, and the details is
+  // history::FavIconChangeDetails (see history_notifications.h).
+  NOTIFY_FAVICON_CHANGED,
+
+  // Bookmarks -----------------------------------------------------------------
+
+  // Sent when the starred state of a URL changes. A URL is starred if there is
+  // at least one bookmark for it. The source is a Profile and the details is
+  // history::URLsStarredDetails that contains the list of URLs and whether
+  // they were starred or unstarred.
   NOTIFY_URLS_STARRED,
 
   // Sent when the bookmark bar model finishes loading. This source is the
   // Profile, and the details aren't used.
   NOTIFY_BOOKMARK_MODEL_LOADED,
 
-  // Sent by history when the favicon of a URL changes.
-  // The source is the profile, and the details is
-  // history::FavIconChangeDetails (see history_notifications.h).
-  NOTIFY_FAVICON_CHANGED,
-
   // Sent when the bookmark bubble is shown for a particular URL. The source
   // is the profile, the details the URL.
   NOTIFY_BOOKMARK_BUBBLE_SHOWN,
diff --git a/chrome/test/testing_profile.cc b/chrome/test/testing_profile.cc
index 59cf5a5..272f94c 100644
--- a/chrome/test/testing_profile.cc
+++ b/chrome/test/testing_profile.cc
@@ -4,6 +4,7 @@
 
 #include "chrome/test/testing_profile.h"
 
+#include "base/string_util.h"
 #include "chrome/browser/history/history_backend.h"
 #include "chrome/common/chrome_constants.h"
 
@@ -49,6 +50,14 @@ TestingProfile::TestingProfile()
   file_util::CreateDirectory(path_);
 }
 
+TestingProfile::TestingProfile(int count)
+    : start_time_(Time::Now()), has_history_service_(false) {
+  PathService::Get(base::DIR_TEMP, &path_);
+  file_util::AppendToPath(&path_, L"TestingProfilePath" + IntToWString(count));
+  file_util::Delete(path_, true);
+  file_util::CreateDirectory(path_);
+}
+
 TestingProfile::~TestingProfile() {
   DestroyHistoryService();
   file_util::Delete(path_, true);
diff --git a/chrome/test/testing_profile.h b/chrome/test/testing_profile.h
index 568369f..c68e9fcb 100644
--- a/chrome/test/testing_profile.h
+++ b/chrome/test/testing_profile.h
@@ -18,6 +18,10 @@
 class TestingProfile : public Profile {
  public:
   TestingProfile();
+  // Creates a new profile by adding |count| to the end of the path. Use this
+  // when you need to have more than one TestingProfile running at the same
+  // time.
+  explicit TestingProfile(int count);
   virtual ~TestingProfile();
 
   // Creates the history service. If |delete_file| is true, the history file is
@@ -107,9 +111,10 @@ class TestingProfile : public Profile {
   virtual void SetName(const std::wstring& name) {
   }
   virtual std::wstring GetID() {
-    return std::wstring();
+    return id_;
   }
   virtual void SetID(const std::wstring& id) {
+    id_ = id;
   }
   virtual bool DidLastSessionExitCleanly() {
     return true;
@@ -170,6 +175,8 @@ class TestingProfile : public Profile {
   // Do we have a history service? This defaults to the value of
   // history_service, but can be explicitly set.
   bool has_history_service_;
+
+  std::wstring id_;
 };
 
 #endif  // CHROME_TEST_TESTING_PROFILE_H__