- Made BookmarkService::GetBookmarks return both urls and title,

  it still return the unique URLs, not matter what the title is.

BUG=http://b/6696843

TEST=AndroidProviderBackendTest, BookmarkModelTest, BookmarkModelSQLHandlerTest

Review URL: https://chromiumcodereview.appspot.com/10825147

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

chrome/browser/bookmarks/bookmark_model.cc

@@ -400,15 +400,20 @@ bool BookmarkModel::IsBookmarked(const GURL& url) {
   return IsBookmarkedNoLock(url);
 }
 
-void BookmarkModel::GetBookmarks(std::vector<GURL>* urls) {
+void BookmarkModel::GetBookmarks(
+    std::vector<BookmarkService::URLAndTitle>* bookmarks) {
   base::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) {
     const GURL* url = &((*i)->url());
     // Only add unique URLs.
-    if (!last_url || *url != *last_url)
-      urls->push_back(*url);
+    if (!last_url || *url != *last_url) {
+      BookmarkService::URLAndTitle bookmark;
+      bookmark.url = *url;
+      bookmark.title = (*i)->GetTitle();
+      bookmarks->push_back(bookmark);
+    }
     last_url = url;
   }
 }

chrome/browser/bookmarks/bookmark_model.h

@@ -292,10 +292,11 @@ class BookmarkModel : public content::NotificationObserver,
   // See BookmarkService for more details on this.
   virtual bool IsBookmarked(const GURL& url) OVERRIDE;
 
-  // Returns all the bookmarked urls.
+  // Returns all the bookmarked urls and their titles.
   // This method is thread safe.
   // See BookmarkService for more details on this.
-  virtual void GetBookmarks(std::vector<GURL>* urls) OVERRIDE;
+  virtual void GetBookmarks(
+      std::vector<BookmarkService::URLAndTitle>* urls) OVERRIDE;
 
   // Blocks until loaded; this is NOT invoked on the main thread.
   // See BookmarkService for more details on this.

chrome/browser/bookmarks/bookmark_model_unittest.cc

@@ -614,13 +614,21 @@ TEST_F(BookmarkModelTest, GetMostRecentlyAddedNodeForURL) {
 // Makes sure GetBookmarks removes duplicates.
 TEST_F(BookmarkModelTest, GetBookmarksWithDups) {
   const GURL url("http://foo.com/0");
-  model_.AddURL(model_.bookmark_bar_node(), 0, ASCIIToUTF16("blah"), url);
-  model_.AddURL(model_.bookmark_bar_node(), 1, ASCIIToUTF16("blah"), url);
-
-  std::vector<GURL> urls;
-  model_.GetBookmarks(&urls);
-  EXPECT_EQ(1U, urls.size());
-  ASSERT_TRUE(urls[0] == url);
+  const string16 title(ASCIIToUTF16("blah"));
+  model_.AddURL(model_.bookmark_bar_node(), 0, title, url);
+  model_.AddURL(model_.bookmark_bar_node(), 1, title, url);
+
+  std::vector<BookmarkService::URLAndTitle> bookmarks;
+  model_.GetBookmarks(&bookmarks);
+  ASSERT_EQ(1U, bookmarks.size());
+  EXPECT_EQ(url, bookmarks[0].url);
+  EXPECT_EQ(title, bookmarks[0].title);
+
+  model_.AddURL(model_.bookmark_bar_node(), 2, ASCIIToUTF16("Title2"), url);
+  // Only one returned, even titles are different.
+  bookmarks.clear();
+  model_.GetBookmarks(&bookmarks);
+  EXPECT_EQ(1U, bookmarks.size());
 }
 
 TEST_F(BookmarkModelTest, HasBookmarks) {

chrome/browser/bookmarks/bookmark_service.h

-// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
@@ -7,7 +7,8 @@
 
 #include <vector>
 
-class GURL;
+#include "base/string16.h"
+#include "googleurl/src/gurl.h"
 
 // BookmarkService provides a thread safe view of bookmarks. It is used by
 // HistoryBackend when it needs to determine the set of bookmarked URLs
@@ -16,17 +17,23 @@ class GURL;
 // BookmarkService is owned by Profile and deleted when the Profile is deleted.
 class BookmarkService {
  public:
+  struct URLAndTitle {
+    GURL url;
+    string16 title;
+  };
+
   // Returns true if the specified URL is bookmarked.
   //
   // 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. This returns
-  // the unique set of URLs. For example, if two bookmarks reference the same
-  // URL only one entry is added.
+  // Returns, by reference in |bookmarks|, the set of bookmarked urls and their
+  // titles. This returns the unique set of URLs. For example, if two bookmarks
+  // reference the same URL only one entry is added not matter the titles are
+  // same or not.
   //
   // If not on the main thread you *must* invoke BlockTillLoaded first.
-  virtual void GetBookmarks(std::vector<GURL>* urls) = 0;
+  virtual void GetBookmarks(std::vector<URLAndTitle>* bookmarks) = 0;
 
   // Blocks until loaded. This is intended for usage on a thread other than
   // the main thread.

chrome/browser/history/android/android_provider_backend.cc

@@ -749,16 +749,16 @@ bool AndroidProviderBackend::UpdateBookmarks() {
   }
 
   bookmark_service_->BlockTillLoaded();
-  std::vector<GURL> bookmark_urls;
-  bookmark_service_->GetBookmarks(&bookmark_urls);
+  std::vector<BookmarkService::URLAndTitle> bookmarks;
+  bookmark_service_->GetBookmarks(&bookmarks);
 
-  if (bookmark_urls.empty())
+  if (bookmarks.empty())
     return true;
 
   std::vector<URLID> url_ids;
-  for (std::vector<GURL>::const_iterator i = bookmark_urls.begin();
-      i != bookmark_urls.end(); ++i) {
-    URLID url_id = history_db_->GetRowForURL(*i, NULL);
+  for (std::vector<BookmarkService::URLAndTitle>::const_iterator i =
+           bookmarks.begin(); i != bookmarks.end(); ++i) {
+    URLID url_id = history_db_->GetRowForURL(i->url, NULL);
     if (url_id == 0)
       // TODO(michaelbai): Add a row to url and android_url table as the
       // bookmark could be added manually by user or insertted by sync.

chrome/browser/history/android/bookmark_model_sql_handler_unittest.cc

@@ -138,10 +138,12 @@ TEST_F(BookmarkModelSQLHandlerTest, UpdateHistoryToBookmark) {
   ASSERT_TRUE(handler.Update(row, id_rows));
   RunMessageLoopForUI();
   // Get all bookmarks and verify there is only one.
-  std::vector<GURL> urls;
-  bookmark_model_->GetBookmarks(&urls);
-  EXPECT_EQ(1u, urls.size());
-  EXPECT_EQ(url_row.url(), urls[0]);
+  std::vector<BookmarkService::URLAndTitle> bookmarks;
+  bookmark_model_->GetBookmarks(&bookmarks);
+  ASSERT_EQ(1u, bookmarks.size());
+  EXPECT_EQ(url_row.url(), bookmarks[0].url);
+  EXPECT_EQ(url_row.title(), bookmarks[0].title);
+
   // Get the bookmark node.
   std::vector<const BookmarkNode*> nodes;
   bookmark_model_->GetNodesByURL(row.url(), &nodes);
@@ -155,9 +157,9 @@ TEST_F(BookmarkModelSQLHandlerTest, UpdateHistoryToBookmark) {
   row.set_is_bookmark(false);
   ASSERT_TRUE(handler.Update(row, id_rows));
   RunMessageLoopForUI();
-  urls.clear();
-  bookmark_model_->GetBookmarks(&urls);
-  EXPECT_TRUE(urls.empty());
+  bookmarks.clear();
+  bookmark_model_->GetBookmarks(&bookmarks);
+  EXPECT_TRUE(bookmarks.empty());
 
   // Update with the parent id.
   row.set_parent_id(bookmark_model_->other_node()->id());
@@ -165,10 +167,11 @@ TEST_F(BookmarkModelSQLHandlerTest, UpdateHistoryToBookmark) {
   ASSERT_TRUE(handler.Update(row, id_rows));
   RunMessageLoopForUI();
   // Get all bookmarks and verify there is only one.
-  urls.clear();
-  bookmark_model_->GetBookmarks(&urls);
-  EXPECT_EQ(1u, urls.size());
-  EXPECT_EQ(url_row.url(), urls[0]);
+  bookmarks.clear();
+  bookmark_model_->GetBookmarks(&bookmarks);
+  ASSERT_EQ(1u, bookmarks.size());
+  EXPECT_EQ(url_row.url(), bookmarks[0].url);
+  EXPECT_EQ(url_row.title(), bookmarks[0].title);
   // Get the bookmark node.
   nodes.clear();
   bookmark_model_->GetNodesByURL(row.url(), &nodes);
@@ -186,10 +189,11 @@ TEST_F(BookmarkModelSQLHandlerTest, UpdateHistoryToBookmark) {
   ASSERT_TRUE(handler.Update(update_title, id_rows));
   RunMessageLoopForUI();
   // Get all bookmarks and verify there is only one.
-  urls.clear();
-  bookmark_model_->GetBookmarks(&urls);
-  EXPECT_EQ(1u, urls.size());
-  EXPECT_EQ(url_row.url(), urls[0]);
+  bookmarks.clear();
+  bookmark_model_->GetBookmarks(&bookmarks);
+  ASSERT_EQ(1u, bookmarks.size());
+  EXPECT_EQ(url_row.url(), bookmarks[0].url);
+  EXPECT_EQ(url_row.title(), bookmarks[0].title);
   // Get the bookmark node.
   nodes.clear();
   bookmark_model_->GetNodesByURL(row.url(), &nodes);

chrome/browser/history/history_backend.cc

@@ -2257,7 +2257,7 @@ void HistoryBackend::DeleteAllHistory() {
   // the original tables directly.
 
   // Get the bookmarked URLs.
-  std::vector<GURL> starred_urls;
+  std::vector<BookmarkService::URLAndTitle> starred_urls;
   BookmarkService* bookmark_service = GetBookmarkService();
   if (bookmark_service)
     bookmark_service_->GetBookmarks(&starred_urls);
@@ -2265,7 +2265,7 @@ void HistoryBackend::DeleteAllHistory() {
   URLRows kept_urls;
   for (size_t i = 0; i < starred_urls.size(); i++) {
     URLRow row;
-    if (!db_->GetRowForURL(starred_urls[i], &row))
+    if (!db_->GetRowForURL(starred_urls[i].url, &row))
       continue;
 
     // Clear the last visit time so when we write these rows they are "clean."