Changes BookmarkDragData to allow for multiple nodes. I'm going to

need this for the bookmark manager, which will let you drag multiple
nodes around.

I didn't make the bookmark bar deal with multiple nodes though (it
won't allow a drop of multiple nodes). I may add support for this at
some point, but not in this patch.

BUG=674
TEST=covered by unit tests, but make sure the aren't problems dragging
  around bookmarks on the bookmark bar, especially with folders.

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

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

5 files changed, 306 insertions, 150 deletions

diff --git a/chrome/browser/bookmarks/bookmark_drag_data.cc b/chrome/browser/bookmarks/bookmark_drag_data.cc
index d2ff684..65d3445 100644
--- a/chrome/browser/bookmarks/bookmark_drag_data.cc
+++ b/chrome/browser/bookmarks/bookmark_drag_data.cc
@@ -13,22 +13,66 @@ static CLIPFORMAT clipboard_format = 0;
 
 static void RegisterFormat() {
   if (clipboard_format == 0) {
-    clipboard_format = RegisterClipboardFormat(L"chrome/x-bookmark-entry");
+    clipboard_format = RegisterClipboardFormat(L"chrome/x-bookmark-entries");
     DCHECK(clipboard_format);
   }
 }
 
-BookmarkDragData::BookmarkDragData() : is_url(false), is_valid(false) {
-}
-
-BookmarkDragData::BookmarkDragData(BookmarkNode* node)
-    : is_url(node->GetType() == history::StarredEntry::URL),
+BookmarkDragData::Element::Element(BookmarkNode* node)
+    : is_url(node->is_url()),
       url(node->GetURL()),
       title(node->GetTitle()),
-      is_valid(true),
       id_(node->id()) {
-  if (!is_url)
-    AddChildren(node);
+  for (int i = 0; i < node->GetChildCount(); ++i)
+    children.push_back(Element(node->GetChild(i)));
+}
+
+void BookmarkDragData::Element::WriteToPickle(Pickle* pickle) const {
+  pickle->WriteBool(is_url);
+  pickle->WriteString(url.spec());
+  pickle->WriteWString(title);
+  pickle->WriteInt(id_);
+  if (!is_url) {
+    pickle->WriteSize(children.size());
+    for (std::vector<Element>::const_iterator i = children.begin();
+         i != children.end(); ++i) {
+      i->WriteToPickle(pickle);
+    }
+  }
+}
+
+bool BookmarkDragData::Element::ReadFromPickle(Pickle* pickle,
+                                               void** iterator) {
+  std::string url_spec;
+  if (!pickle->ReadBool(iterator, &is_url) ||
+      !pickle->ReadString(iterator, &url_spec) ||
+      !pickle->ReadWString(iterator, &title) ||
+      !pickle->ReadInt(iterator, &id_)) {
+    return false;
+  }
+  url = GURL(url_spec);
+  children.clear();
+  if (!is_url) {
+    size_t children_count;
+    if (!pickle->ReadSize(iterator, &children_count))
+      return false;
+    children.resize(children_count);
+    for (std::vector<Element>::iterator i = children.begin();
+         i != children.end(); ++i) {
+      if (!i->ReadFromPickle(pickle, iterator))
+        return false;
+    }
+  }
+  return true;
+}
+
+BookmarkDragData::BookmarkDragData(BookmarkNode* node) {
+  elements.push_back(Element(node));
+}
+
+BookmarkDragData::BookmarkDragData(const std::vector<BookmarkNode*>& nodes) {
+  for (size_t i = 0; i < nodes.size(); ++i)
+    elements.push_back(Element(nodes[i]));
 }
 
 void BookmarkDragData::Write(Profile* profile, OSExchangeData* data) const {
@@ -36,81 +80,77 @@ void BookmarkDragData::Write(Profile* profile, OSExchangeData* data) const {
 
   DCHECK(data);
 
-  if (is_url)
-    data->SetURL(url, title);
+  // If there is only one element and it is a URL, write the URL to the
+  // clipboard.
+  if (elements.size() == 1 && elements[0].is_url)
+    data->SetURL(elements[0].url, elements[0].title);
+
   Pickle data_pickle;
   data_pickle.WriteWString(profile->GetPath());
-  data_pickle.WriteInt(id_);
-  WriteToPickle(&data_pickle);
+  data_pickle.WriteSize(elements.size());
+
+  for (size_t i = 0; i < elements.size(); ++i)
+    elements[i].WriteToPickle(&data_pickle);
+
   data->SetPickledData(clipboard_format, data_pickle);
 }
 
 bool BookmarkDragData::Read(const OSExchangeData& data) {
   RegisterFormat();
 
-  is_valid = data.GetURLAndTitle(&url, &title) && url.is_valid();
-  is_url = is_valid;
+  elements.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;
+      size_t element_count;
       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;
+          drag_data_pickle.ReadSize(&data_iterator, &element_count)) {
+        std::vector<Element> tmp_elements;
+        tmp_elements.resize(element_count);
+        for (size_t i = 0; i < element_count; ++i) {
+          if (!tmp_elements[i].ReadFromPickle(&drag_data_pickle,
+                                              &data_iterator)) {
+            return false;
+          }
+        }
+        elements.swap(tmp_elements);
       }
     }
+  } else {
+    // See if there is a URL on the clipboard.
+    Element element;
+    if (data.GetURLAndTitle(&element.url, &element.title) &&
+        element.url.is_valid()) {
+      element.is_url = true;
+      elements.push_back(element);
+    }
   }
-  return is_valid;
+  return is_valid();
 }
 
-BookmarkNode* BookmarkDragData::GetNode(Profile* profile) const {
-  DCHECK(is_valid);
-  return (profile->GetPath() == profile_path_) ?
-      profile->GetBookmarkModel()->GetNodeByID(id_) : NULL;
-}
+std::vector<BookmarkNode*> BookmarkDragData::GetNodes(Profile* profile) const {
+  std::vector<BookmarkNode*> nodes;
 
-void BookmarkDragData::WriteToPickle(Pickle* pickle) const {
-  pickle->WriteBool(is_url);
-  pickle->WriteString(url.spec());
-  pickle->WriteWString(title);
-  if (!is_url) {
-    pickle->WriteInt(static_cast<int>(children.size()));
-    for (std::vector<BookmarkDragData>::const_iterator i = children.begin();
-         i != children.end(); ++i) {
-      i->WriteToPickle(pickle);
-    }
-  }
-}
+  if (profile->GetPath() != profile_path_)
+    return nodes;
 
-bool BookmarkDragData::ReadFromPickle(Pickle* pickle, void** iterator) {
-  std::string url_spec;
-  is_valid = false;
-  if (!pickle->ReadBool(iterator, &is_url) ||
-      !pickle->ReadString(iterator, &url_spec) ||
-      !pickle->ReadWString(iterator, &title)) {
-    return false;
-  }
-  url = GURL(url_spec);
-  if (!is_url) {
-    children.clear();
-    int children_count;
-    if (!pickle->ReadInt(iterator, &children_count))
-      return false;
-    children.resize(children_count);
-    for (std::vector<BookmarkDragData>::iterator i = children.begin();
-         i != children.end(); ++i) {
-      if (!i->ReadFromPickle(pickle, iterator))
-        return false;
+  for (size_t i = 0; i < elements.size(); ++i) {
+    BookmarkNode* node =
+        profile->GetBookmarkModel()->GetNodeByID(elements[i].id_);
+    if (!node) {
+      nodes.clear();
+      return nodes;
     }
+    nodes.push_back(node);
   }
-  is_valid = true;
-  return true;
+  return nodes;
 }
 
-void BookmarkDragData::AddChildren(BookmarkNode* node) {
-  for (int i = 0, max = node->GetChildCount(); i < max; ++i)
-    children.push_back(BookmarkDragData(node->GetChild(i)));
+BookmarkNode* BookmarkDragData::GetFirstNode(Profile* profile) const {
+  std::vector<BookmarkNode*> nodes = GetNodes(profile);
+  return nodes.size() == 1 ? nodes[0] : NULL;
 }
diff --git a/chrome/browser/bookmarks/bookmark_drag_data.h b/chrome/browser/bookmarks/bookmark_drag_data.h
index e3e1b3f..10d269b 100644
--- a/chrome/browser/bookmarks/bookmark_drag_data.h
+++ b/chrome/browser/bookmarks/bookmark_drag_data.h
@@ -16,68 +16,94 @@ 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.
+// BookmarkDragData is used to represent the following:
+//
+// . A single URL.
+// . A single node from the bookmark model.
+// . A set of nodes from the bookmark model.
+//
+// BookmarkDragData is used by bookmark related views to represent a dragged
+// bookmark or bookmarks.
 //
 // Typical usage when writing data for a drag is:
 //   BookmarkDragData data(node_user_is_dragging);
-//   data.profile_id = profile_id;
 //   data.Write(os_exchange_data_for_drag);
 //
 // Typical usage to read is:
 //   BookmarkDragData data;
 //   if (data.Read(os_exchange_data))
-//     // data is valid
+//     // data is valid, contents are in elements.
 
 struct BookmarkDragData {
-  BookmarkDragData();
 
-  // Created a BookmarkDragData populated from node.
+  // Element represents a single node.
+  struct Element {
+    explicit Element(BookmarkNode* node);
+
+    Element() : is_url(false), id_(0) {}
+
+    // If true, this element represents a URL.
+    bool is_url;
+
+    // The URL, only valid if is_url is true.
+    GURL url;
+
+    // Title of the entry, used for both urls and groups/folders.
+    std::wstring title;
+
+    // Children, only used for non-URL nodes.
+    std::vector<Element> children;
+
+   private:
+    friend struct BookmarkDragData;
+
+    // For reading/writing this Element.
+    void WriteToPickle(Pickle* pickle) const;
+    bool ReadFromPickle(Pickle* pickle, void** iterator);
+
+    // ID of the node.
+    int id_;
+  };
+
+  BookmarkDragData() { }
+
+  // Created a BookmarkDragData populated from the arguments.
   explicit BookmarkDragData(BookmarkNode* node);
+  explicit BookmarkDragData(const std::vector<BookmarkNode*>& nodes);
 
-  // Writes this BookmarkDragData to data. If BookmarkDragData is a URL,
-  // this writes out the URL and URL title clipboard data as well.
+  // Writes elements to data. If there is only one element and it is a URL
+  // the URL and title are written to the clipboard in a format other apps can
+  // use.
   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 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* GetNode(Profile* profile) const;
+  // Returns the nodes represented by this DragData. If this DragData was
+  // created from the same profile then the nodes from the model are returned.
+  // If the nodes can't be found (may have been deleted), an empty vector is
+  // returned.
+  std::vector<BookmarkNode*> GetNodes(Profile* profile) const;
 
-  // If true, this entry represents a StarredEntry of type URL.
-  bool is_url;
+  // Convenience for getting the first node. Returns NULL if the data doesn't
+  // match any nodes or there is more than one node.
+  BookmarkNode* GetFirstNode(Profile* profile) const;
 
-  // The URL, only valid if is_url is true.
-  GURL url;
+  // Do we contain valid data?
+  bool is_valid() const { return !elements.empty(); }
 
-  // Title of the entry
-  std::wstring title;
+  // Returns true if there is a single url.
+  bool has_single_url() const { return is_valid() && elements[0].is_url; }
 
-  // Children, only used for non-URL nodes.
-  std::vector<BookmarkDragData> children;
+  // Number of elements.
+  size_t size() { return elements.size(); }
 
-  // If true our data is valid.
-  bool is_valid;
+  // The actual elements written to the clipboard.
+  std::vector<Element> elements;
 
  private:
-  // Writes the data to a Pickle.
-  void WriteToPickle(Pickle* pickle) const;
-
-  bool ReadFromPickle(Pickle* pickle, void** iterator);
-
-  // 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_;
 };
 
 #endif  // CHROME_BROWSER_BOOKMARKS_BOOKMARK_DRAG_DATA_
diff --git a/chrome/browser/bookmarks/bookmark_drag_data_unittest.cc b/chrome/browser/bookmarks/bookmark_drag_data_unittest.cc
index a3022f8..c952471 100644
--- a/chrome/browser/bookmarks/bookmark_drag_data_unittest.cc
+++ b/chrome/browser/bookmarks/bookmark_drag_data_unittest.cc
@@ -12,20 +12,41 @@
 
 typedef testing::Test BookmarkDragDataTest;
 
+// Makes sure BookmarkDragData is initially invalid.
 TEST_F(BookmarkDragDataTest, InitialState) {
   BookmarkDragData data;
-  EXPECT_FALSE(data.is_valid);
+  EXPECT_FALSE(data.is_valid());
 }
 
+// Makes sure reading bogus data leaves the BookmarkDragData invalid.
 TEST_F(BookmarkDragDataTest, BogusRead) {
   scoped_refptr<OSExchangeData> data(new OSExchangeData());
   BookmarkDragData drag_data;
-  drag_data.is_valid = true;
   EXPECT_FALSE(drag_data.Read(data.get()));
-  EXPECT_FALSE(drag_data.is_valid);
+  EXPECT_FALSE(drag_data.is_valid());
+}
+
+// Writes a URL to the clipboard and make sure BookmarkDragData can correctly
+// read it.
+TEST_F(BookmarkDragDataTest, JustURL) {
+  const GURL url("http://google.com");
+  const std::wstring title(L"title");
+
+  scoped_refptr<OSExchangeData> data(new OSExchangeData());
+  data->SetURL(url, title);
+
+  BookmarkDragData drag_data;
+  EXPECT_TRUE(drag_data.Read(data.get()));
+  EXPECT_TRUE(drag_data.is_valid());
+  ASSERT_EQ(1, drag_data.elements.size());
+  EXPECT_TRUE(drag_data.elements[0].is_url);
+  EXPECT_TRUE(drag_data.elements[0].url == url);
+  EXPECT_TRUE(drag_data.elements[0].title == title);
+  EXPECT_EQ(0, drag_data.elements[0].children.size());
 }
 
 TEST_F(BookmarkDragDataTest, URL) {
+  // Write a single node representing a URL to the clipboard.
   TestingProfile profile;
   profile.CreateBookmarkModel(false);
   profile.SetID(L"id");
@@ -35,10 +56,11 @@ TEST_F(BookmarkDragDataTest, URL) {
   const std::wstring title(L"blah");
   BookmarkNode* node = model->AddURL(root, 0, title, url);
   BookmarkDragData drag_data(node);
-  EXPECT_TRUE(drag_data.url == url);
-  EXPECT_EQ(title, drag_data.title);
-  EXPECT_TRUE(drag_data.is_url);
-
+  EXPECT_TRUE(drag_data.is_valid());
+  ASSERT_EQ(1, drag_data.elements.size());
+  EXPECT_TRUE(drag_data.elements[0].is_url);
+  EXPECT_TRUE(drag_data.elements[0].url == url);
+  EXPECT_EQ(title, drag_data.elements[0].title);
   scoped_refptr<OSExchangeData> data(new OSExchangeData());
   drag_data.Write(&profile, data.get());
 
@@ -46,14 +68,16 @@ TEST_F(BookmarkDragDataTest, URL) {
   scoped_refptr<OSExchangeData> data2(new OSExchangeData(data.get()));
   BookmarkDragData read_data;
   EXPECT_TRUE(read_data.Read(*data2));
-  EXPECT_TRUE(read_data.url == 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);
-
+  EXPECT_TRUE(read_data.is_valid());
+  ASSERT_EQ(1, read_data.elements.size());
+  EXPECT_TRUE(read_data.elements[0].is_url);
+  EXPECT_TRUE(read_data.elements[0].url == url);
+  EXPECT_EQ(title, read_data.elements[0].title);
+  EXPECT_TRUE(read_data.GetFirstNode(&profile) == node);
+
+  // Make sure asking for the node with a different profile returns NULL.
   TestingProfile profile2(1);
-  EXPECT_TRUE(read_data.GetNode(&profile2) == NULL);
+  EXPECT_TRUE(read_data.GetFirstNode(&profile2) == NULL);
 
   // Writing should also put the URL and title on the clipboard.
   GURL read_url;
@@ -63,6 +87,7 @@ TEST_F(BookmarkDragDataTest, URL) {
   EXPECT_EQ(title, read_title);
 }
 
+// Tests writing a group to the clipboard.
 TEST_F(BookmarkDragDataTest, Group) {
   TestingProfile profile;
   profile.CreateBookmarkModel(false);
@@ -74,8 +99,10 @@ TEST_F(BookmarkDragDataTest, Group) {
   BookmarkNode* g12 = model->AddGroup(g1, 0, L"g12");
 
   BookmarkDragData drag_data(g12);
-  EXPECT_EQ(g12->GetTitle(), drag_data.title);
-  EXPECT_FALSE(drag_data.is_url);
+  EXPECT_TRUE(drag_data.is_valid());
+  ASSERT_EQ(1, drag_data.elements.size());
+  EXPECT_EQ(g12->GetTitle(), drag_data.elements[0].title);
+  EXPECT_FALSE(drag_data.elements[0].is_url);
 
   scoped_refptr<OSExchangeData> data(new OSExchangeData());
   drag_data.Write(&profile, data.get());
@@ -84,17 +111,21 @@ TEST_F(BookmarkDragDataTest, Group) {
   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_TRUE(read_data.is_valid);
-  EXPECT_FALSE(read_data.is_url);
+  EXPECT_TRUE(read_data.is_valid());
+  ASSERT_EQ(1, read_data.elements.size());
+  EXPECT_EQ(g12->GetTitle(), read_data.elements[0].title);
+  EXPECT_FALSE(read_data.elements[0].is_url);
 
-  BookmarkNode* r_g12 = read_data.GetNode(&profile);
+  // We should get back the same node when asking for the same profile.
+  BookmarkNode* r_g12 = read_data.GetFirstNode(&profile);
   EXPECT_TRUE(g12 == r_g12);
 
+  // A different profile should return NULL for the node.
   TestingProfile profile2(1);
-  EXPECT_TRUE(read_data.GetNode(&profile2) == NULL);
+  EXPECT_TRUE(read_data.GetFirstNode(&profile2) == NULL);
 }
 
+// Tests reading/writing a folder with children.
 TEST_F(BookmarkDragDataTest, GroupWithChild) {
   TestingProfile profile;
   profile.SetID(L"id");
@@ -117,14 +148,68 @@ TEST_F(BookmarkDragDataTest, GroupWithChild) {
   scoped_refptr<OSExchangeData> data2(new OSExchangeData(data.get()));
   BookmarkDragData read_data;
   EXPECT_TRUE(read_data.Read(*data2));
+  ASSERT_EQ(1, read_data.elements.size());
+  ASSERT_EQ(1, read_data.elements[0].children.size());
+  const BookmarkDragData::Element& read_child =
+      read_data.elements[0].children[0];
+
+  EXPECT_TRUE(read_child.is_url);
+  EXPECT_EQ(title, read_child.title);
+  EXPECT_TRUE(url == read_child.url);
+  EXPECT_TRUE(read_child.is_url);
+
+  // And make sure we get the node back.
+  BookmarkNode* r_group = read_data.GetFirstNode(&profile);
+  EXPECT_TRUE(group == r_group);
+}
 
-  EXPECT_EQ(1, read_data.children.size());
-  EXPECT_TRUE(read_data.children[0].is_valid);
-  EXPECT_TRUE(read_data.children[0].is_url);
-  EXPECT_EQ(title, read_data.children[0].title);
-  EXPECT_TRUE(url == read_data.children[0].url);
-  EXPECT_TRUE(read_data.children[0].is_url);
+// Tests reading/writing of multiple nodes.
+TEST_F(BookmarkDragDataTest, MultipleNodes) {
+  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");
 
-  BookmarkNode* r_group = read_data.GetNode(&profile);
-  EXPECT_TRUE(group == r_group);
+  GURL url(GURL("http://foo.com"));
+  const std::wstring title(L"blah2");
+
+  BookmarkNode* url_node = model->AddURL(group, 0, title, url);
+
+  // Write the nodes to the clipboard.
+  std::vector<BookmarkNode*> nodes;
+  nodes.push_back(group);
+  nodes.push_back(url_node);
+  BookmarkDragData drag_data(nodes);
+  scoped_refptr<OSExchangeData> data(new OSExchangeData());
+  drag_data.Write(&profile, data.get());
+
+  // Read the data back in.
+  scoped_refptr<OSExchangeData> data2(new OSExchangeData(data.get()));
+  BookmarkDragData read_data;
+  EXPECT_TRUE(read_data.Read(*data2));
+  EXPECT_TRUE(read_data.is_valid());
+  ASSERT_EQ(2, read_data.elements.size());
+  ASSERT_EQ(1, read_data.elements[0].children.size());
+
+  const BookmarkDragData::Element& read_group = read_data.elements[0];
+  EXPECT_FALSE(read_group.is_url);
+  EXPECT_EQ(L"g1", read_group.title);
+  EXPECT_EQ(1, read_group.children.size());
+
+  const BookmarkDragData::Element& read_url = read_data.elements[1];
+  EXPECT_TRUE(read_url.is_url);
+  EXPECT_EQ(title, read_url.title);
+  EXPECT_EQ(0, read_url.children.size());
+
+  // And make sure we get the node back.
+  std::vector<BookmarkNode*> read_nodes = read_data.GetNodes(&profile);
+  ASSERT_EQ(2, read_nodes.size());
+  EXPECT_TRUE(read_nodes[0] == group);
+  EXPECT_TRUE(read_nodes[1] == url_node);
+
+  // Asking for the first node should return NULL with more than one element
+  // present.
+  EXPECT_TRUE(read_data.GetFirstNode(&profile) == NULL);
 }
diff --git a/chrome/browser/views/bookmark_bar_view.cc b/chrome/browser/views/bookmark_bar_view.cc
index 1ddf120..457339c 100644
--- a/chrome/browser/views/bookmark_bar_view.cc
+++ b/chrome/browser/views/bookmark_bar_view.cc
@@ -472,13 +472,15 @@ class MenuRunner : public views::MenuDelegate,
   }
 
   virtual bool CanDrop(MenuItemView* menu, const OSExchangeData& data) {
-    if (!drop_data_.Read(data))
+    // Only accept drops of 1 node, which is the case for all data dragged from
+    // bookmark bar and menus.
+    if (!drop_data_.Read(data) || drop_data_.elements.size() != 1)
       return false;
 
-    if (drop_data_.is_url)
+    if (drop_data_.has_single_url())
       return true;
 
-    BookmarkNode* drag_node = drop_data_.GetNode(view_->GetProfile());
+    BookmarkNode* drag_node = drop_data_.GetFirstNode(view_->GetProfile());
     if (!drag_node) {
       // Dragging a group from another profile, always accept.
       return true;
@@ -496,7 +498,7 @@ class MenuRunner : public views::MenuDelegate,
   virtual int GetDropOperation(MenuItemView* item,
                                const views::DropTargetEvent& event,
                                DropPosition* position) {
-    DCHECK(drop_data_.is_valid);
+    DCHECK(drop_data_.is_valid());
     BookmarkNode* node = menu_id_to_node_map_[item->GetCommand()];
     BookmarkNode* drop_parent = node->GetParent();
     int index_to_drop_at = drop_parent->IndexOfChild(node);
@@ -972,7 +974,9 @@ bool BookmarkBarView::CanDrop(const OSExchangeData& data) {
   if (!drop_info_.get())
     drop_info_.reset(new DropInfo());
 
-  return drop_info_->data.Read(data);
+  // Only accept drops of 1 node, which is the case for all data dragged from
+  // bookmark bar and menus.
+  return drop_info_->data.Read(data) && drop_info_->data.size() == 1;
 }
 
 void BookmarkBarView::OnDragEntered(const DropTargetEvent& event) {
@@ -1068,7 +1072,7 @@ int BookmarkBarView::OnPerformDrop(const DropTargetEvent& event) {
   const bool drop_on = drop_info_->drop_on;
   const BookmarkDragData data = drop_info_->data;
   const bool is_over_other = drop_info_->is_over_other;
-  DCHECK(data.is_valid);
+  DCHECK(data.is_valid());
 
   if (drop_info_->drop_index != -1) {
     // TODO(sky): optimize the SchedulePaint region.
@@ -1628,7 +1632,7 @@ int BookmarkBarView::CalculateDropOperation(const DropTargetEvent& event,
                                             bool* is_over_other) {
   DCHECK(model_);
   DCHECK(model_->IsLoaded());
-  DCHECK(data.is_valid);
+  DCHECK(data.is_valid());
 
   // The drop event uses the screen coordinates while the child Views are
   // always laid out from left to right (even though they are rendered from
@@ -1659,7 +1663,7 @@ int BookmarkBarView::CalculateDropOperation(const DropTargetEvent& event,
   } else if (!GetBookmarkButtonCount()) {
     // No bookmarks, accept the drop.
     *index = 0;
-    int ops = data.GetNode(profile_)
+    int ops = data.GetFirstNode(profile_)
         ? DragDropTypes::DRAG_MOVE
         : DragDropTypes::DRAG_COPY | DragDropTypes::DRAG_LINK;
     return PreferredDropOperation(event, ops);
@@ -1722,7 +1726,8 @@ int BookmarkBarView::CalculateDropOperation(const DropTargetEvent& event,
                          model_->GetBookmarkBarNode()->GetChild(*index);
     int operation =
         CalculateDropOperation(event, data, parent, parent->GetChildCount());
-    if (!operation && !data.is_url && data.GetNode(profile_) == parent) {
+    if (!operation && !data.has_single_url() &&
+        data.GetFirstNode(profile_) == parent) {
       // Don't open a menu if the node being dragged is the the menu to
       // open.
       *drop_on = false;
@@ -1741,7 +1746,7 @@ int BookmarkBarView::CalculateDropOperation(const DropTargetEvent& event,
   if (!CanDropAt(data, parent, index))
     return DragDropTypes::DRAG_NONE;
 
-  if (data.GetNode(profile_)) {
+  if (data.GetFirstNode(profile_)) {
     // User is dragging from this profile: move.
     return DragDropTypes::DRAG_MOVE;
   } else {
@@ -1754,8 +1759,8 @@ int BookmarkBarView::CalculateDropOperation(const DropTargetEvent& event,
 bool BookmarkBarView::CanDropAt(const BookmarkDragData& data,
                                 BookmarkNode* parent,
                                 int index) {
-  DCHECK(data.is_valid);
-  BookmarkNode* dragged_node = data.GetNode(profile_);
+  DCHECK(data.is_valid());
+  BookmarkNode* dragged_node = data.GetFirstNode(profile_);
   if (dragged_node) {
     if (dragged_node->GetParent() == parent) {
       const int existing_index = parent->IndexOfChild(dragged_node);
@@ -1776,40 +1781,40 @@ bool BookmarkBarView::CanDropAt(const BookmarkDragData& data,
 int BookmarkBarView::PerformDropImpl(const BookmarkDragData& data,
                                      BookmarkNode* parent_node,
                                      int index) {
-  BookmarkNode* dragged_node = data.GetNode(profile_);
+  BookmarkNode* dragged_node = data.GetFirstNode(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.is_url) {
+  } else if (data.has_single_url()) {
     // New URL, add it at the specified location.
-    std::wstring title = data.title;
+    std::wstring title = data.elements[0].title;
     if (title.empty()) {
       // No title, use the host.
-      title = UTF8ToWide(data.url.host());
+      title = UTF8ToWide(data.elements[0].url.host());
       if (title.empty())
         title = l10n_util::GetString(IDS_BOOMARK_BAR_UNKNOWN_DRAG_TITLE);
     }
-    model_->AddURL(parent_node, index, title, data.url);
+    model_->AddURL(parent_node, index, title, data.elements[0].url);
     return DragDropTypes::DRAG_COPY | DragDropTypes::DRAG_LINK;
   } else {
     // Dropping a group from different profile. Always accept.
-    CloneDragData(data, parent_node, index);
+    CloneDragData(data.elements[0], parent_node, index);
     return DragDropTypes::DRAG_COPY;
   }
 }
 
-void BookmarkBarView::CloneDragData(const BookmarkDragData& data,
+void BookmarkBarView::CloneDragData(const BookmarkDragData::Element& element,
                                     BookmarkNode* parent,
                                     int index_to_add_at) {
-  DCHECK(data.is_valid && model_);
-  if (data.is_url) {
-    model_->AddURL(parent, index_to_add_at, data.title, data.url);
+  DCHECK(model_);
+  if (element.is_url) {
+    model_->AddURL(parent, index_to_add_at, element.title, element.url);
   } else {
     BookmarkNode* new_folder = model_->AddGroup(parent, index_to_add_at,
-                                                data.title);
-    for (int i = 0; i < static_cast<int>(data.children.size()); ++i)
-      CloneDragData(data.children[i], new_folder, i);
+                                                element.title);
+    for (int i = 0; i < static_cast<int>(element.children.size()); ++i)
+      CloneDragData(element.children[i], new_folder, i);
   }
 }
 
diff --git a/chrome/browser/views/bookmark_bar_view.h b/chrome/browser/views/bookmark_bar_view.h
index 91eb3206..c5480ff 100644
--- a/chrome/browser/views/bookmark_bar_view.h
+++ b/chrome/browser/views/bookmark_bar_view.h
@@ -367,7 +367,7 @@ class BookmarkBarView : public views::View,
   // Creates a new group/entry for data, and recursively invokes itself for
   // all children of data. This is used during drag and drop to clone a
   // group from another profile.
-  void CloneDragData(const BookmarkDragData& data,
+  void CloneDragData(const BookmarkDragData::Element& data,
                      BookmarkNode* parent,
                      int index_to_add_at);