Moves bookmarks out of history into its own file (JSON).

Interesting points:
. Migration was a bit atypical. Here is the approach I took:
  . If the URL db contains bookmarks it writes the bookmarks to a
    temporary file.
  . When the bookmark bar model is loaded it assumes bookmarks are
    stored in a file. If the bookmarks file doesn't exist it then
    attempts to load from history, after waiting for history to finish
    processing tasks.
. I've broken having the omnibox query for starred only. This patch
  was already too ginormous for me to contemplate this too. I'll return
  to it after I land this.
. Similarly the history page isn't searching for starred titles
  now. As we discussed with Glen, that is probably fine for now.
. I've converted NOTIFY_STARRED_FAVICON_CHANGED to
  NOTIFY_FAVICON_CHANGED and it is notified ANY time a favicon
  changes. I'm mildly concerned about the extra notifications, but
  without having history know about starred it's the best I can do for
  now.
. Autocomplete (specifically URLDatabase::AutocompleteForPrefix)
  previously sorted by starred. It can no longer do this. I don't
  think I can get this functionality back:( Luckily it only mattered
  if you had a starred and non-starred URL with the same type count
  that matched a query. Probably pretty rare.

What's left:
. Fix up HistoryContentsProvider to query for starred entries titles.
. Clean up the delete all case. I basically just made it compile; it
  can be greatly simplified.
. Rename BookmarkBarModel to BookmarksModel.

BUG=1256202
TEST=this is a huge change to bookmarks. Thanfully it's pretty well
  covered by tests, none-the-less make sure you exercise bookmarks
  pretty heavily to make sure nothing is busted.


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

4 files changed, 28 insertions, 37 deletions

diff --git a/chrome/browser/autocomplete/history_contents_provider.cc b/chrome/browser/autocomplete/history_contents_provider.cc
index c68b17c..564f832 100644
--- a/chrome/browser/autocomplete/history_contents_provider.cc
+++ b/chrome/browser/autocomplete/history_contents_provider.cc
@@ -30,6 +30,7 @@
 #include "chrome/browser/autocomplete/history_contents_provider.h"
 
 #include "base/string_util.h"
+#include "chrome/browser/bookmark_bar_model.h"
 #include "chrome/browser/history/query_parser.h"
 #include "chrome/browser/profile.h"
 #include "net/base/net_util.h"
@@ -108,6 +109,7 @@ void HistoryContentsProvider::Start(const AutocompleteInput& input,
     return;
   }
 
+  // TODO(sky): this needs to query BookmarkBarModel for starred entries.
   if (!synchronous_only) {
     HistoryService* history = history_service_ ? history_service_ :
         profile_->GetHistoryService(Profile::EXPLICIT_ACCESS);
@@ -201,7 +203,8 @@ AutocompleteMatch HistoryContentsProvider::ResultToMatch(
   match.contents_class.push_back(
       ACMatchClassification(0, ACMatchClassification::URL));
   match.description = result.title();
-  match.starred = result.starred();
+  match.starred =
+      (profile_ && profile_->GetBookmarkBarModel()->IsBookmarked(result.url()));
 
   ClassifyDescription(result, &match);
   return match;
@@ -235,11 +238,13 @@ void HistoryContentsProvider::ClassifyDescription(
 int HistoryContentsProvider::CalculateRelevance(
     const history::URLResult& result) {
   bool in_title = !!result.title_match_positions().size();
+  bool is_starred =
+      (profile_ && profile_->GetBookmarkBarModel()->IsBookmarked(result.url()));
 
   switch (input_type_) {
     case AutocompleteInput::UNKNOWN:
     case AutocompleteInput::REQUESTED_URL:
-      if (result.starred()) {
+      if (is_starred) {
         return in_title ? 1000 + star_title_count_++ :
                           550 + star_contents_count_++;
       } else {
@@ -249,7 +254,7 @@ int HistoryContentsProvider::CalculateRelevance(
 
     case AutocompleteInput::QUERY:
     case AutocompleteInput::FORCED_QUERY:
-      if (result.starred()) {
+      if (is_starred) {
         return in_title ? 1200 + star_title_count_++ :
                           750 + star_contents_count_++;
       } else {
diff --git a/chrome/browser/autocomplete/history_url_provider.cc b/chrome/browser/autocomplete/history_url_provider.cc
index 1db8c00..d0ee441 100644
--- a/chrome/browser/autocomplete/history_url_provider.cc
+++ b/chrome/browser/autocomplete/history_url_provider.cc
@@ -48,6 +48,8 @@
 #include "googleurl/src/url_util.h"
 #include "net/base/net_util.h"
 
+// TODO(sky): this needs to check and update starred state.
+
 HistoryURLProviderParams::HistoryURLProviderParams(
     const AutocompleteInput& input,
     bool trim_http,
@@ -315,7 +317,6 @@ bool HistoryURLProvider::FixupExactSuggestion(history::URLDatabase* db,
       return false;
   } else {
     // We have data for this match, use it.
-    match.starred = info.starred();
     match.deletable = true;
     match.description = info.title();
     AutocompleteMatch::ClassifyMatchInString(params->input.text(),
@@ -439,10 +440,6 @@ bool HistoryURLProvider::CompareHistoryMatch(const HistoryMatch& a,
   if (a.url_info.typed_count() != b.url_info.typed_count())
     return a.url_info.typed_count() > b.url_info.typed_count();
 
-  // Starred pages are better than unstarred pages.
-  if (a.url_info.starred() != b.url_info.starred())
-    return a.url_info.starred();
-
   // For URLs that have each been typed once, a host (alone) is better than a
   // page inside.
   if (a.url_info.typed_count() == 1) {
@@ -851,6 +848,5 @@ AutocompleteMatch HistoryURLProvider::HistoryMatchToACMatch(
                                            ACMatchClassification::NONE,
                                            &match.description_class);
 
-  match.starred = history_match.url_info.starred();
   return match;
 }
diff --git a/chrome/browser/autocomplete/history_url_provider_unittest.cc b/chrome/browser/autocomplete/history_url_provider_unittest.cc
index 44486e7..9e70a38 100644
--- a/chrome/browser/autocomplete/history_url_provider_unittest.cc
+++ b/chrome/browser/autocomplete/history_url_provider_unittest.cc
@@ -31,7 +31,9 @@
 #include "base/message_loop.h"
 #include "base/path_service.h"
 #include "chrome/browser/autocomplete/history_url_provider.h"
+#include "chrome/browser/bookmark_bar_model.h"
 #include "chrome/browser/history/history.h"
+#include "chrome/test/testing_profile.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 struct TestURLInfo {
@@ -122,10 +124,10 @@ class HistoryURLProviderTest : public testing::Test,
                int num_results);
 
   ACMatches matches_;
-  scoped_refptr<HistoryService> history_service_;
+  scoped_ptr<TestingProfile> profile_;
+  HistoryService* history_service_;
 
  private:
-  std::wstring history_dir_;
   scoped_refptr<HistoryURLProvider> autocomplete_;
 };
 
@@ -135,32 +137,18 @@ void HistoryURLProviderTest::OnProviderUpdate(bool updated_matches) {
 }
 
 void HistoryURLProviderTest::SetUp() {
-  PathService::Get(base::DIR_TEMP, &history_dir_);
-  file_util::AppendToPath(&history_dir_, L"HistoryURLProviderTest");
-  file_util::Delete(history_dir_, true);  // Normally won't exist.
-  file_util::CreateDirectoryW(history_dir_);
+  profile_.reset(new TestingProfile());
+  profile_->CreateBookmarkBarModel();
+  profile_->CreateHistoryService(true);
+  history_service_ = profile_->GetHistoryService(Profile::EXPLICIT_ACCESS);
 
-  history_service_ = new HistoryService;
-  history_service_->Init(history_dir_);
-
-  autocomplete_ = new HistoryURLProvider(this, history_service_);
+  autocomplete_ = new HistoryURLProvider(this, profile_.get());
 
   FillData();
 }
 
 void HistoryURLProviderTest::TearDown() {
-  history_service_->SetOnBackendDestroyTask(new MessageLoop::QuitTask);
-  history_service_->Cleanup();
   autocomplete_ = NULL;
-  history_service_ = NULL;
-
-  // Wait for history thread to complete (the QuitTask will cause it to exit
-  // on destruction). Note: if this never terminates, somebody is probably
-  // leaking a reference to the history backend, so it never calls our
-  // destroy task.
-  MessageLoop::current()->Run();
-
-  file_util::Delete(history_dir_, true);
 }
 
 void HistoryURLProviderTest::FillData() {
@@ -180,11 +168,8 @@ void HistoryURLProviderTest::FillData() {
                                          cur.visit_count, cur.typed_count,
                                          visit_time, false);
     if (cur.starred) {
-      history::StarredEntry star_entry;
-      star_entry.type = history::StarredEntry::URL;
-      star_entry.parent_group_id = HistoryService::kBookmarkBarID;
-      star_entry.url = current_url;
-      history_service_->CreateStarredEntry(star_entry, NULL, NULL);
+      profile_->GetBookmarkBarModel()->SetURLStarred(
+          current_url, std::wstring(), true);
     }
   }
 }
@@ -288,7 +273,9 @@ TEST_F(HistoryURLProviderTest, PromoteShorterURLs) {
           arraysize(short_4));
 }
 
-TEST_F(HistoryURLProviderTest, Starred) {
+// Bookmarks have been moved out of the history db, resulting in this no longer
+// working. See TODO in URLDatabase::AutocompleteForPrefix.
+TEST_F(HistoryURLProviderTest, DISABLED_Starred) {
   // Test that starred pages sort properly.
   const std::wstring star_1[] = {
     L"http://startest/",
diff --git a/chrome/browser/autocomplete/search_provider.cc b/chrome/browser/autocomplete/search_provider.cc
index fbc24b5..6f66694 100644
--- a/chrome/browser/autocomplete/search_provider.cc
+++ b/chrome/browser/autocomplete/search_provider.cc
@@ -31,6 +31,7 @@
 
 #include "base/message_loop.h"
 #include "base/string_util.h"
+#include "chrome/browser/bookmark_bar_model.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/google_util.h"
 #include "chrome/browser/profile.h"
@@ -238,7 +239,9 @@ void SearchProvider::OnQueryURLComplete(HistoryService::Handle handle,
                                         bool success,
                                         const history::URLRow* url_row,
                                         history::VisitVector* unused) {
-  bool is_starred = success ? url_row->starred() : false;
+  bool is_starred =
+      (success && profile_ &&
+       profile_->GetBookmarkBarModel()->IsBookmarked(url_row->url()));
   star_requests_pending_ = false;
   // We can't just use star_request_consumer_.HasPendingRequests() here;
   // see comment in ConvertResultsToAutocompleteMatches().