Remove code obsoleted by the removal of StarredURLDatabase.

Second attempt - first one caused failures in sync tests that assumed History was initialized if Bookmarks was.

First patch is original CL (https://chromiumcodereview.appspot.com/10874027).

BUG=144050
TEST=sync_integration_tests


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

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

8 files changed, 21 insertions, 233 deletions

diff --git a/chrome/browser/bookmarks/bookmark_storage.cc b/chrome/browser/bookmarks/bookmark_storage.cc
index 3166088..03fc870 100644
--- a/chrome/browser/bookmarks/bookmark_storage.cc
+++ b/chrome/browser/bookmarks/bookmark_storage.cc
@@ -6,6 +6,7 @@
 
 #include "base/bind.h"
 #include "base/compiler_specific.h"
+#include "base/file_path.h"
 #include "base/file_util.h"
 #include "base/file_util_proxy.h"
 #include "base/json/json_file_value_serializer.h"
@@ -17,9 +18,7 @@
 #include "chrome/browser/history/history_service_factory.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/common/chrome_constants.h"
-#include "chrome/common/chrome_notification_types.h"
 #include "content/public/browser/browser_thread.h"
-#include "content/public/browser/notification_source.h"
 
 using base::TimeTicks;
 using content::BrowserThread;
@@ -83,8 +82,7 @@ void LoadCallback(const FilePath& path,
 
   BrowserThread::PostTask(
       BrowserThread::UI, FROM_HERE,
-      base::Bind(&BookmarkStorage::OnLoadFinished, storage,
-                 bookmark_file_exists, path));
+      base::Bind(&BookmarkStorage::OnLoadFinished, storage));
 }
 
 }  // namespace
@@ -111,12 +109,10 @@ BookmarkLoadDetails::~BookmarkLoadDetails() {
 // BookmarkStorage -------------------------------------------------------------
 
 BookmarkStorage::BookmarkStorage(Profile* profile, BookmarkModel* model)
-    : profile_(profile),
-      model_(model),
+    : model_(model),
       writer_(profile->GetPath().Append(chrome::kBookmarksFileName),
-              BrowserThread::GetMessageLoopProxyForThread(BrowserThread::FILE)),
-      tmp_history_path_(
-          profile->GetPath().Append(chrome::kHistoryBookmarksFileName)) {
+              BrowserThread::GetMessageLoopProxyForThread(
+                  BrowserThread::FILE)) {
   writer_.set_commit_interval(base::TimeDelta::FromMilliseconds(kSaveDelayMS));
   BrowserThread::PostTask(BrowserThread::FILE, FROM_HERE,
                           base::Bind(&BackupCallback, writer_.path()));
@@ -131,43 +127,8 @@ void BookmarkStorage::LoadBookmarks(BookmarkLoadDetails* details) {
   DCHECK(!details_.get());
   DCHECK(details);
   details_.reset(details);
-  DoLoadBookmarks(writer_.path());
-}
-
-void BookmarkStorage::DoLoadBookmarks(const FilePath& path) {
-  BrowserThread::PostTask(
-      BrowserThread::FILE, FROM_HERE,
-      base::Bind(&LoadCallback, path, make_scoped_refptr(this),
-                 details_.get()));
-}
-
-void BookmarkStorage::MigrateFromHistory() {
-  // We need to wait until history has finished loading before reading
-  // from generated bookmarks file.
-  HistoryService* history =
-      HistoryServiceFactory::GetForProfile(profile_, Profile::EXPLICIT_ACCESS);
-  if (!history) {
-    // This happens in unit tests.
-    if (model_)
-      model_->DoneLoading(details_.release());
-    return;
-  }
-  if (!history->BackendLoaded()) {
-    // The backend isn't finished loading. Wait for it.
-    notification_registrar_.Add(this, chrome::NOTIFICATION_HISTORY_LOADED,
-                                content::Source<Profile>(profile_));
-  } else {
-    DoLoadBookmarks(tmp_history_path_);
-  }
-}
-
-void BookmarkStorage::OnHistoryFinishedWriting() {
-  notification_registrar_.Remove(this, chrome::NOTIFICATION_HISTORY_LOADED,
-                                 content::Source<Profile>(profile_));
-
-  // This is used when migrating bookmarks data from database to file.
-  // History wrote the file for us, and now we want to load data from it.
-  DoLoadBookmarks(tmp_history_path_);
+  BrowserThread::PostTask(BrowserThread::FILE, FROM_HERE, base::Bind(
+      &LoadCallback, writer_.path(), make_scoped_refptr(this), details_.get()));
 }
 
 void BookmarkStorage::ScheduleSave() {
@@ -190,47 +151,11 @@ bool BookmarkStorage::SerializeData(std::string* output) {
   return serializer.Serialize(*(value.get()));
 }
 
-void BookmarkStorage::OnLoadFinished(bool file_exists, const FilePath& path) {
-  if (path == writer_.path() && !file_exists) {
-    // The file doesn't exist. This means one of two things:
-    // 1. A clean profile.
-    // 2. The user is migrating from an older version where bookmarks were
-    //    saved in history.
-    // We assume step 2. If history had the bookmarks, it will write the
-    // bookmarks to a file for us.
-    MigrateFromHistory();
-    return;
-  }
-
+void BookmarkStorage::OnLoadFinished() {
   if (!model_)
     return;
 
   model_->DoneLoading(details_.release());
-
-  if (path == tmp_history_path_) {
-    // We just finished migration from history. Save now to new file,
-    // after the model is created and done loading.
-    SaveNow();
-
-    // Clean up after migration from history.
-    base::FileUtilProxy::Delete(
-        BrowserThread::GetMessageLoopProxyForThread(BrowserThread::FILE),
-        tmp_history_path_, false, base::FileUtilProxy::StatusCallback());
-  }
-}
-
-void BookmarkStorage::Observe(int type,
-                              const content::NotificationSource& source,
-                              const content::NotificationDetails& details) {
-  switch (type) {
-    case chrome::NOTIFICATION_HISTORY_LOADED:
-      OnHistoryFinishedWriting();
-      break;
-
-    default:
-      NOTREACHED();
-      break;
-  }
 }
 
 bool BookmarkStorage::SaveNow() {
diff --git a/chrome/browser/bookmarks/bookmark_storage.h b/chrome/browser/bookmarks/bookmark_storage.h
index 73aa9c8..6bb7d97 100644
--- a/chrome/browser/bookmarks/bookmark_storage.h
+++ b/chrome/browser/bookmarks/bookmark_storage.h
@@ -5,13 +5,10 @@
 #ifndef CHROME_BROWSER_BOOKMARKS_BOOKMARK_STORAGE_H_
 #define CHROME_BROWSER_BOOKMARKS_BOOKMARK_STORAGE_H_
 
-#include "base/file_path.h"
 #include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
 #include "chrome/browser/bookmarks/bookmark_index.h"
 #include "chrome/common/important_file_writer.h"
-#include "content/public/browser/notification_observer.h"
-#include "content/public/browser/notification_registrar.h"
 
 class BookmarkModel;
 class BookmarkPermanentNode;
@@ -92,8 +89,7 @@ class BookmarkLoadDetails {
 // as notifying the BookmarkStorage every time the model changes.
 //
 // Internally BookmarkStorage uses BookmarkCodec to do the actual read/write.
-class BookmarkStorage : public content::NotificationObserver,
-                        public ImportantFileWriter::DataSerializer,
+class BookmarkStorage : public ImportantFileWriter::DataSerializer,
                         public base::RefCountedThreadSafe<BookmarkStorage> {
  public:
   // Creates a BookmarkStorage for the specified model
@@ -110,11 +106,8 @@ class BookmarkStorage : public content::NotificationObserver,
   // a pending save, it is saved immediately.
   void BookmarkModelDeleted();
 
-  // Callback from backend with the results of the bookmark file. This may be
-  // called multiple times, with different paths. This happens when we migrate
-  // bookmark data from database.
-  void OnLoadFinished(bool file_exists,
-                      const FilePath& path);
+  // Callback from backend after loading the bookmark file.
+  void OnLoadFinished();
 
   // ImportantFileWriter::DataSerializer implementation.
   virtual bool SerializeData(std::string* output) OVERRIDE;
@@ -124,44 +117,16 @@ class BookmarkStorage : public content::NotificationObserver,
 
   virtual ~BookmarkStorage();
 
-  // Loads bookmark data from |file| and notifies the model when finished.
-  void DoLoadBookmarks(const FilePath& file);
-
-  // Load bookmarks data from the file written by history (StarredURLDatabase).
-  void MigrateFromHistory();
-
-  // Called when history has written the file with bookmarks data. Loads data
-  // from that file.
-  void OnHistoryFinishedWriting();
-
-  // Called after we loaded file generated by history. Saves the data, deletes
-  // the temporary file, and notifies the model.
-  void FinishHistoryMigration();
-
-  // content::NotificationObserver
-  virtual void Observe(int type,
-                       const content::NotificationSource& source,
-                       const content::NotificationDetails& details) OVERRIDE;
-
   // Serializes the data and schedules save using ImportantFileWriter.
   // Returns true on successful serialization.
   bool SaveNow();
 
-  // Keep the pointer to profile, we may need it for migration from history.
-  Profile* profile_;
-
   // The model. The model is NULL once BookmarkModelDeleted has been invoked.
   BookmarkModel* model_;
 
   // Helper to write bookmark data safely.
   ImportantFileWriter writer_;
 
-  // Helper to ensure that we unregister from notifications on destruction.
-  content::NotificationRegistrar notification_registrar_;
-
-  // Path to temporary file created during migrating bookmarks from history.
-  const FilePath tmp_history_path_;
-
   // See class description of BookmarkLoadDetails for details on this.
   scoped_ptr<BookmarkLoadDetails> details_;
 
diff --git a/chrome/browser/history/history_database.h b/chrome/browser/history/history_database.h
index 0d46459..23c73ca 100644
--- a/chrome/browser/history/history_database.h
+++ b/chrome/browser/history/history_database.h
@@ -64,9 +64,9 @@ class HistoryDatabase : public DownloadDatabase,
 
   virtual ~HistoryDatabase();
 
-  // Must call this function to complete initialization. Will return true on
-  // success. On false, no other function should be called. You may want to call
-  // BeginExclusiveMode after this when you are ready.
+  // Must call this function to complete initialization. Will return
+  // sql::INIT_OK on success. Otherwise, no other function should be called. You
+  // may want to call BeginExclusiveMode after this when you are ready.
   sql::InitStatus Init(const FilePath& history_name);
 
   // Call to set the mode on the database to exclusive. The default locking mode
@@ -166,9 +166,8 @@ class HistoryDatabase : public DownloadDatabase,
   // Migration -----------------------------------------------------------------
 
   // Makes sure the version is up-to-date, updating if necessary. If the
-  // database is too old to migrate, the user will be notified. In this case, or
-  // for other errors, false will be returned. True means it is up-to-date and
-  // ready for use.
+  // database is too old to migrate, the user will be notified. Returns
+  // sql::INIT_OK iff  the DB is up-to-date and ready for use.
   //
   // This assumes it is called from the init function inside a transaction. It
   // may commit the transaction and start a new one if migration requires it.
diff --git a/chrome/browser/history/history_types.cc b/chrome/browser/history/history_types.cc
index 61aecc3..00cafa3 100644
--- a/chrome/browser/history/history_types.cc
+++ b/chrome/browser/history/history_types.cc
@@ -90,33 +90,6 @@ VisitRow::VisitRow(URLID arg_url_id,
 VisitRow::~VisitRow() {
 }
 
-// StarredEntry ----------------------------------------------------------------
-
-StarredEntry::StarredEntry()
-    : id(0),
-      parent_folder_id(0),
-      folder_id(0),
-      visual_order(0),
-      type(URL),
-      url_id(0) {
-}
-
-StarredEntry::~StarredEntry() {
-}
-
-void StarredEntry::Swap(StarredEntry* other) {
-  std::swap(id, other->id);
-  title.swap(other->title);
-  std::swap(date_added, other->date_added);
-  std::swap(parent_folder_id, other->parent_folder_id);
-  std::swap(folder_id, other->folder_id);
-  std::swap(visual_order, other->visual_order);
-  std::swap(type, other->type);
-  url.Swap(&other->url);
-  std::swap(url_id, other->url_id);
-  std::swap(date_folder_modified, other->date_folder_modified);
-}
-
 // URLResult -------------------------------------------------------------------
 
 URLResult::URLResult() {
diff --git a/chrome/browser/history/history_types.h b/chrome/browser/history/history_types.h
index 326137f..38dcf80 100644
--- a/chrome/browser/history/history_types.h
+++ b/chrome/browser/history/history_types.h
@@ -40,8 +40,6 @@ typedef std::map<GURL, scoped_refptr<RefCountedVector<GURL> > > RedirectMap;
 // Container for a list of URLs.
 typedef std::vector<GURL> RedirectList;
 
-typedef int64 StarID;  // Unique identifier for star entries.
-typedef int64 UIStarID;  // Identifier for star entries that come from the UI.
 typedef int64 DownloadID;   // Identifier for a download.
 typedef int64 FaviconID;  // For favicons.
 typedef int64 FaviconBitmapID; // Identifier for a bitmap in a favicon.
@@ -285,79 +283,6 @@ struct PageVisit {
   base::Time visit_time;
 };
 
-// StarredEntry ---------------------------------------------------------------
-
-// StarredEntry represents either a starred page, or a folder (where a folder
-// consists of child starred entries). Use the type to determine the type of a
-// particular entry.
-//
-// The database internally uses the id field to uniquely identify a starred
-// entry. On the other hand, the UI, which is anything routed through
-// HistoryService and HistoryBackend (including BookmarkBarView), uses the
-// url field to uniquely identify starred entries of type URL and the folder_id
-// field to uniquely identify starred entries of type USER_FOLDER. For example,
-// HistoryService::UpdateStarredEntry identifies the entry by url (if the
-// type is URL) or folder_id (if the type is not URL).
-struct StarredEntry {
-  enum Type {
-    // Type represents a starred URL.
-    URL,
-
-    // The bookmark bar folder.
-    BOOKMARK_BAR,
-
-    // User created folder.
-    USER_FOLDER,
-
-    // The "other bookmarks" folder that holds uncategorized bookmarks.
-    OTHER,
-
-    // The mobile folder.
-    MOBILE,
-  };
-
-  StarredEntry();
-  ~StarredEntry();
-
-  void Swap(StarredEntry* other);
-
-  // Unique identifier of this entry.
-  StarID id;
-
-  // Title.
-  string16 title;
-
-  // When this was added.
-  base::Time date_added;
-
-  // Folder ID of the folder this entry is in. If 0, this entry is not in a
-  // folder.
-  UIStarID parent_folder_id;
-
-  // Unique identifier for folders. This is assigned by the UI.
-  //
-  // WARNING: this is NOT the same as id, id is assigned by the database,
-  // this is assigned by the UI. See note about StarredEntry for more info.
-  UIStarID folder_id;
-
-  // Visual order within the parent. Only valid if folder_id is not 0.
-  int visual_order;
-
-  // Type of this entry (see enum).
-  Type type;
-
-  // If type == URL, this is the URL of the page that was starred.
-  GURL url;
-
-  // If type == URL, this is the ID of the URL of the primary page that was
-  // starred.
-  URLID url_id;
-
-  // Time the entry was last modified. This is only used for folders and
-  // indicates the last time a URL was added as a child to the folder.
-  base::Time date_folder_modified;
-};
-
 // URLResult -------------------------------------------------------------------
 
 class URLResult : public URLRow {
diff --git a/chrome/browser/sync/test/integration/sync_test.cc b/chrome/browser/sync/test/integration/sync_test.cc
index bc28f73..4f9f578 100644
--- a/chrome/browser/sync/test/integration/sync_test.cc
+++ b/chrome/browser/sync/test/integration/sync_test.cc
@@ -20,6 +20,7 @@
 #include "base/values.h"
 #include "chrome/browser/bookmarks/bookmark_model_factory.h"
 #include "chrome/browser/google/google_url_tracker.h"
+#include "chrome/browser/history/history_service_factory.h"
 #include "chrome/browser/lifetime/application_lifetime.h"
 #include "chrome/browser/password_manager/encryptor.h"
 #include "chrome/browser/profiles/profile.h"
@@ -280,6 +281,8 @@ bool SyncTest::SetupClients() {
   verifier_ = MakeProfile(FILE_PATH_LITERAL("Verifier"));
   ui_test_utils::WaitForBookmarkModelToLoad(
       BookmarkModelFactory::GetForProfile(verifier()));
+  ui_test_utils::WaitForHistoryToLoad(HistoryServiceFactory::GetForProfile(
+      verifier(), Profile::EXPLICIT_ACCESS));
   ui_test_utils::WaitForTemplateURLServiceToLoad(
       TemplateURLServiceFactory::GetForProfile(verifier()));
   return (verifier_ != NULL);
@@ -303,7 +306,8 @@ void SyncTest::InitializeInstance(int index) {
 
   ui_test_utils::WaitForBookmarkModelToLoad(
       BookmarkModelFactory::GetForProfile(GetProfile(index)));
-
+  ui_test_utils::WaitForHistoryToLoad(HistoryServiceFactory::GetForProfile(
+      GetProfile(index), Profile::EXPLICIT_ACCESS));
   ui_test_utils::WaitForTemplateURLServiceToLoad(
       TemplateURLServiceFactory::GetForProfile(GetProfile(index)));
 }
diff --git a/chrome/common/chrome_constants.cc b/chrome/common/chrome_constants.cc
index 0435e55..9abe9b8 100644
--- a/chrome/common/chrome_constants.cc
+++ b/chrome/common/chrome_constants.cc
@@ -136,8 +136,6 @@ const FilePath::CharType kCustomDictionaryFileName[] =
     FPL("Custom Dictionary.txt");
 const FilePath::CharType kExtensionsCookieFilename[] = FPL("Extension Cookies");
 const FilePath::CharType kFaviconsFilename[] = FPL("Favicons");
-const FilePath::CharType kHistoryBookmarksFileName[] =
-    FPL("Bookmarks From History");
 const FilePath::CharType kHistoryFilename[] = FPL("History");
 const FilePath::CharType kJumpListIconDirname[] = FPL("JumpListIcons");
 const FilePath::CharType kLocalStateFilename[] = FPL("Local State");
diff --git a/chrome/common/chrome_constants.h b/chrome/common/chrome_constants.h
index 70f6ee9..cdc07e4 100644
--- a/chrome/common/chrome_constants.h
+++ b/chrome/common/chrome_constants.h
@@ -63,7 +63,6 @@ extern const FilePath::CharType kCRLSetFilename[];
 extern const FilePath::CharType kCustomDictionaryFileName[];
 extern const FilePath::CharType kExtensionsCookieFilename[];
 extern const FilePath::CharType kFaviconsFilename[];
-extern const FilePath::CharType kHistoryBookmarksFileName[];
 extern const FilePath::CharType kHistoryFilename[];
 extern const FilePath::CharType kJumpListIconDirname[];
 extern const FilePath::CharType kLocalStateFilename[];