Makes deleting history no longer delete starred urls. Thiseffectively reenables the code in ExpireHistoryBackend. I also madethe code consistent so that when we delete visits as the result ofhistory deletion we don't change the typed/visit count of theunderlying url.BUG=1214201 1256202TEST=none

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

28 files changed, 1029 insertions, 478 deletions

diff --git a/chrome/browser/autocomplete/history_contents_provider_unittest.cc b/chrome/browser/autocomplete/history_contents_provider_unittest.cc
index ba7985a..2e1a646 100644
--- a/chrome/browser/autocomplete/history_contents_provider_unittest.cc
+++ b/chrome/browser/autocomplete/history_contents_provider_unittest.cc
@@ -54,7 +54,7 @@ class HistoryContentsProviderTest : public testing::Test,
     file_util::CreateDirectoryW(history_dir_);
 
     history_service_ = new HistoryService;
-    history_service_->Init(history_dir_);
+    history_service_->Init(history_dir_, NULL);
 
     // Populate history.
     for (int i = 0; i < arraysize(test_entries); i++) {
diff --git a/chrome/browser/autocomplete/history_url_provider_unittest.cc b/chrome/browser/autocomplete/history_url_provider_unittest.cc
index 05a696d..6dd80e9 100644
--- a/chrome/browser/autocomplete/history_url_provider_unittest.cc
+++ b/chrome/browser/autocomplete/history_url_provider_unittest.cc
@@ -114,7 +114,7 @@ void HistoryURLProviderTest::OnProviderUpdate(bool updated_matches) {
 
 void HistoryURLProviderTest::SetUp() {
   profile_.reset(new TestingProfile());
-  profile_->CreateBookmarkBarModel();
+  profile_->CreateBookmarkBarModel(true);
   profile_->CreateHistoryService(true);
   history_service_ = profile_->GetHistoryService(Profile::EXPLICIT_ACCESS);
 
diff --git a/chrome/browser/bookmark_bar_context_menu_controller_test.cc b/chrome/browser/bookmark_bar_context_menu_controller_test.cc
index 4c2cfa4..d2aac6f 100644
--- a/chrome/browser/bookmark_bar_context_menu_controller_test.cc
+++ b/chrome/browser/bookmark_bar_context_menu_controller_test.cc
@@ -39,7 +39,7 @@ class BookmarkBarContextMenuControllerTest : public testing::Test {
 
     profile_.reset(new TestingProfile());
     profile_->set_has_history_service(true);
-    profile_->CreateBookmarkBarModel();
+    profile_->CreateBookmarkBarModel(true);
 
     model_ = profile_->GetBookmarkBarModel();
 
diff --git a/chrome/browser/bookmark_bar_model.cc b/chrome/browser/bookmark_bar_model.cc
index 3aa6e26..b86131d 100644
--- a/chrome/browser/bookmark_bar_model.cc
+++ b/chrome/browser/bookmark_bar_model.cc
@@ -73,7 +73,9 @@ BookmarkBarModel::BookmarkBarModel(Profile* profile)
 #pragma warning(suppress: 4355)  // Okay to pass "this" here.
       root_(this, GURL()),
       bookmark_bar_node_(NULL),
-      other_node_(NULL) {
+      other_node_(NULL),
+      waiting_for_history_load_(false),
+      loaded_signal_(CreateEvent(NULL, TRUE, FALSE, NULL)) {
   // Create the bookmark bar and other bookmarks folders. These always exist.
   CreateBookmarkBarNode();
   CreateOtherBookmarksNode();
@@ -87,26 +89,21 @@ BookmarkBarModel::BookmarkBarModel(Profile* profile)
 
   if (!profile_) {
     // Profile is null during testing.
-    loaded_ = true;
-    return;
+    DoneLoading();
   }
-
-  // Listen for changes to starred icons so that we can update the favicon of
-  // the node appropriately.
-  NotificationService::current()->AddObserver(
-      this, NOTIFY_FAVICON_CHANGED, Source<Profile>(profile_));
-
-  // Load the bookmarks. BookmarkStorage notifies us when done.
-  store_ = new BookmarkStorage(profile_, this);
-  store_->LoadBookmarks(false);
 }
 
 BookmarkBarModel::~BookmarkBarModel() {
-  if (profile_) {
+  if (profile_ && store_.get()) {
     NotificationService::current()->RemoveObserver(
         this, NOTIFY_FAVICON_CHANGED, Source<Profile>(profile_));
   }
 
+  if (waiting_for_history_load_) {
+    NotificationService::current()->RemoveObserver(
+        this, NOTIFY_HISTORY_LOADED, Source<Profile>(profile_));
+  }
+
   if (store_) {
     // The store maintains a reference back to us. We need to tell it we're gone
     // so that it doesn't try and invoke a method back on us again.
@@ -114,6 +111,24 @@ BookmarkBarModel::~BookmarkBarModel() {
   }
 }
 
+void BookmarkBarModel::Load() {
+  if (store_.get()) {
+    // If the store is non-null, it means Load was already invoked. Load should
+    // only be invoked once.
+    NOTREACHED();
+    return;
+  }
+
+  // Listen for changes to favicons so that we can update the favicon of the
+  // node appropriately.
+  NotificationService::current()->AddObserver(
+      this, NOTIFY_FAVICON_CHANGED, Source<Profile>(profile_));
+
+  // Load the bookmarks. BookmarkStorage notifies us when done.
+  store_ = new BookmarkStorage(profile_, this);
+  store_->LoadBookmarks(false);
+}
+
 BookmarkBarNode* BookmarkBarModel::GetParentForNewNodes() {
   std::vector<BookmarkBarNode*> nodes;
 
@@ -142,6 +157,7 @@ std::vector<BookmarkBarNode*> BookmarkBarModel::GetMostRecentlyModifiedGroups(
 void BookmarkBarModel::GetMostRecentlyAddedEntries(
     size_t count,
     std::vector<BookmarkBarNode*>* nodes) {
+  AutoLock url_lock(url_lock_);
   for (NodesOrderedByURLSet::iterator i = nodes_ordered_by_url_set_.begin();
        i != nodes_ordered_by_url_set_.end(); ++i) {
     std::vector<BookmarkBarNode*>::iterator insert_position =
@@ -163,6 +179,7 @@ void BookmarkBarModel::GetBookmarksMatchingText(
   if (query_nodes.empty())
     return;
 
+  AutoLock url_lock(url_lock_);
   for (NodesOrderedByURLSet::iterator i = nodes_ordered_by_url_set_.begin();
        i != nodes_ordered_by_url_set_.end(); ++i) {
     if (parser.DoesQueryMatch((*i)->GetTitle(), query_nodes.get()))
@@ -236,11 +253,20 @@ void BookmarkBarModel::SetTitle(BookmarkBarNode* node,
 }
 
 BookmarkBarNode* BookmarkBarModel::GetNodeByURL(const GURL& url) {
+  AutoLock url_lock(url_lock_);
   BookmarkBarNode 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;
 }
 
+void BookmarkBarModel::GetBookmarks(std::vector<GURL>* urls) {
+  AutoLock url_lock(url_lock_);
+  for (NodesOrderedByURLSet::iterator i = nodes_ordered_by_url_set_.begin();
+       i != nodes_ordered_by_url_set_.end(); ++i) {
+    urls->push_back((*i)->GetURL());
+  }
+}
+
 BookmarkBarNode* BookmarkBarModel::GetNodeByID(int id) {
   // TODO(sky): TreeNode needs a method that visits all nodes using a predicate.
   return GetNodeByID(&root_, id);
@@ -296,6 +322,7 @@ BookmarkBarNode* BookmarkBarModel::AddURLWithCreationTime(
   new_node->date_added_ = creation_time;
   new_node->type_ = history::StarredEntry::URL;
 
+  AutoLock url_lock(url_lock_);
   nodes_ordered_by_url_set_.insert(new_node);
 
   return AddNode(parent, index, new_node);
@@ -333,6 +360,8 @@ void BookmarkBarModel::RemoveNode(BookmarkBarNode* node,
   }
 
   if (node->GetType() == history::StarredEntry::URL) {
+    // NOTE: this is called in such a way that url_lock_ is already held. As
+    // 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());
     nodes_ordered_by_url_set_.erase(i);
@@ -367,14 +396,23 @@ void BookmarkBarModel::OnBookmarkStorageLoadedBookmarks(
     return;
   }
 
-  // The file doesn't exist. If the bookmarks were in the db the history db
-  // will copy them to a file on init. Schedule an empty request to history so
-  // that we know history will have saved the bookmarks (if it had them). When
-  // the callback is processed 
-  // when done we'll try and load the bookmarks again.
-  profile_->GetHistoryService(Profile::EXPLICIT_ACCESS)->
-      ScheduleEmptyCallback(&load_consumer_,
-      NewCallback(this, &BookmarkBarModel::OnHistoryDone));
+  // 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, history will write the
+  // bookmarks to a file for us. We need to wait until history has finished
+  // loading before reading from that file.
+  HistoryService* history =
+      profile_->GetHistoryService(Profile::EXPLICIT_ACCESS);
+  if (!history->backend_loaded()) {
+    // The backend isn't finished loading. Wait for it.
+    waiting_for_history_load_ = true;
+    NotificationService::current()->AddObserver(
+        this, NOTIFY_HISTORY_LOADED, Source<Profile>(profile_));
+  } else {
+    OnHistoryDone();
+  }
 }
 
 void BookmarkBarModel::OnHistoryDone() {
@@ -389,11 +427,18 @@ void BookmarkBarModel::OnHistoryDone() {
 }
 
 void BookmarkBarModel::DoneLoading() {
-  // Update nodes_ordered_by_url_set_ from the nodes.
-  PopulateNodesByURL(&root_);
+  {
+    AutoLock url_lock(url_lock_);
+    // Update nodes_ordered_by_url_set_ from the nodes.
+    PopulateNodesByURL(&root_);
+  }
 
   loaded_ = true;
 
+  if (loaded_signal_.Get())
+    SetEvent(loaded_signal_.Get());
+
+
   // Notify our direct observers.
   FOR_EACH_OBSERVER(BookmarkBarModelObserver, observers_, Loaded(this));
 
@@ -412,7 +457,10 @@ void BookmarkBarModel::RemoveAndDeleteNode(BookmarkBarNode* delete_me) {
   int index = parent->IndexOfChild(node.get());
   parent->Remove(index);
   history::URLsStarredDetails details(false);
-  RemoveNode(node.get(), &details.changed_urls);
+  {
+    AutoLock url_lock(url_lock_);
+    RemoveNode(node.get(), &details.changed_urls);
+  }
 
   if (store_.get())
     store_->ScheduleSave();
@@ -420,6 +468,13 @@ void BookmarkBarModel::RemoveAndDeleteNode(BookmarkBarNode* delete_me) {
   FOR_EACH_OBSERVER(BookmarkBarModelObserver, observers_,
                     BookmarkNodeRemoved(this, parent, index));
 
+  if (profile_) {
+    HistoryService* history =
+        profile_->GetHistoryService(Profile::EXPLICIT_ACCESS);
+    if (history)
+      history->URLsNoLongerBookmarked(details.changed_urls);
+  }
+
   NotificationService::current()->Notify(NOTIFY_URLS_STARRED,
       Source<Profile>(profile_),
       Details<history::URLsStarredDetails>(&details));
@@ -446,6 +501,11 @@ BookmarkBarNode* BookmarkBarModel::AddNode(BookmarkBarNode* parent,
   return node;
 }
 
+void BookmarkBarModel::BlockTillLoaded() {
+  if (loaded_signal_.Get())
+    WaitForSingleObject(loaded_signal_.Get(), INFINITE);
+}
+
 BookmarkBarNode* BookmarkBarModel::GetNodeByID(BookmarkBarNode* node,
                                                int id) {
   if (node->id() == id)
@@ -595,6 +655,18 @@ void BookmarkBarModel::Observe(NotificationType type,
       break;
     }
 
+    case NOTIFY_HISTORY_LOADED: {
+      if (waiting_for_history_load_) {
+        waiting_for_history_load_ = false;
+        NotificationService::current()->RemoveObserver(
+            this, NOTIFY_HISTORY_LOADED, Source<Profile>(profile_));
+        OnHistoryDone();
+      } else {
+        NOTREACHED();
+      }
+      break;
+    }
+
     default:
       NOTREACHED();
       break;
@@ -602,9 +674,10 @@ void BookmarkBarModel::Observe(NotificationType type,
 }
 
 void BookmarkBarModel::PopulateNodesByURL(BookmarkBarNode* node) {
+  // NOTE: this is called with url_lock_ already held. As such, this doesn't
+  // explicitly grab the lock.
   if (node->is_url())
     nodes_ordered_by_url_set_.insert(node);
   for (int i = 0; i < node->GetChildCount(); ++i)
     PopulateNodesByURL(node->GetChild(i));
 }
-
diff --git a/chrome/browser/bookmark_bar_model.h b/chrome/browser/bookmark_bar_model.h
index c4c3bd6..22c5291 100644
--- a/chrome/browser/bookmark_bar_model.h
+++ b/chrome/browser/bookmark_bar_model.h
@@ -5,7 +5,10 @@
 #ifndef CHROME_BROWSER_BOOKMARK_BAR_MODEL_H_
 #define CHROME_BROWSER_BOOKMARK_BAR_MODEL_H_
 
+#include "base/lock.h"
 #include "base/observer_list.h"
+#include "base/scoped_handle.h"
+#include "chrome/browser/bookmarks/bookmark_service.h"
 #include "chrome/browser/bookmark_storage.h"
 #include "chrome/browser/cancelable_request.h"
 #include "chrome/browser/history/history.h"
@@ -37,7 +40,6 @@ class BookmarkBarNode : public ChromeViews::TreeNode<BookmarkBarNode> {
 
  public:
   BookmarkBarNode(BookmarkBarModel* model, const GURL& url);
-
   virtual ~BookmarkBarNode() {}
 
   // Returns the favicon for the this node. If the favicon has not yet been
@@ -167,7 +169,7 @@ class BookmarkBarModelObserver {
 // Profile.
 
 // TODO(sky): rename to BookmarkModel.
-class BookmarkBarModel : public NotificationObserver {
+class BookmarkBarModel : public NotificationObserver, public BookmarkService {
   friend class BookmarkBarNode;
   friend class BookmarkBarModelTest;
   friend class BookmarkStorage;
@@ -175,6 +177,10 @@ class BookmarkBarModel : public NotificationObserver {
  public:
   explicit BookmarkBarModel(Profile* profile);
   virtual ~BookmarkBarModel();
+  
+  // Loads the bookmarks. This is called by Profile upon creation of the
+  // BookmarkBarModel. You need not invoke this directly.
+  void Load();
 
   // Returns the root node. The bookmark bar node and other node are children of
   // the root node.
@@ -224,14 +230,22 @@ class BookmarkBarModel : public NotificationObserver {
   bool IsLoaded() { return loaded_; }
 
   // Returns the node with the specified URL, or NULL if there is no node with
-  // the specified URL.
+  // the specified URL. This method is thread safe.
   BookmarkBarNode* GetNodeByURL(const GURL& url);
 
-  // Returns true if there is a bookmark for the specified URL.
-  bool IsBookmarked(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;
   }
 
+  // Blocks until loaded; this is NOT invoked on the main thread. See
+  // BookmarkService for more details on this.
+  virtual void BlockTillLoaded();
+
   // Returns the node with the specified id, or NULL if there is no node with
   // the specified id.
   BookmarkBarNode* GetNodeByID(int id);
@@ -381,8 +395,11 @@ class BookmarkBarModel : public NotificationObserver {
 
   // Set of nodes ordered by URL. This is not a map to avoid copying the
   // 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<BookmarkBarNode*,NodeURLComparator> NodesOrderedByURLSet;
   NodesOrderedByURLSet nodes_ordered_by_url_set_;
+  Lock url_lock_;
 
   // Used for loading favicons and the empty history request.
   CancelableRequestConsumerT<BookmarkBarNode*, NULL> load_consumer_;
@@ -390,6 +407,14 @@ class BookmarkBarModel : public NotificationObserver {
   // Reads/writes bookmarks to disk.
   scoped_refptr<BookmarkStorage> store_;
 
+  // Have we installed a listener on the NotificationService for
+  // NOTIFY_HISTORY_LOADED? A listener is installed if the bookmarks file
+  // doesn't exist and the history service hasn't finished loading.
+  bool waiting_for_history_load_;
+
+  // Handle to event signaled when loading is done.
+  ScopedHandle loaded_signal_;
+
   DISALLOW_EVIL_CONSTRUCTORS(BookmarkBarModel);
 };
 
diff --git a/chrome/browser/bookmark_bar_model_unittest.cc b/chrome/browser/bookmark_bar_model_unittest.cc
index 809084e..3f5493d 100644
--- a/chrome/browser/bookmark_bar_model_unittest.cc
+++ b/chrome/browser/bookmark_bar_model_unittest.cc
@@ -438,8 +438,6 @@ class BookmarkBarModelTestWithProfile : public testing::Test,
                                         public BookmarkBarModelObserver {
  public:
   virtual void SetUp() {
-    profile_.reset(new TestingProfile());
-    profile_->CreateHistoryService(true);
   }
 
   virtual void TearDown() {
@@ -473,30 +471,24 @@ class BookmarkBarModelTestWithProfile : public testing::Test,
     }
   }
 
-  // Creates the bookmark bar model. If the bookmark bar model has already been
-  // created a new one is created and the old one destroyed.
-  void CreateBookmarkBarModel() {
-    bb_model_.reset(NULL);
-
-    BookmarkBarModel* new_model = new BookmarkBarModel(profile_.get());
-    if (!new_model->IsLoaded())
-      BlockTillLoaded(new_model);
+  void BlockTillBookmarkModelLoaded() {
+    bb_model_ = profile_->GetBookmarkBarModel();
+    if (!bb_model_->IsLoaded())
+      BlockTillLoaded(bb_model_);
     else
-      new_model->AddObserver(this);
-    bb_model_.reset(new_model);
+      bb_model_->AddObserver(this);
   }
 
-  // Destroys the bookmark bar model, then the profile, then creates a new
-  // profile.
+  // Destroys the current profile, creates a new one and creates the history
+  // service.
   void RecreateProfile() {
-    bb_model_.reset(NULL);
     // Need to shutdown the old one before creating a new one.
     profile_.reset(NULL);
     profile_.reset(new TestingProfile());
     profile_->CreateHistoryService(true);
   }
 
-  scoped_ptr<BookmarkBarModel> bb_model_;
+  BookmarkBarModel* bb_model_;
 
  private:
   // Blocks until the BookmarkBarModel has finished loading.
@@ -548,20 +540,25 @@ TEST_F(BookmarkBarModelTestWithProfile, CreateAndRestore) {
     { L"a b c [ d e [ f ] ]", L"g h i [ j k [ l ] ]"},
   };
   for (int i = 0; i < arraysize(data); ++i) {
-    RecreateProfile();
-
-    CreateBookmarkBarModel();
+    // Recreate the profile. We need to reset with NULL first so that the last
+    // HistoryService releases the locks on the files it creates and we can
+    // delete them.
+    profile_.reset(NULL);
+    profile_.reset(new TestingProfile());
+    profile_->CreateBookmarkBarModel(true);
+    profile_->CreateHistoryService(true);
+    BlockTillBookmarkModelLoaded();
 
     TestNode bbn;
     PopulateNodeFromString(data[i].bbn_contents, &bbn);
-    PopulateBookmarkBarNode(&bbn, bb_model_.get(),
-                            bb_model_->GetBookmarkBarNode());
+    PopulateBookmarkBarNode(&bbn, bb_model_, bb_model_->GetBookmarkBarNode());
 
     TestNode other;
     PopulateNodeFromString(data[i].other_contents, &other);
-    PopulateBookmarkBarNode(&other, bb_model_.get(), bb_model_->other_node());
+    PopulateBookmarkBarNode(&other, bb_model_, bb_model_->other_node());
 
-    CreateBookmarkBarModel();
+    profile_->CreateBookmarkBarModel(false);
+    BlockTillBookmarkModelLoaded();
 
     VerifyModelMatchesNode(&bbn, bb_model_->GetBookmarkBarNode());
     VerifyModelMatchesNode(&other, bb_model_->other_node());
@@ -656,8 +653,8 @@ TEST_F(BookmarkBarModelTestWithProfile2, MigrateFromDBToFileTest) {
   // Create the history service making sure it doesn't blow away the file we
   // just copied.
   profile_->CreateHistoryService(false);
-
-  CreateBookmarkBarModel();
+  profile_->CreateBookmarkBarModel(true);
+  BlockTillBookmarkModelLoaded();
 
   // Make sure we loaded OK.
   VerifyExpectedState();
@@ -665,7 +662,8 @@ TEST_F(BookmarkBarModelTestWithProfile2, MigrateFromDBToFileTest) {
     return;
 
   // Create again. This time we shouldn't load from history at all.
-  CreateBookmarkBarModel();
+  profile_->CreateBookmarkBarModel(false);
+  BlockTillBookmarkModelLoaded();
 
   // Make sure we loaded OK.
   VerifyExpectedState();
@@ -675,7 +673,27 @@ TEST_F(BookmarkBarModelTestWithProfile2, MigrateFromDBToFileTest) {
   // Recreate the history service (with a clean db). Do this just to make sure
   // we're loading correctly from the bookmarks file.
   profile_->CreateHistoryService(true);
-  CreateBookmarkBarModel();
+  profile_->CreateBookmarkBarModel(false);
+  BlockTillBookmarkModelLoaded();
   VerifyExpectedState();
 }
 
+// Simple test that removes a bookmark. This test exercises the code paths in
+// History that block till bookmark bar model is loaded.
+TEST_F(BookmarkBarModelTestWithProfile2, RemoveNotification) {
+  profile_->CreateHistoryService(false);
+  profile_->CreateBookmarkBarModel(true);
+  BlockTillBookmarkModelLoaded();
+
+  // Add a URL.
+  GURL url("http://www.google.com");
+  bb_model_->SetURLStarred(url, std::wstring(), true);
+
+  profile_->GetHistoryService(Profile::EXPLICIT_ACCESS)->AddPage(
+      url, NULL, 1, GURL(), PageTransition::TYPED,
+      HistoryService::RedirectList());
+
+  // This won't actually delete the URL, rather it'll empty out the visits.
+  // This triggers blocking on the BookmarkBarModel.
+  profile_->GetHistoryService(Profile::EXPLICIT_ACCESS)->DeleteURL(url);
+}
diff --git a/chrome/browser/bookmark_codec.cc b/chrome/browser/bookmark_codec.cc
index de8e47c..14d8851 100644
--- a/chrome/browser/bookmark_codec.cc
+++ b/chrome/browser/bookmark_codec.cc
@@ -139,6 +139,8 @@ bool BookmarkCodec::DecodeNode(BookmarkBarModel* model,
   if (!value.GetString(kNameKey, &title))
     return false;
 
+  // TODO(sky): this should be more flexible. Don't hoark if we can't parse it
+  // all.
   std::wstring date_added_string;
   if (!value.GetString(kDateAddedKey, &date_added_string))
     return false;
@@ -154,6 +156,7 @@ bool BookmarkCodec::DecodeNode(BookmarkBarModel* model,
     std::wstring url_string;
     if (!value.GetString(kURLKey, &url_string))
       return false;
+    // TODO(sky): this should ignore the node if not a valid URL.
     if (!node)
       node = new BookmarkBarNode(model, GURL(url_string));
     if (parent)
diff --git a/chrome/browser/bookmark_storage.cc b/chrome/browser/bookmark_storage.cc
index d760b5a..e944de7 100644
--- a/chrome/browser/bookmark_storage.cc
+++ b/chrome/browser/bookmark_storage.cc
@@ -1,171 +1,166 @@
-// Copyright (c) 2006-2008 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.

-

-// NOTE: This class is currently unsed. The plan is to move bookmarks

-// out of the history db using this class and BookmarksCodec instead

-// (bug 1256202).

-

-#include "chrome/browser/bookmark_storage.h"

-

-#include "base/file_util.h"

-#include "base/json_writer.h"

-#include "base/message_loop.h"

-#include "chrome/browser/bookmark_bar_model.h"

-#include "chrome/browser/bookmark_codec.h"

-#include "chrome/browser/profile.h"

-#include "chrome/common/chrome_constants.h"

-#include "chrome/common/json_value_serializer.h"

-

-namespace {

-

-// Extension used for backup files (copy of main file created during startup).

-const wchar_t* const kBackupExtension = L"bak";

-

-// Extension for the temporary file. We write to the temp file than move to

-// kBookmarksFileName.

-const wchar_t* const kTmpExtension = L"tmp";

-

-// How often we save.

-const int kSaveDelayMS = 2500;

-

-}  // namespace

-

-// BookmarkStorage -------------------------------------------------------------

-

-BookmarkStorage::BookmarkStorage(Profile* profile, BookmarkBarModel* model)

-    : model_(model),

-#pragma warning(suppress: 4355)  // Okay to pass "this" here.

-      save_factory_(this),

-      backend_thread_(g_browser_process->file_thread()) {

-  std::wstring path = profile->GetPath();

-  file_util::AppendToPath(&path, chrome::kBookmarksFileName);

-  std::wstring tmp_history_path = profile->GetPath();

-  file_util::AppendToPath(&tmp_history_path, chrome::kHistoryBookmarksFileName);

-  backend_ = new BookmarkStorageBackend(path, tmp_history_path);

-}

-

-void BookmarkStorage::LoadBookmarks(bool load_from_history) {

-  if (!backend_thread()) {

-    backend_->Read(scoped_refptr<BookmarkStorage>(this), NULL,

-                   load_from_history);

-  } else {

-    backend_thread()->message_loop()->PostTask(

-        FROM_HERE,

-        NewRunnableMethod(backend_.get(), &BookmarkStorageBackend::Read,

-                          scoped_refptr<BookmarkStorage>(this),

-                          MessageLoop::current(), load_from_history));

-  }

-}

-

-void BookmarkStorage::ScheduleSave() {

-  if (save_factory_.empty()) {

-    MessageLoop::current()->PostDelayedTask(

-        FROM_HERE, save_factory_.NewRunnableMethod(&BookmarkStorage::SaveNow),

-        kSaveDelayMS);

-  }

-}

-

-void BookmarkStorage::BookmarkModelDeleted() {

-  if (!save_factory_.empty()) {

-    // There's a pending save. We need to save now as otherwise by the time

-    // SaveNow is invoked the model is gone.

-    save_factory_.RevokeAll();

-    SaveNow();

-  }

-  model_ = NULL;

-}

-

-void BookmarkStorage::LoadedBookmarks(Value* root_value,

-                                      bool bookmark_file_exists,

-                                      bool loaded_from_history) {

-  scoped_ptr<Value> value_ref(root_value);

-

-  if (model_) {

-    if (root_value) {

-      BookmarkCodec codec;

-      codec.Decode(model_, *root_value);

-    }

-    model_->OnBookmarkStorageLoadedBookmarks(bookmark_file_exists,

-                                             loaded_from_history);

-  }

-}

-

-void BookmarkStorage::SaveNow() {

-  if (!model_ || !model_->IsLoaded()) {

-    // We should only get here if we have a valid model and it's finished

-    // loading.

-    NOTREACHED();

-    return;

-  }

-

-  BookmarkCodec codec;

-  Value* value = codec.Encode(model_);

-  // The backend deletes value in write.

-  if (!backend_thread()) {

-    backend_->Write(value);

-  } else {

-    backend_thread()->message_loop()->PostTask(

-        FROM_HERE,

-        NewRunnableMethod(backend_.get(), &BookmarkStorageBackend::Write,

-                          value));

-  }

-}

-

-// BookmarkStorageBackend ------------------------------------------------------

-

-BookmarkStorageBackend::BookmarkStorageBackend(

-    const std::wstring& path,

-    const std::wstring& tmp_history_path)

-    : path_(path),

-      tmp_history_path_(tmp_history_path) {

-  // Make a backup of the current file.

-  std::wstring backup_path = path;

-  file_util::ReplaceExtension(&backup_path, kBackupExtension);

-  file_util::CopyFile(path, backup_path);

-}

-

-void BookmarkStorageBackend::Write(Value* value) {

-  DCHECK(value);

-

-  // We own Value.

-  scoped_ptr<Value> value_ref(value);

-

-  std::string content;

-  JSONWriter::Write(value, true, &content);

-

-  // Write to a temp file, then rename.

-  std::wstring tmp_file = path_;

-  file_util::ReplaceExtension(&tmp_file, kTmpExtension);

-

-  int bytes_written = file_util::WriteFile(tmp_file, content.c_str(),

-                                           static_cast<int>(content.length()));

-  if (bytes_written != -1) {

-    MoveFileEx(tmp_file.c_str(), path_.c_str(),

-               MOVEFILE_COPY_ALLOWED | MOVEFILE_REPLACE_EXISTING);

-    // Nuke the history file so that we don't attempt to load from it again.

-    file_util::Delete(tmp_history_path_, false);

-  }

-}

-

-void BookmarkStorageBackend::Read(scoped_refptr<BookmarkStorage> service,

-                                  MessageLoop* message_loop,

-                                  bool load_from_history) {

-  const std::wstring& path = load_from_history ? tmp_history_path_ : path_;

-  bool bookmark_file_exists = file_util::PathExists(path);

-  Value* root = NULL;

-  if (bookmark_file_exists) {

-    JSONFileValueSerializer serializer(path);

-    serializer.Deserialize(&root);

-  }

-

-  // BookmarkStorage takes ownership of root.

-  if (message_loop) {

-    message_loop->PostTask(FROM_HERE, NewRunnableMethod(

-        service.get(), &BookmarkStorage::LoadedBookmarks, root,

-        bookmark_file_exists, load_from_history));

-  } else {

-    service->LoadedBookmarks(root, bookmark_file_exists, load_from_history);

-  }

-}

-

+// Copyright (c) 2006-2008 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.
+
+#include "chrome/browser/bookmark_storage.h"
+
+#include "base/file_util.h"
+#include "base/json_writer.h"
+#include "base/message_loop.h"
+#include "chrome/browser/bookmark_bar_model.h"
+#include "chrome/browser/bookmark_codec.h"
+#include "chrome/browser/profile.h"
+#include "chrome/common/chrome_constants.h"
+#include "chrome/common/json_value_serializer.h"
+
+namespace {
+
+// Extension used for backup files (copy of main file created during startup).
+const wchar_t* const kBackupExtension = L"bak";
+
+// Extension for the temporary file. We write to the temp file than move to
+// kBookmarksFileName.
+const wchar_t* const kTmpExtension = L"tmp";
+
+// How often we save.
+const int kSaveDelayMS = 2500;
+
+}  // namespace
+
+// BookmarkStorage -------------------------------------------------------------
+
+BookmarkStorage::BookmarkStorage(Profile* profile, BookmarkBarModel* model)
+    : model_(model),
+#pragma warning(suppress: 4355)  // Okay to pass "this" here.
+      save_factory_(this),
+      backend_thread_(g_browser_process->file_thread()) {
+  std::wstring path = profile->GetPath();
+  file_util::AppendToPath(&path, chrome::kBookmarksFileName);
+  std::wstring tmp_history_path = profile->GetPath();
+  file_util::AppendToPath(&tmp_history_path, chrome::kHistoryBookmarksFileName);
+  backend_ = new BookmarkStorageBackend(path, tmp_history_path);
+}
+
+void BookmarkStorage::LoadBookmarks(bool load_from_history) {
+  if (!backend_thread()) {
+    backend_->Read(scoped_refptr<BookmarkStorage>(this), NULL,
+                   load_from_history);
+  } else {
+    backend_thread()->message_loop()->PostTask(
+        FROM_HERE,
+        NewRunnableMethod(backend_.get(), &BookmarkStorageBackend::Read,
+                          scoped_refptr<BookmarkStorage>(this),
+                          MessageLoop::current(), load_from_history));
+  }
+}
+
+void BookmarkStorage::ScheduleSave() {
+  if (save_factory_.empty()) {
+    MessageLoop::current()->PostDelayedTask(
+        FROM_HERE, save_factory_.NewRunnableMethod(&BookmarkStorage::SaveNow),
+        kSaveDelayMS);
+  }
+}
+
+void BookmarkStorage::BookmarkModelDeleted() {
+  if (!save_factory_.empty()) {
+    // There's a pending save. We need to save now as otherwise by the time
+    // SaveNow is invoked the model is gone.
+    save_factory_.RevokeAll();
+    SaveNow();
+  }
+  model_ = NULL;
+}
+
+void BookmarkStorage::LoadedBookmarks(Value* root_value,
+                                      bool bookmark_file_exists,
+                                      bool loaded_from_history) {
+  scoped_ptr<Value> value_ref(root_value);
+
+  if (model_) {
+    if (root_value) {
+      BookmarkCodec codec;
+      codec.Decode(model_, *root_value);
+    }
+    model_->OnBookmarkStorageLoadedBookmarks(bookmark_file_exists,
+                                             loaded_from_history);
+  }
+}
+
+void BookmarkStorage::SaveNow() {
+  if (!model_ || !model_->IsLoaded()) {
+    // We should only get here if we have a valid model and it's finished
+    // loading.
+    NOTREACHED();
+    return;
+  }
+
+  BookmarkCodec codec;
+  Value* value = codec.Encode(model_);
+  // The backend deletes value in write.
+  if (!backend_thread()) {
+    backend_->Write(value);
+  } else {
+    backend_thread()->message_loop()->PostTask(
+        FROM_HERE,
+        NewRunnableMethod(backend_.get(), &BookmarkStorageBackend::Write,
+                          value));
+  }
+}
+
+// BookmarkStorageBackend ------------------------------------------------------
+
+BookmarkStorageBackend::BookmarkStorageBackend(
+    const std::wstring& path,
+    const std::wstring& tmp_history_path)
+    : path_(path),
+      tmp_history_path_(tmp_history_path) {
+  // Make a backup of the current file.
+  std::wstring backup_path = path;
+  file_util::ReplaceExtension(&backup_path, kBackupExtension);
+  file_util::CopyFile(path, backup_path);
+}
+
+void BookmarkStorageBackend::Write(Value* value) {
+  DCHECK(value);
+
+  // We own Value.
+  scoped_ptr<Value> value_ref(value);
+
+  std::string content;
+  JSONWriter::Write(value, true, &content);
+
+  // Write to a temp file, then rename.
+  std::wstring tmp_file = path_;
+  file_util::ReplaceExtension(&tmp_file, kTmpExtension);
+
+  int bytes_written = file_util::WriteFile(tmp_file, content.c_str(),
+                                           static_cast<int>(content.length()));
+  if (bytes_written != -1) {
+    MoveFileEx(tmp_file.c_str(), path_.c_str(),
+               MOVEFILE_COPY_ALLOWED | MOVEFILE_REPLACE_EXISTING);
+    // Nuke the history file so that we don't attempt to load from it again.
+    file_util::Delete(tmp_history_path_, false);
+  }
+}
+
+void BookmarkStorageBackend::Read(scoped_refptr<BookmarkStorage> service,
+                                  MessageLoop* message_loop,
+                                  bool load_from_history) {
+  const std::wstring& path = load_from_history ? tmp_history_path_ : path_;
+  bool bookmark_file_exists = file_util::PathExists(path);
+  Value* root = NULL;
+  if (bookmark_file_exists) {
+    JSONFileValueSerializer serializer(path);
+    serializer.Deserialize(&root);
+  }
+
+  // BookmarkStorage takes ownership of root.
+  if (message_loop) {
+    message_loop->PostTask(FROM_HERE, NewRunnableMethod(
+        service.get(), &BookmarkStorage::LoadedBookmarks, root,
+        bookmark_file_exists, load_from_history));
+  } else {
+    service->LoadedBookmarks(root, bookmark_file_exists, load_from_history);
+  }
+}
diff --git a/chrome/browser/bookmarks/bookmark_service.h b/chrome/browser/bookmarks/bookmark_service.h
new file mode 100644
index 0000000..a758382
--- /dev/null
+++ b/chrome/browser/bookmarks/bookmark_service.h
@@ -0,0 +1,61 @@
+// Copyright 2008, Google Inc.
+// All rights reserved.
+//
+// Redistribution and use in source and binary forms, with or without
+// modification, are permitted provided that the following conditions are
+// met:
+//
+//    * Redistributions of source code must retain the above copyright
+// notice, this list of conditions and the following disclaimer.
+//    * Redistributions in binary form must reproduce the above
+// copyright notice, this list of conditions and the following disclaimer
+// in the documentation and/or other materials provided with the
+// distribution.
+//    * Neither the name of Google Inc. nor the names of its
+// contributors may be used to endorse or promote products derived from
+// this software without specific prior written permission.
+//
+// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+#ifndef CHROME_BROWSER_BOOKMARKS_BOOKMARK_SERVICE_H_
+#define CHROME_BROWSER_BOOKMARKS_BOOKMARK_SERVICE_H_
+
+#include <vector>
+
+#include "base/ref_counted.h"
+
+class GURL;
+
+// BookmarkService provides a thread safe view of bookmarks. It is used by
+// HistoryBackend when it needs to determine the set of bookmarked URLs
+// or if a URL is bookmarked.
+//
+// BookmarkService is owned by Profile and deleted when the Profile is deleted.
+class BookmarkService {
+ public:
+  // 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.
+  //
+  // If not on the main thread you *must* invoke BlockTillLoaded first.
+  virtual void GetBookmarks(std::vector<GURL>* urls) = 0;
+
+  // Blocks until loaded. This is intended for usage on a thread other than
+  // the main thread.
+  virtual void BlockTillLoaded() = 0;
+};
+
+#endif  // CHROME_BROWSER_BOOKMARKS_BOOKMARK_SERVICE_H_
diff --git a/chrome/browser/browser.vcproj b/chrome/browser/browser.vcproj
index cb2ee81..5a76452 100644
--- a/chrome/browser/browser.vcproj
+++ b/chrome/browser/browser.vcproj
@@ -834,6 +834,10 @@
 				>
 			</File>
 			<File
+				RelativePath=".\bookmarks\bookmark_service.h"
+				>
+			</File>
+			<File
 				RelativePath=".\bookmark_storage.cc"
 				>
 			</File>
diff --git a/chrome/browser/history/expire_history_backend.cc b/chrome/browser/history/expire_history_backend.cc
index 63983a0..fcd8f67 100644
--- a/chrome/browser/history/expire_history_backend.cc
+++ b/chrome/browser/history/expire_history_backend.cc
@@ -8,6 +8,7 @@
 #include <limits>
 
 #include "base/file_util.h"
+#include "chrome/browser/bookmarks/bookmark_service.h"
 #include "chrome/browser/history/archived_database.h"
 #include "chrome/browser/history/history_database.h"
 #include "chrome/browser/history/history_notifications.h"
@@ -63,14 +64,16 @@ const int kExpirationEmptyDelayMin = 5;
 }  // namespace
 
 ExpireHistoryBackend::ExpireHistoryBackend(
-    BroadcastNotificationDelegate* delegate)
+    BroadcastNotificationDelegate* delegate,
+    BookmarkService* bookmark_service)
     : delegate_(delegate),
       main_db_(NULL),
       archived_db_(NULL),
       thumb_db_(NULL),
       text_db_(NULL),
 #pragma warning(suppress: 4355)  // Okay to pass "this" here.
-      factory_(this) {
+      factory_(this),
+      bookmark_service_(bookmark_service) {
 }
 
 ExpireHistoryBackend::~ExpireHistoryBackend() {
@@ -94,10 +97,6 @@ void ExpireHistoryBackend::DeleteURL(const GURL& url) {
   if (!main_db_->GetRowForURL(url, &url_row))
     return;  // Nothing to delete.
 
-  // The URL may be in the text database manager's temporary cache.
-  if (text_db_)
-    text_db_->DeleteURLFromUncommitted(url);
-
   // Collect all the visits and delete them. Note that we don't give up if
   // there are no visits, since the URL could still have an entry that we should
   // delete.
@@ -111,11 +110,18 @@ void ExpireHistoryBackend::DeleteURL(const GURL& url) {
   // We skip ExpireURLsForVisits (since we are deleting from the URL, and not
   // starting with visits in a given time range). We therefore need to call the
   // deletion and favicon update functions manually.
-  DeleteOneURL(url_row, &dependencies);
-  DeleteFaviconsIfPossible(dependencies.affected_favicons);
+
+  BookmarkService* bookmark_service = GetBookmarkService();
+  bool is_bookmarked =
+      (bookmark_service && bookmark_service->IsBookmarked(url));
+
+  DeleteOneURL(url_row, is_bookmarked, &dependencies);
+  if (!is_bookmarked)
+    DeleteFaviconsIfPossible(dependencies.affected_favicons);
 
   if (text_db_)
     text_db_->OptimizeChangedDatabases(dependencies.text_db_changes);
+
   BroadcastDeleteNotifications(&dependencies);
 }
 
@@ -243,20 +249,28 @@ void ExpireHistoryBackend::DeleteVisitRelatedInfo(
 
 void ExpireHistoryBackend::DeleteOneURL(
     const URLRow& url_row,
+    bool is_bookmarked,
     DeleteDependencies* dependencies) {
-  dependencies->deleted_urls.push_back(url_row);
-
-  // Delete stuff that references this URL.
-  if (thumb_db_)
-    thumb_db_->DeleteThumbnail(url_row.id());
   main_db_->DeleteSegmentForURL(url_row.id());
 
-  // Collect shared information.
-  if (url_row.favicon_id())
-    dependencies->affected_favicons.insert(url_row.favicon_id());
+  // The URL may be in the text database manager's temporary cache.
+  if (text_db_)
+    text_db_->DeleteURLFromUncommitted(url_row.url());
 
-  // Last, delete the URL entry.
-  main_db_->DeleteURLRow(url_row.id());
+  if (!is_bookmarked) {
+    dependencies->deleted_urls.push_back(url_row);
+
+    // Delete stuff that references this URL.
+    if (thumb_db_)
+      thumb_db_->DeleteThumbnail(url_row.id());
+
+    // Collect shared information.
+    if (url_row.favicon_id())
+      dependencies->affected_favicons.insert(url_row.favicon_id());
+
+    // Last, delete the URL entry.
+    main_db_->DeleteURLRow(url_row.id());
+  }
 }
 
 URLID ExpireHistoryBackend::ArchiveOneURL(const URLRow& url_row) {
@@ -304,6 +318,7 @@ void ExpireHistoryBackend::ExpireURLsForVisits(
   }
 
   // Check each unique URL with deleted visits.
+  BookmarkService* bookmark_service = GetBookmarkService();
   for (std::map<URLID, ChangedURL>::const_iterator i = changed_urls.begin();
        i != changed_urls.end(); ++i) {
     // The unique URL rows should already be filled into the dependencies.
@@ -320,20 +335,22 @@ void ExpireHistoryBackend::ExpireURLsForVisits(
     else
       url_row.set_last_visit(Time());
 
-    // Don't delete URLs with visits still in the DB.
-    if (!url_row.last_visit().is_null()) {
-      // We're not deleting the URL, update its counts when we're deleting those
-      // visits.
+    // Don't delete URLs with visits still in the DB, or bookmarked.
+    bool is_bookmarked =
+        (bookmark_service && bookmark_service->IsBookmarked(url_row.url()));
+    if (!is_bookmarked && url_row.last_visit().is_null()) {
+      // Not bookmarked and no more visits. Nuke the url.
+      DeleteOneURL(url_row, is_bookmarked, dependencies);
+    } else {
       // NOTE: The calls to std::max() below are a backstop, but they should
       // never actually be needed unless the database is corrupt (I think).
       url_row.set_visit_count(
           std::max(0, url_row.visit_count() - i->second.visit_count));
       url_row.set_typed_count(
           std::max(0, url_row.typed_count() - i->second.typed_count));
+
+      // Update the db with the new details.
       main_db_->UpdateURLRow(url_row.id(), url_row);
-    } else {
-      // This URL is toast.
-      DeleteOneURL(url_row, dependencies);
     }
   }
 }
@@ -467,5 +484,15 @@ void ExpireHistoryBackend::ParanoidExpireHistory() {
   // FIXME(brettw): Bug 1067331: write this to clean up any errors.
 }
 
+BookmarkService* ExpireHistoryBackend::GetBookmarkService() {
+  // We use the bookmark service to determine if a URL is bookmarked. The
+  // bookmark service is loaded on a separate thread and may not be done by the
+  // time we get here. We therefor block until the bookmarks have finished
+  // loading.
+  if (bookmark_service_)
+    bookmark_service_->BlockTillLoaded();
+  return bookmark_service_;
+}
+
 }  // namespace history
 
diff --git a/chrome/browser/history/expire_history_backend.h b/chrome/browser/history/expire_history_backend.h
index 78c7c75..d795f88 100644
--- a/chrome/browser/history/expire_history_backend.h
+++ b/chrome/browser/history/expire_history_backend.h
@@ -15,6 +15,7 @@
 #include "chrome/browser/history/text_database_manager.h"
 #include "testing/gtest/include/gtest/gtest_prod.h"
 
+class BookmarkService;
 class GURL;
 enum NotificationType;
 
@@ -43,7 +44,11 @@ class BroadcastNotificationDelegate {
 class ExpireHistoryBackend {
  public:
   // The delegate pointer must be non-NULL. We will NOT take ownership of it.
-  explicit ExpireHistoryBackend(BroadcastNotificationDelegate* delegate);
+  // BookmarkService may be NULL. The BookmarkService is used when expiring
+  // URLs so that we don't remove any URLs or favicons that are bookmarked
+  // (visits are removed though).
+  ExpireHistoryBackend(BroadcastNotificationDelegate* delegate,
+                       BookmarkService* bookmark_service);
   ~ExpireHistoryBackend();
 
   // Completes initialization by setting the databases that this class will use.
@@ -80,6 +85,7 @@ class ExpireHistoryBackend {
   FRIEND_TEST(ExpireHistoryTest, DeleteTextIndexForURL);
   FRIEND_TEST(ExpireHistoryTest, DeleteFaviconsIfPossible);
   FRIEND_TEST(ExpireHistoryTest, ArchiveSomeOldHistory);
+  friend class TestingProfile;
 
   struct DeleteDependencies {
     // The time range affected. These can be is_null() to be unbounded in one
@@ -144,7 +150,13 @@ class ExpireHistoryBackend {
   // check for shared information at the end).
   //
   // Assumes the main_db_ is non-NULL.
-  void DeleteOneURL(const URLRow& url_row, DeleteDependencies* dependencies);
+  //
+  // NOTE: If the url is bookmarked only the segments and text db are updated,
+  // everything else is unchanged. This is done so that bookmarks retain their
+  // favicons and thumbnails.
+  void DeleteOneURL(const URLRow& url_row,
+                    bool is_bookmarked,
+                    DeleteDependencies* dependencies);
 
   // Adds or merges the given URL row with the archived database, returning the
   // ID of the URL in the archived database, or 0 on failure. The main (source)
@@ -191,12 +203,7 @@ class ExpireHistoryBackend {
   // care about favicons so much, so don't want to stop everything if it fails).
   void DeleteFaviconsIfPossible(const std::set<FavIconID>& favicon_id);
 
-  // Broadcasts that the given URLs and star entries were deleted. Either list
-  // can be empty, in which case no notification will be sent for that type.
-  //
-  // The passed-in arguments will be cleared becuase they will be swapped into
-  // the destination message to avoid copying). However, ownership of the
-  // dependencies object will not transfer.
+  // Broadcast the URL deleted notification.
   void BroadcastDeleteNotifications(DeleteDependencies* dependencies);
 
   // Schedules a call to DoArchiveIteration at the given time in the
@@ -217,6 +224,10 @@ class ExpireHistoryBackend {
   // and deletes items. For example, URLs with no visits.
   void ParanoidExpireHistory();
 
+  // Returns the BookmarkService, blocking until it is loaded. This may return
+  // NULL.
+  BookmarkService* GetBookmarkService();
+
   // Non-owning pointer to the notification delegate (guaranteed non-NULL).
   BroadcastNotificationDelegate* delegate_;
 
@@ -234,10 +245,15 @@ class ExpireHistoryBackend {
   // the archived database.
   TimeDelta expiration_threshold_;
 
+  // The BookmarkService; may be null. This is owned by the Profile.
+  //
+  // Use GetBookmarkService to access this, which makes sure the service is
+  // loaded.
+  BookmarkService* bookmark_service_;
+
   DISALLOW_EVIL_CONSTRUCTORS(ExpireHistoryBackend);
 };
 
 }  // namespace history
 
 #endif  // CHROME_BROWSER_HISTORY_EXPIRE_HISTORY_BACKEND_H__
-
diff --git a/chrome/browser/history/expire_history_backend_unittest.cc b/chrome/browser/history/expire_history_backend_unittest.cc
index fba70eb..05c382a 100644
--- a/chrome/browser/history/expire_history_backend_unittest.cc
+++ b/chrome/browser/history/expire_history_backend_unittest.cc
@@ -7,6 +7,7 @@
 #include "base/file_util.h"
 #include "base/path_service.h"
 #include "base/scoped_ptr.h"
+#include "chrome/browser/bookmark_bar_model.h"
 #include "chrome/browser/history/archived_database.h"
 #include "chrome/browser/history/expire_history_backend.h"
 #include "chrome/browser/history/history_database.h"
@@ -29,7 +30,9 @@ class ExpireHistoryTest : public testing::Test,
                           public BroadcastNotificationDelegate {
  public:
   ExpireHistoryTest()
-      : ALLOW_THIS_IN_INITIALIZER_LIST(expirer_(this)), now_(Time::Now()) {
+      : bookmark_model_(NULL),
+        ALLOW_THIS_IN_INITIALIZER_LIST(expirer_(this, &bookmark_model_)),
+        now_(Time::Now()) {
   }
 
  protected:
@@ -55,8 +58,15 @@ class ExpireHistoryTest : public testing::Test,
     notifications_.clear();
   }
 
+  void StarURL(const GURL& url) {
+    bookmark_model_.AddURL(
+        bookmark_model_.GetBookmarkBarNode(), 0, std::wstring(), url);
+  }
+
   static bool IsStringInFile(std::wstring& filename, const char* str);
 
+  BookmarkBarModel bookmark_model_;
+
   MessageLoop message_loop_;
 
   ExpireHistoryBackend expirer_;
@@ -438,6 +448,47 @@ TEST_F(ExpireHistoryTest, DeleteURLWithoutFavicon) {
   EXPECT_TRUE(HasFavIcon(last_row.favicon_id()));
 }
 
+// DeleteURL should not delete starred urls.
+TEST_F(ExpireHistoryTest, DontDeleteStarredURL) {
+  URLID url_ids[3];
+  Time visit_times[4];
+  AddExampleData(url_ids, visit_times);
+
+  URLRow url_row;
+  ASSERT_TRUE(main_db_->GetURLRow(url_ids[2], &url_row));
+
+  // Star the last URL.
+  StarURL(url_row.url());
+
+  // Attempt to delete the url.
+  expirer_.DeleteURL(url_row.url());
+
+  // Because the url is starred, it shouldn't be deleted.
+  GURL url = url_row.url();
+  ASSERT_TRUE(main_db_->GetRowForURL(url, &url_row));
+
+  // And the favicon should exist.
+  EXPECT_TRUE(HasFavIcon(url_row.favicon_id()));
+
+  // But there should be no fts. 
+  ASSERT_EQ(0, CountTextMatchesForURL(url_row.url()));
+
+  // And no visits.
+  VisitVector visits;
+  main_db_->GetVisitsForURL(url_row.id(), &visits);
+  ASSERT_EQ(0, visits.size());
+
+  // Should still have the thumbnail.
+  ASSERT_TRUE(HasThumbnail(url_row.id()));
+
+  // Unstar the URL and delete again.
+  bookmark_model_.SetURLStarred(url, std::wstring(), false);
+  expirer_.DeleteURL(url);
+  
+  // Now it should be completely deleted.
+  EnsureURLInfoGone(url_row);
+}
+
 // Expires all URLs more recent than a given time, with no starred items.
 // Our time threshold is such that one URL should be updated (we delete one of
 // the two visits) and one is deleted.
@@ -492,6 +543,49 @@ TEST_F(ExpireHistoryTest, FlushRecentURLsUnstarred) {
   EXPECT_FALSE(HasFavIcon(url_row2.favicon_id()));
 }
 
+// Expire a starred URL, it shouldn't get deleted
+TEST_F(ExpireHistoryTest, FlushRecentURLsStarred) {
+  URLID url_ids[3];
+  Time visit_times[4];
+  AddExampleData(url_ids, visit_times);
+
+  URLRow url_row1, url_row2;
+  ASSERT_TRUE(main_db_->GetURLRow(url_ids[1], &url_row1));
+  ASSERT_TRUE(main_db_->GetURLRow(url_ids[2], &url_row2));
+
+  // Star the last two URLs.
+  StarURL(url_row1.url());
+  StarURL(url_row2.url());
+
+  // This should delete the last two visits.
+  expirer_.ExpireHistoryBetween(visit_times[2], Time());
+
+  // The URL rows should still exist.
+  URLRow new_url_row1, new_url_row2;
+  ASSERT_TRUE(main_db_->GetURLRow(url_ids[1], &new_url_row1));
+  ASSERT_TRUE(main_db_->GetURLRow(url_ids[2], &new_url_row2));
+
+  // The visit times should be updated.
+  EXPECT_TRUE(new_url_row1.last_visit() == visit_times[1]);
+  EXPECT_TRUE(new_url_row2.last_visit().is_null());  // No last visit time.
+
+  // Visit/typed count should not be updated for bookmarks.
+  EXPECT_EQ(0, new_url_row1.typed_count());
+  EXPECT_EQ(1, new_url_row1.visit_count());
+  EXPECT_EQ(0, new_url_row2.typed_count());
+  EXPECT_EQ(0, new_url_row2.visit_count());
+
+  // Thumbnails and favicons should still exist. Note that we keep thumbnails
+  // that may have been updated since the time threshold. Since the URL still
+  // exists in history, this should not be a privacy problem, we only update
+  // the visit counts in this case for consistency anyway.
+  EXPECT_TRUE(HasFavIcon(new_url_row1.favicon_id()));
+  EXPECT_TRUE(HasThumbnail(new_url_row1.id()));
+  EXPECT_TRUE(HasFavIcon(new_url_row2.favicon_id()));
+  EXPECT_TRUE(HasThumbnail(new_url_row2.id()));
+
+}
+
 TEST_F(ExpireHistoryTest, ArchiveHistoryBeforeUnstarred) {
   URLID url_ids[3];
   Time visit_times[4];
@@ -530,6 +624,51 @@ TEST_F(ExpireHistoryTest, ArchiveHistoryBeforeUnstarred) {
   EXPECT_EQ(1, archived_visits.size());
 }
 
+TEST_F(ExpireHistoryTest, ArchiveHistoryBeforeStarred) {
+  URLID url_ids[3];
+  Time visit_times[4];
+  AddExampleData(url_ids, visit_times);
+
+  URLRow url_row0, url_row1;
+  ASSERT_TRUE(main_db_->GetURLRow(url_ids[0], &url_row0));
+  ASSERT_TRUE(main_db_->GetURLRow(url_ids[1], &url_row1));
+
+  // Star the URLs.
+  StarURL(url_row0.url());
+  StarURL(url_row1.url());
+
+  // Now archive the first three visits (first two URLs). The first two visits
+  // should be, the third deleted, but the URL records should not.
+  expirer_.ArchiveHistoryBefore(visit_times[2]);
+
+  // The first URL should have its visit deleted, but it should still be present
+  // in the main DB and not in the archived one since it is starred.
+  URLRow temp_row;
+  ASSERT_TRUE(main_db_->GetURLRow(url_ids[0], &temp_row));
+  // Note that the ID is different in the archived DB, so look up by URL.
+  EXPECT_FALSE(archived_db_->GetRowForURL(temp_row.url(), NULL));
+  VisitVector visits;
+  main_db_->GetVisitsForURL(temp_row.id(), &visits);
+  EXPECT_EQ(0, visits.size());
+
+  // The second URL should have its first visit deleted and its second visit
+  // archived. It should be present in both the main DB (because it's starred)
+  // and the archived DB (for the archived visit).
+  ASSERT_TRUE(main_db_->GetURLRow(url_ids[1], &temp_row));
+  main_db_->GetVisitsForURL(temp_row.id(), &visits);
+  EXPECT_EQ(0, visits.size());
+
+  // Note that the ID is different in the archived DB, so look up by URL.
+  ASSERT_TRUE(archived_db_->GetRowForURL(temp_row.url(), &temp_row));
+  archived_db_->GetVisitsForURL(temp_row.id(), &visits);
+  ASSERT_EQ(1, visits.size());
+  EXPECT_TRUE(visit_times[2] == visits[0].visit_time);
+
+  // The third URL should be unchanged.
+  EXPECT_TRUE(main_db_->GetURLRow(url_ids[2], &temp_row));
+  EXPECT_FALSE(archived_db_->GetRowForURL(temp_row.url(), NULL));
+}
+
 // Tests the return values from ArchiveSomeOldHistory. The rest of the
 // functionality of this function is tested by the ArchiveHistoryBefore*
 // tests which use this function internally.
@@ -557,4 +696,3 @@ TEST_F(ExpireHistoryTest, ArchiveSomeOldHistory) {
 // Maybe also refer to invalid favicons.
 
 }  // namespace history
-
diff --git a/chrome/browser/history/history.cc b/chrome/browser/history/history.cc
index abbde8b..94f980e 100644
--- a/chrome/browser/history/history.cc
+++ b/chrome/browser/history/history.cc
@@ -78,6 +78,11 @@ class HistoryService::BackendDelegate : public HistoryBackend::Delegate {
         &HistoryService::BroadcastNotifications, type, details));
   }
 
+  virtual void DBLoaded() {
+    message_loop_->PostTask(FROM_HERE, NewRunnableMethod(history_service_.get(),
+        &HistoryService::OnDBLoaded));
+  }
+
  private:
   scoped_refptr<HistoryService> history_service_;
   MessageLoop* message_loop_;
@@ -88,7 +93,8 @@ const history::StarID HistoryService::kBookmarkBarID = 1;
 
 HistoryService::HistoryService()
     : thread_(new ChromeThread(ChromeThread::HISTORY)),
-      profile_(NULL) {
+      profile_(NULL),
+      backend_loaded_(false) {
   if (NotificationService::current()) {  // Is NULL when running generate_profile.
     NotificationService::current()->AddObserver(
         this, NOTIFY_HISTORY_URLS_DELETED, Source<Profile>(profile_));
@@ -97,7 +103,8 @@ HistoryService::HistoryService()
 
 HistoryService::HistoryService(Profile* profile)
     : thread_(new ChromeThread(ChromeThread::HISTORY)),
-      profile_(profile) {
+      profile_(profile),
+      backend_loaded_(false) {
   NotificationService::current()->AddObserver(
       this, NOTIFY_HISTORY_URLS_DELETED, Source<Profile>(profile_));
 }
@@ -113,13 +120,15 @@ HistoryService::~HistoryService() {
   }
 }
 
-bool HistoryService::Init(const std::wstring& history_dir) {
+bool HistoryService::Init(const std::wstring& history_dir,
+                          BookmarkService* bookmark_service) {
   if (!thread_->Start())
     return false;
 
   // Create the history backend.
   scoped_refptr<HistoryBackend> backend(
-      new HistoryBackend(history_dir, new BackendDelegate(this)));
+      new HistoryBackend(history_dir, new BackendDelegate(this),
+                         bookmark_service));
   history_backend_.swap(backend);
 
   ScheduleAndForget(PRIORITY_UI, &HistoryBackend::Init);
@@ -206,6 +215,11 @@ HistoryService::Handle HistoryService::GetMostRecentKeywordSearchTerms(
                   keyword_id, prefix, max_count);
 }
 
+void HistoryService::URLsNoLongerBookmarked(const std::set<GURL>& urls) {
+  ScheduleAndForget(PRIORITY_NORMAL, &HistoryBackend::URLsNoLongerBookmarked,
+                    urls);
+}
+
 HistoryService::Handle HistoryService::ScheduleDBTask(
     HistoryDBTask* task,
     CancelableRequestConsumerBase* consumer) {
@@ -216,13 +230,6 @@ HistoryService::Handle HistoryService::ScheduleDBTask(
                   request);
 }
 
-HistoryService::Handle HistoryService::ScheduleEmptyCallback(
-    CancelableRequestConsumerBase* consumer,
-    EmptyHistoryCallback* callback) {
-  return Schedule(PRIORITY_UI, &HistoryBackend::ProcessEmptyRequest,
-                  consumer, new history::EmptyHistoryRequest(callback));
-}
-
 HistoryService::Handle HistoryService::QuerySegmentUsageSince(
     CancelableRequestConsumerBase* consumer,
     const Time from_time,
@@ -629,3 +636,9 @@ void HistoryService::BroadcastNotifications(
   NotificationService::current()->Notify(type, source, det);
 }
 
+void HistoryService::OnDBLoaded() {
+  backend_loaded_ = true;
+  NotificationService::current()->Notify(NOTIFY_HISTORY_LOADED,
+                                         Source<Profile>(profile_),
+                                         Details<HistoryService>(this));
+}
diff --git a/chrome/browser/history/history.h b/chrome/browser/history/history.h
index ec42335..74ef67b 100644
--- a/chrome/browser/history/history.h
+++ b/chrome/browser/history/history.h
@@ -25,7 +25,7 @@
 #include "chrome/common/page_transition_types.h"
 #include "chrome/common/ref_counted_util.h"
 
-class BookmarkBarModel;
+class BookmarkService;
 struct DownloadCreateInfo;
 class GURL;
 class HistoryURLProvider;
@@ -98,8 +98,12 @@ class HistoryService : public CancelableRequestProvider,
 
   // Initializes the history service, returning true on success. On false, do
   // not call any other functions. The given directory will be used for storing
-  // the history files.
-  bool Init(const std::wstring& history_dir);
+  // the history files. The BookmarkService is used when deleting URLs to
+  // test if a URL is bookmarked; it may be NULL during testing.
+  bool Init(const std::wstring& history_dir, BookmarkService* bookmark_service);
+
+  // Did the backend finish loading the databases?
+  bool backend_loaded() const { return backend_loaded_; }
 
   // Called on shutdown, this will tell the history backend to complete and
   // will release pointers to it. No other functions should be called once
@@ -487,6 +491,11 @@ class HistoryService : public CancelableRequestProvider,
       CancelableRequestConsumerBase* consumer,
       GetMostRecentKeywordSearchTermsCallback* callback);
 
+  // Bookmarks -----------------------------------------------------------------
+
+  // Notification that a URL is no longer bookmarked.
+  void URLsNoLongerBookmarked(const std::set<GURL>& urls);
+
   // Generic Stuff -------------------------------------------------------------
 
   typedef Callback0::Type HistoryDBTaskCallback;
@@ -496,13 +505,6 @@ class HistoryService : public CancelableRequestProvider,
   Handle ScheduleDBTask(HistoryDBTask* task,
                         CancelableRequestConsumerBase* consumer);
 
-  typedef Callback0::Type EmptyHistoryCallback;
-
-  // Schedules a task that does nothing on the backend. This can be used to get
-  // notification when then history backend is done processing requests.
-  Handle ScheduleEmptyCallback(CancelableRequestConsumerBase* consumer,
-                               EmptyHistoryCallback* callback);
-
   // Testing -------------------------------------------------------------------
 
   // Designed for unit tests, this passes the given task on to the history
@@ -536,6 +538,16 @@ class HistoryService : public CancelableRequestProvider,
  private:
   class BackendDelegate;
   friend class BackendDelegate;
+  friend class history::HistoryBackend;
+  friend class history::HistoryQueryTest;
+  friend class HistoryOperation;
+  friend class HistoryURLProvider;
+  friend class HistoryURLProviderTest;
+  template<typename Info, typename Callback> friend class DownloadRequest;
+  friend class PageUsageRequest;
+  friend class RedirectRequest;
+  friend class FavIconRequest;
+  friend class TestingProfile;
 
   // These are not currently used, hopefully we can do something in the future
   // to ensure that the most important things happen first.
@@ -545,17 +557,6 @@ class HistoryService : public CancelableRequestProvider,
     PRIORITY_LOW,     // Low priority things like indexing or expiration.
   };
 
-  friend class BookmarkBarModel;
-  friend class HistoryURLProvider;
-  friend class history::HistoryBackend;
-  template<typename Info, typename Callback> friend class DownloadRequest;
-  friend class PageUsageRequest;
-  friend class RedirectRequest;
-  friend class FavIconRequest;
-  friend class history::HistoryQueryTest;
-  friend class HistoryOperation;
-  friend class HistoryURLProviderTest;
-
   // Implementation of NotificationObserver.
   virtual void Observe(NotificationType type,
                        const NotificationSource& source,
@@ -577,6 +578,10 @@ class HistoryService : public CancelableRequestProvider,
   void BroadcastNotifications(NotificationType type,
                               history::HistoryDetails* details_deleted);
 
+  // Notification from the backend that it has finished loading. Sends
+  // notification (NOTIFY_HISTORY_LOADED) and sets backend_loaded_ to true.
+  void OnDBLoaded();
+
   // Returns true if this looks like the type of URL we want to add to the
   // history. We filter out some URLs such as JavaScript.
   bool CanAddURL(const GURL& url) const;
@@ -747,8 +752,11 @@ class HistoryService : public CancelableRequestProvider,
   // The profile, may be null when testing.
   Profile* profile_;
 
+  // Has the backend finished loading? The backend is loaded once Init has
+  // completed.
+  bool backend_loaded_;
+
   DISALLOW_EVIL_CONSTRUCTORS(HistoryService);
 };
 
 #endif  // CHROME_BROWSER_HISTORY_HISTORY_H__
-
diff --git a/chrome/browser/history/history_backend.cc b/chrome/browser/history/history_backend.cc
index 7682f6074..21f77fb 100644
--- a/chrome/browser/history/history_backend.cc
+++ b/chrome/browser/history/history_backend.cc
@@ -13,6 +13,7 @@
 #include "base/string_util.h"
 #include "base/time.h"
 #include "chrome/browser/autocomplete/history_url_provider.h"
+#include "chrome/browser/bookmarks/bookmark_service.h"
 #include "chrome/browser/history/download_types.h"
 #include "chrome/browser/history/in_memory_history_backend.h"
 #include "chrome/browser/history/page_usage_data.h"
@@ -25,8 +26,7 @@
 /* The HistoryBackend consists of a number of components:
 
     HistoryDatabase (stores past 3 months of history)
-      StarredURLDatabase (stores starred pages)
-        URLDatabase (stores a list of URLs)
+      URLDatabase (stores a list of URLs)
       DownloadDatabase (stores a list of downloads)
       VisitDatabase (stores a list of visits for the URLs)
       VisitSegmentDatabase (stores groups of URLs for the most visited view).
@@ -36,8 +36,7 @@
       DownloadDatabase (stores a list of downloads)
       VisitDatabase (stores a list of visits for the URLs)
 
-      (this does not store starred things or visit segments, all starred info
-      is stored in HistoryDatabase, and visit segments expire after 3 mos.)
+      (this does not store visit segments as they expire after 3 mos.)
 
     TextDatabaseManager (manages multiple text database for different times)
       TextDatabase (represents a single month of full-text index).
@@ -187,15 +186,17 @@ class HistoryBackend::URLQuerier {
 // HistoryBackend --------------------------------------------------------------
 
 HistoryBackend::HistoryBackend(const std::wstring& history_dir,
-                               Delegate* delegate)
+                               Delegate* delegate,
+                               BookmarkService* bookmark_service)
     : delegate_(delegate),
       history_dir_(history_dir),
 #pragma warning(suppress: 4355)  // OK to pass "this" here.
-      expirer_(this),
+      expirer_(this, bookmark_service),
       backend_destroy_message_loop_(NULL),
       recent_redirects_(kMaxRedirectCount),
       backend_destroy_task_(NULL),
-      segment_queried_(false) {
+      segment_queried_(false),
+      bookmark_service_(bookmark_service) {
 }
 
 HistoryBackend::~HistoryBackend() {
@@ -229,103 +230,8 @@ HistoryBackend::~HistoryBackend() {
 }
 
 void HistoryBackend::Init() {
-  DCHECK(!db_.get()) << "Initializing HistoryBackend twice";
-  // In the rare case where the db fails to initialize a dialog may get shown
-  // the blocks the caller, yet allows other messages through. For this reason
-  // we only set db_ to the created database if creation is successful. That
-  // way other methods won't do anything as db_ is still NULL.
-
-  TimeTicks beginning_time = TimeTicks::Now();
-
-  // Compute the file names. Note that the index file can be removed when the
-  // text db manager is finished being hooked up.
-  std::wstring history_name = history_dir_;
-  file_util::AppendToPath(&history_name, chrome::kHistoryFilename);
-  std::wstring thumbnail_name = GetThumbnailFileName();
-  std::wstring archived_name = GetArchivedFileName();
-  std::wstring tmp_bookmarks_file = history_dir_;
-  file_util::AppendToPath(&tmp_bookmarks_file,
-                          chrome::kHistoryBookmarksFileName);
-
-  // History database.
-  db_.reset(new HistoryDatabase());
-  switch (db_->Init(history_name, tmp_bookmarks_file)) {
-    case INIT_OK:
-      break;
-    case INIT_FAILURE:
-      // A NULL db_ will cause all calls on this object to notice this error
-      // and to not continue.
-      LOG(WARNING) << "Unable to initialize history DB.";
-      db_.reset();
-      return;
-    case INIT_TOO_NEW:
-      delegate_->NotifyTooNew();
-      db_.reset();
-      return;
-    default:
-      NOTREACHED();
-  }
-
-  // Fill the in-memory database and send it back to the history service on the
-  // main thread.
-  InMemoryHistoryBackend* mem_backend = new InMemoryHistoryBackend;
-  if (mem_backend->Init(history_name))
-    delegate_->SetInMemoryBackend(mem_backend);  // Takes ownership of pointer.
-  else
-    delete mem_backend;  // Error case, run without the in-memory DB.
-  db_->BeginExclusiveMode();  // Must be after the mem backend read the data.
-
-  // Full-text database. This has to be first so we can pass it to the
-  // HistoryDatabase for migration.
-  text_database_.reset(new TextDatabaseManager(history_dir_, db_.get()));
-  if (!text_database_->Init()) {
-    LOG(WARNING) << "Text database initialization failed, running without it.";
-    text_database_.reset();
-  }
-
-  // Thumbnail database.
-  thumbnail_db_.reset(new ThumbnailDatabase());
-  if (thumbnail_db_->Init(thumbnail_name) != INIT_OK) {
-    // Unlike the main database, we don't error out when the database is too
-    // new because this error is much less severe. Generally, this shouldn't
-    // happen since the thumbnail and main datbase versions should be in sync.
-    // We'll just continue without thumbnails & favicons in this case or any
-    // other error.
-    LOG(WARNING) << "Could not initialize the thumbnail database.";
-    thumbnail_db_.reset();
-  }
-
-  // Archived database.
-  archived_db_.reset(new ArchivedDatabase());
-  if (!archived_db_->Init(archived_name)) {
-    LOG(WARNING) << "Could not initialize the archived database.";
-    archived_db_.reset();
-  }
-
-  // Tell the expiration module about all the nice databases we made. This must
-  // happen before db_->Init() is called since the callback ForceArchiveHistory
-  // may need to expire stuff.
-  //
-  // *sigh*, this can all be cleaned up when that migration code is removed.
-  // The main DB initialization should intuitively be first (not that it
-  // actually matters) and the expirer should be set last.
-  expirer_.SetDatabases(db_.get(), archived_db_.get(),
-                        thumbnail_db_.get(), text_database_.get());
-
-  // Open the long-running transaction.
-  db_->BeginTransaction();
-  if (thumbnail_db_.get())
-    thumbnail_db_->BeginTransaction();
-  if (archived_db_.get())
-    archived_db_->BeginTransaction();
-  if (text_database_.get())
-    text_database_->BeginTransaction();
-
-  // Start expiring old stuff.
-  expirer_.StartArchivingOldStuff(TimeDelta::FromDays(kArchiveDaysThreshold));
-
-  HISTOGRAM_TIMES(L"History.InitTime",
-                  TimeTicks::Now() - beginning_time);
+  InitImpl();
+  delegate_->DBLoaded();
 }
 
 void HistoryBackend::SetOnBackendDestroyTask(MessageLoop* message_loop,
@@ -574,6 +480,106 @@ void HistoryBackend::AddPage(scoped_refptr<HistoryAddPageArgs> request) {
   ScheduleCommit();
 }
 
+void HistoryBackend::InitImpl() {
+  DCHECK(!db_.get()) << "Initializing HistoryBackend twice";
+  // In the rare case where the db fails to initialize a dialog may get shown
+  // the blocks the caller, yet allows other messages through. For this reason
+  // we only set db_ to the created database if creation is successful. That
+  // way other methods won't do anything as db_ is still NULL.
+
+  TimeTicks beginning_time = TimeTicks::Now();
+
+  // Compute the file names. Note that the index file can be removed when the
+  // text db manager is finished being hooked up.
+  std::wstring history_name = history_dir_;
+  file_util::AppendToPath(&history_name, chrome::kHistoryFilename);
+  std::wstring thumbnail_name = GetThumbnailFileName();
+  std::wstring archived_name = GetArchivedFileName();
+  std::wstring tmp_bookmarks_file = history_dir_;
+  file_util::AppendToPath(&tmp_bookmarks_file,
+                          chrome::kHistoryBookmarksFileName);
+
+  // History database.
+  db_.reset(new HistoryDatabase());
+  switch (db_->Init(history_name, tmp_bookmarks_file)) {
+    case INIT_OK:
+      break;
+    case INIT_FAILURE:
+      // A NULL db_ will cause all calls on this object to notice this error
+      // and to not continue.
+      LOG(WARNING) << "Unable to initialize history DB.";
+      db_.reset();
+      return;
+    case INIT_TOO_NEW:
+      delegate_->NotifyTooNew();
+      db_.reset();
+      return;
+    default:
+      NOTREACHED();
+  }
+
+  // Fill the in-memory database and send it back to the history service on the
+  // main thread.
+  InMemoryHistoryBackend* mem_backend = new InMemoryHistoryBackend;
+  if (mem_backend->Init(history_name))
+    delegate_->SetInMemoryBackend(mem_backend);  // Takes ownership of pointer.
+  else
+    delete mem_backend;  // Error case, run without the in-memory DB.
+  db_->BeginExclusiveMode();  // Must be after the mem backend read the data.
+
+  // Full-text database. This has to be first so we can pass it to the
+  // HistoryDatabase for migration.
+  text_database_.reset(new TextDatabaseManager(history_dir_, db_.get()));
+  if (!text_database_->Init()) {
+    LOG(WARNING) << "Text database initialization failed, running without it.";
+    text_database_.reset();
+  }
+
+  // Thumbnail database.
+  thumbnail_db_.reset(new ThumbnailDatabase());
+  if (thumbnail_db_->Init(thumbnail_name) != INIT_OK) {
+    // Unlike the main database, we don't error out when the database is too
+    // new because this error is much less severe. Generally, this shouldn't
+    // happen since the thumbnail and main datbase versions should be in sync.
+    // We'll just continue without thumbnails & favicons in this case or any
+    // other error.
+    LOG(WARNING) << "Could not initialize the thumbnail database.";
+    thumbnail_db_.reset();
+  }
+
+  // Archived database.
+  archived_db_.reset(new ArchivedDatabase());
+  if (!archived_db_->Init(archived_name)) {
+    LOG(WARNING) << "Could not initialize the archived database.";
+    archived_db_.reset();
+  }
+
+  // Tell the expiration module about all the nice databases we made. This must
+  // happen before db_->Init() is called since the callback ForceArchiveHistory
+  // may need to expire stuff.
+  //
+  // *sigh*, this can all be cleaned up when that migration code is removed.
+  // The main DB initialization should intuitively be first (not that it
+  // actually matters) and the expirer should be set last.
+  expirer_.SetDatabases(db_.get(), archived_db_.get(),
+                        thumbnail_db_.get(), text_database_.get());
+
+  // Open the long-running transaction.
+  db_->BeginTransaction();
+  if (thumbnail_db_.get())
+    thumbnail_db_->BeginTransaction();
+  if (archived_db_.get())
+    archived_db_->BeginTransaction();
+  if (text_database_.get())
+    text_database_->BeginTransaction();
+
+  // Start expiring old stuff.
+  expirer_.StartArchivingOldStuff(TimeDelta::FromDays(kArchiveDaysThreshold));
+
+  HISTOGRAM_TIMES(L"History.InitTime",
+                  TimeTicks::Now() - beginning_time);
+}
+
 std::pair<URLID, VisitID> HistoryBackend::AddPageVisit(
     const GURL& url,
     Time time,
@@ -1052,8 +1058,7 @@ void HistoryBackend::QueryHistoryFTS(const std::wstring& text_query,
 
   URLQuerier querier(db_.get(), archived_db_.get(), true);
 
-  // Now get the row and visit information for each one, optionally
-  // filtering on starred items.
+  // Now get the row and visit information for each one.
   URLResult url_result;  // Declare outside loop to prevent re-construction.
   for (size_t i = 0; i < fts_matches.size(); i++) {
     if (options.max_count != 0 &&
@@ -1291,7 +1296,7 @@ void HistoryBackend::SetImportedFavicons(
 
   Time now = Time::Now();
 
-  // Track all starred URLs that had their favicons set or updated.
+  // Track all URLs that had their favicons set or updated.
   std::set<GURL> favicons_changed;
 
   for (size_t i = 0; i < favicon_usage.size(); i++) {
@@ -1320,7 +1325,7 @@ void HistoryBackend::SetImportedFavicons(
   }
 
   if (!favicons_changed.empty()) {
-    // Send the notification about the changed favicons for starred URLs.
+    // Send the notification about the changed favicon URLs.
     FavIconChangeDetails* changed_details = new FavIconChangeDetails;
     changed_details->urls.swap(favicons_changed);
     BroadcastNotifications(NOTIFY_FAVICON_CHANGED, changed_details);
@@ -1602,6 +1607,23 @@ void HistoryBackend::ExpireHistoryBetween(
   request->ForwardResult(ExpireHistoryRequest::TupleType());
 }
 
+void HistoryBackend::URLsNoLongerBookmarked(const std::set<GURL>& urls) {
+  if (!db_.get())
+    return;
+
+  for (std::set<GURL>::const_iterator i = urls.begin(); i != urls.end(); ++i) {
+    URLRow url_row;
+    if (!db_->GetRowForURL(*i, &url_row))
+      continue;  // The URL isn't in the db; nothing to do.
+
+    VisitVector visits;
+    db_->GetVisitsForURL(url_row.id(), &visits);
+
+    if (visits.empty())
+      expirer_.DeleteURL(*i);  // There are no more visits; nuke the URL.
+  }
+}
+
 void HistoryBackend::ProcessDBTask(
     scoped_refptr<HistoryDBTaskRequest> request) {
   DCHECK(request.get());
@@ -1619,11 +1641,6 @@ void HistoryBackend::ProcessDBTask(
   }
 }
 
-void HistoryBackend::ProcessEmptyRequest(
-    scoped_refptr<EmptyHistoryRequest> request) {
-  request->ForwardResult(EmptyHistoryRequest::TupleType());
-}
-
 void HistoryBackend::BroadcastNotifications(
     NotificationType type,
     HistoryDetails* details_deleted) {
@@ -1645,29 +1662,23 @@ void HistoryBackend::DeleteAllHistory() {
   // Since we are likely to have very few bookmarks and their dependencies
   // compared to all history, this is also much faster than just deleting from
   // the original tables directly.
-  //
-  // TODO(brettw): bug 989802: When we store bookmarks in a separate file, this
-  // function can be simplified to having all the database objects close their
-  // connections and we just delete the files.
 
-  // Get starred entries and their corresponding URL rows.
-  std::vector<StarredEntry> starred_entries;
+  // Get the bookmarked URLs.
+  std::vector<GURL> starred_urls;
+  BookmarkService* bookmark_service = GetBookmarkService();
+  if (bookmark_service)
+    bookmark_service_->GetBookmarks(&starred_urls);
 
   std::vector<URLRow> kept_urls;
-  for (size_t i = 0; i < starred_entries.size(); i++) {
-    if (starred_entries[i].type != StarredEntry::URL)
-      continue;
-
+  for (size_t i = 0; i < starred_urls.size(); i++) {
     URLRow row;
-    if (!db_->GetURLRow(starred_entries[i].url_id, &row))
+    if (!db_->GetRowForURL(starred_urls[i], &row))
       continue;
 
     // Clear the last visit time so when we write these rows they are "clean."
-    // We keep the typed and visit counts. Since the kept URLs are bookmarks,
-    // we can assume that the user isn't trying to hide that they like them,
-    // and we can use these counts for giving better autocomplete suggestions.
     row.set_last_visit(Time());
-
+    row.set_visit_count(0);
+    row.set_typed_count(0);
     kept_urls.push_back(row);
   }
 
@@ -1681,7 +1692,7 @@ void HistoryBackend::DeleteAllHistory() {
 
   // ClearAllMainHistory will change the IDs of the URLs in kept_urls. Therfore,
   // we clear the list afterwards to make sure nobody uses this invalid data.
-  if (!ClearAllMainHistory(&starred_entries, kept_urls))
+  if (!ClearAllMainHistory(kept_urls))
     LOG(ERROR) << "Main history could not be cleared";
   kept_urls.clear();
 
@@ -1775,7 +1786,6 @@ bool HistoryBackend::ClearAllThumbnailHistory(
 }
 
 bool HistoryBackend::ClearAllMainHistory(
-    std::vector<StarredEntry>* starred_entries,
     const std::vector<URLRow>& kept_urls) {
   // Create the duplicate URL table. We will copy the kept URLs into this.
   if (!db_->CreateTemporaryURLTable())
@@ -1797,7 +1807,7 @@ bool HistoryBackend::ClearAllMainHistory(
     return false;
 
   // Delete the old tables and recreate them empty.
-  db_->RecreateAllButStarAndURLTables();
+  db_->RecreateAllTablesButURL();
 
   // Vacuum to reclaim the space from the dropped tables. This must be done
   // when there is no transaction open, and we assume that our long-running
@@ -1808,5 +1818,11 @@ bool HistoryBackend::ClearAllMainHistory(
   return true;
 }
 
+BookmarkService* HistoryBackend::GetBookmarkService() {
+  if (bookmark_service_)
+    bookmark_service_->BlockTillLoaded();
+  return bookmark_service_;
+}
+
 }  // namespace history
 
diff --git a/chrome/browser/history/history_backend.h b/chrome/browser/history/history_backend.h
index 1143cef..dd37c3e 100644
--- a/chrome/browser/history/history_backend.h
+++ b/chrome/browser/history/history_backend.h
@@ -25,6 +25,7 @@
 #include "chrome/common/scoped_vector.h"
 #include "testing/gtest/include/gtest/gtest_prod.h"
 
+class BookmarkService;
 struct ThumbnailScore;
 
 namespace history {
@@ -75,6 +76,9 @@ class HistoryBackend : public base::RefCountedThreadSafe<HistoryBackend>,
     // Ownership of the HistoryDetails is transferred to this function.
     virtual void BroadcastNotifications(NotificationType type,
                                         HistoryDetails* details) = 0;
+
+    // Invoked when the backend has finished loading the db.
+    virtual void DBLoaded() = 0;
   };
 
   // Init must be called to complete object creation. This object can be
@@ -85,8 +89,13 @@ class HistoryBackend : public base::RefCountedThreadSafe<HistoryBackend>,
   // See the definition of BroadcastNotificationsCallback above. This function
   // takes ownership of the callback pointer.
   //
+  // |bookmark_service| is used to determine bookmarked URLs when deleting and
+  // may be NULL.
+  //
   // This constructor is fast and does no I/O, so can be called at any time.
-  HistoryBackend(const std::wstring& history_dir, Delegate* delegate);
+  HistoryBackend(const std::wstring& history_dir,
+                 Delegate* delegate,
+                 BookmarkService* bookmark_service);
 
   ~HistoryBackend();
 
@@ -218,8 +227,6 @@ class HistoryBackend : public base::RefCountedThreadSafe<HistoryBackend>,
 
   void ProcessDBTask(scoped_refptr<HistoryDBTaskRequest> request);
 
-  void ProcessEmptyRequest(scoped_refptr<EmptyHistoryRequest> request);
-
   // Deleting ------------------------------------------------------------------
 
   void DeleteURL(const GURL& url);
@@ -229,6 +236,12 @@ class HistoryBackend : public base::RefCountedThreadSafe<HistoryBackend>,
                             Time begin_time,
                             Time end_time);
 
+  // Bookmarks -----------------------------------------------------------------
+
+  // Notification that a URL is no longer bookmarked. If there are no visits
+  // for the specified url, it is deleted.
+  void URLsNoLongerBookmarked(const std::set<GURL>& urls);
+
   // Testing -------------------------------------------------------------------
 
   // Sets the task to run and the message loop to run it on when this object
@@ -244,6 +257,8 @@ class HistoryBackend : public base::RefCountedThreadSafe<HistoryBackend>,
   friend class CommitLaterTask;  // The commit task needs to call Commit().
   friend class HistoryTest;  // So the unit tests can poke our innards.
   FRIEND_TEST(HistoryBackendTest, DeleteAll);
+  FRIEND_TEST(HistoryBackendTest, URLsNoLongerBookmarked);
+  friend class TestingProfile;
 
   // For invoking methods that circumvent requests.
   friend class HistoryTest;
@@ -255,6 +270,9 @@ class HistoryBackend : public base::RefCountedThreadSafe<HistoryBackend>,
   class URLQuerier;
   friend class URLQuerier;
 
+  // Does the work of Init.
+  void InitImpl();
+
   // Adds a single visit to the database, updating the URL information such
   // as visit and typed count. The visit ID of the added visit and the URL ID
   // of the associated URL (whether added or not) is returned. Both values will
@@ -374,15 +392,16 @@ class HistoryBackend : public base::RefCountedThreadSafe<HistoryBackend>,
   // vector to reference the new IDs.
   bool ClearAllThumbnailHistory(std::vector<URLRow>* kept_urls);
 
-  // Deletes all information in the history database, except the star table
-  // (all entries should be in the given vector) and the given URLs in the URL
-  // table (these should correspond to the bookmarked URLs).
+  // Deletes all information in the history database, except for the supplied
+  // set of URLs in the URL table (these should correspond to the bookmarked
+  // URLs).
   //
-  // The IDs of the URLs may change, and the starred table will be updated
-  // accordingly. This function will also update the |starred_entries| input
-  // vector.
-  bool ClearAllMainHistory(std::vector<StarredEntry>* starred_entries,
-                           const std::vector<URLRow>& kept_urls);
+  // The IDs of the URLs may change.
+  bool ClearAllMainHistory(const std::vector<URLRow>& kept_urls);
+
+  // Returns the BookmarkService, blocking until it is loaded. This may return
+  // NULL during testing.
+  BookmarkService* GetBookmarkService();
 
   // Data ----------------------------------------------------------------------
 
@@ -457,10 +476,16 @@ class HistoryBackend : public base::RefCountedThreadSafe<HistoryBackend>,
   // done.
   std::list<HistoryDBTaskRequest*> db_task_requests_;
 
+  // Used to determine if a URL is bookmarked. This is owned by the Profile and
+  // may be NULL (during testing).
+  //
+  // Use GetBookmarkService to access this, which makes sure the service is
+  // loaded.
+  BookmarkService* bookmark_service_;
+
   DISALLOW_EVIL_CONSTRUCTORS(HistoryBackend);
 };
 
 }  // namespace history
 
 #endif  // CHROME_BROWSER_HISTORY_HISTORY_BACKEND_H__
-
diff --git a/chrome/browser/history/history_backend_unittest.cc b/chrome/browser/history/history_backend_unittest.cc
index baf5f0e..4067440 100644
--- a/chrome/browser/history/history_backend_unittest.cc
+++ b/chrome/browser/history/history_backend_unittest.cc
@@ -5,6 +5,7 @@
 #include "base/file_util.h"
 #include "base/path_service.h"
 #include "base/scoped_ptr.h"
+#include "chrome/browser/bookmark_bar_model.h"
 #include "chrome/browser/history/history_backend.h"
 #include "chrome/browser/history/in_memory_history_backend.h"
 #include "chrome/browser/history/in_memory_database.h"
@@ -34,6 +35,7 @@ class HistoryBackendTestDelegate : public HistoryBackend::Delegate {
   virtual void SetInMemoryBackend(InMemoryHistoryBackend* backend);
   virtual void BroadcastNotifications(NotificationType type,
                                       HistoryDetails* details);
+  virtual void DBLoaded();
 
  private:
   // Not owned by us.
@@ -44,6 +46,7 @@ class HistoryBackendTestDelegate : public HistoryBackend::Delegate {
 
 class HistoryBackendTest : public testing::Test {
  public:
+  HistoryBackendTest() : bookmark_model_(NULL), loaded_(false) {}
   virtual ~HistoryBackendTest() {
   }
 
@@ -66,6 +69,11 @@ class HistoryBackendTest : public testing::Test {
     backend_->AddPage(request);
   }
 
+  BookmarkBarModel bookmark_model_;
+
+ protected:
+  bool loaded_;
+
  private:
   friend HistoryBackendTestDelegate;
 
@@ -74,7 +82,8 @@ class HistoryBackendTest : public testing::Test {
     if (!file_util::CreateNewTempDirectory(L"BackendTest", &test_dir_))
       return;
     backend_ = new HistoryBackend(test_dir_,
-                                  new HistoryBackendTestDelegate(this));
+                                  new HistoryBackendTestDelegate(this),
+                                  &bookmark_model_);
     backend_->Init();
   }
   virtual void TearDown() {
@@ -113,6 +122,14 @@ void HistoryBackendTestDelegate::BroadcastNotifications(
   test_->BroadcastNotifications(type, details);
 }
 
+void HistoryBackendTestDelegate::DBLoaded() {
+  test_->loaded_ = true;
+}
+
+TEST_F(HistoryBackendTest, Loaded) {
+  ASSERT_TRUE(backend_.get());
+  ASSERT_TRUE(loaded_);
+}
 
 TEST_F(HistoryBackendTest, DeleteAll) {
   ASSERT_TRUE(backend_.get());
@@ -181,6 +198,10 @@ TEST_F(HistoryBackendTest, DeleteAll) {
      JPEGCodec::Decode(kWeewarThumbnail, sizeof(kWeewarThumbnail)));
   backend_->thumbnail_db_->SetPageThumbnail(row2_id, *weewar_bitmap, score);
 
+  // Star row1.
+  bookmark_model_.AddURL(
+      bookmark_model_.GetBookmarkBarNode(), 0, std::wstring(), row1.url());
+
   // Set full text index for each one.
   backend_->text_database_->AddPageData(row1.url(), row1_id, visit1_id,
                                         row1.last_visit(),
@@ -192,8 +213,11 @@ TEST_F(HistoryBackendTest, DeleteAll) {
   // Now finally clear all history.
   backend_->DeleteAllHistory();
 
-  // The first URL should be deleted.
-  EXPECT_FALSE(backend_->db_->GetRowForURL(row1.url(), &outrow1));
+  // The first URL should be preserved but the time should be cleared.
+  EXPECT_TRUE(backend_->db_->GetRowForURL(row1.url(), &outrow1));
+  EXPECT_EQ(0, outrow1.visit_count());
+  EXPECT_EQ(0, outrow1.typed_count());
+  EXPECT_TRUE(Time() == outrow1.last_visit());
 
   // The second row should be deleted.
   URLRow outrow2;
@@ -210,15 +234,22 @@ TEST_F(HistoryBackendTest, DeleteAll) {
                                                          &out_data));
   EXPECT_FALSE(backend_->thumbnail_db_->GetPageThumbnail(row2_id, &out_data));
 
-  // Make sure the favicons were deleted.
-  // TODO(sky): would be nice if this didn't happen.
+  // We should have a favicon for the first URL only. We look them up by favicon
+  // URL since the IDs may hav changed.
   FavIconID out_favicon1 = backend_->thumbnail_db_->
       GetFavIconIDForFavIconURL(favicon_url1);
-  EXPECT_FALSE(out_favicon1);
+  EXPECT_TRUE(out_favicon1);
   FavIconID out_favicon2 = backend_->thumbnail_db_->
       GetFavIconIDForFavIconURL(favicon_url2);
   EXPECT_FALSE(out_favicon2) << "Favicon not deleted";
 
+  // The remaining URL should still reference the same favicon, even if its
+  // ID has changed.
+  EXPECT_EQ(out_favicon1, outrow1.favicon_id());
+
+  // The first URL should still be bookmarked.
+  EXPECT_TRUE(bookmark_model_.IsBookmarked(row1.url()));
+
   // The full text database should have no data.
   std::vector<TextDatabase::Match> text_matches;
   Time first_time_searched;
@@ -228,6 +259,94 @@ TEST_F(HistoryBackendTest, DeleteAll) {
   EXPECT_EQ(0, text_matches.size());
 }
 
+TEST_F(HistoryBackendTest, URLsNoLongerBookmarked) {
+  GURL favicon_url1("http://www.google.com/favicon.ico");
+  GURL favicon_url2("http://news.google.com/favicon.ico");
+  FavIconID favicon2 = backend_->thumbnail_db_->AddFavIcon(favicon_url2);
+  FavIconID favicon1 = backend_->thumbnail_db_->AddFavIcon(favicon_url1);
+
+  std::vector<unsigned char> data;
+  data.push_back('1');
+  EXPECT_TRUE(backend_->thumbnail_db_->SetFavIcon(
+                  favicon1, data, Time::Now()));
+
+  data[0] = '2';
+  EXPECT_TRUE(backend_->thumbnail_db_->SetFavIcon(
+                  favicon2, data, Time::Now()));
+
+  // First visit two URLs.
+  URLRow row1(GURL("http://www.google.com/"));
+  row1.set_visit_count(2);
+  row1.set_typed_count(1);
+  row1.set_last_visit(Time::Now());
+  row1.set_favicon_id(favicon1);
+
+  URLRow row2(GURL("http://news.google.com/"));
+  row2.set_visit_count(1);
+  row2.set_last_visit(Time::Now());
+  row2.set_favicon_id(favicon2);
+
+  std::vector<URLRow> rows;
+  rows.push_back(row2);  // Reversed order for the same reason as favicons.
+  rows.push_back(row1);
+  backend_->AddPagesWithDetails(rows);
+
+  URLID row1_id = backend_->db_->GetRowForURL(row1.url(), NULL);
+  URLID row2_id = backend_->db_->GetRowForURL(row2.url(), NULL);
+
+  // Star the two URLs.
+  bookmark_model_.SetURLStarred(row1.url(), std::wstring(), true);
+  bookmark_model_.SetURLStarred(row2.url(), std::wstring(), true);
+
+  // Delete url 2. Because url 2 is starred this won't delete the URL, only
+  // the visits.
+  backend_->expirer_.DeleteURL(row2.url());
+
+  // Make sure url 2 is still valid, but has no visits.
+  URLRow tmp_url_row;
+  EXPECT_EQ(row2_id, backend_->db_->GetRowForURL(row2.url(), NULL));
+  VisitVector visits;
+  backend_->db_->GetVisitsForURL(row2_id, &visits);
+  EXPECT_EQ(0, visits.size());
+  // The favicon should still be valid.
+  EXPECT_EQ(favicon2,
+      backend_->thumbnail_db_->GetFavIconIDForFavIconURL(favicon_url2));
+
+  // Unstar row2.
+  bookmark_model_.SetURLStarred(row2.url(), std::wstring(), false);
+  // Tell the backend it was unstarred. We have to explicitly do this as
+  // BookmarkBarModel isn't wired up to the backend during testing.
+  std::set<GURL> unstarred_urls;
+  unstarred_urls.insert(row2.url());
+  backend_->URLsNoLongerBookmarked(unstarred_urls);
+
+  // The URL should no longer exist.
+  EXPECT_FALSE(backend_->db_->GetRowForURL(row2.url(), &tmp_url_row));
+  // And the favicon should be deleted.
+  EXPECT_EQ(0,
+      backend_->thumbnail_db_->GetFavIconIDForFavIconURL(favicon_url2));
+
+  // Unstar row 1.
+  bookmark_model_.SetURLStarred(row1.url(), std::wstring(), false);
+  // Tell the backend it was unstarred. We have to explicitly do this as
+  // BookmarkBarModel isn't wired up to the backend during testing.
+  unstarred_urls.clear();
+  unstarred_urls.insert(row1.url());
+  backend_->URLsNoLongerBookmarked(unstarred_urls);
+
+  // The URL should still exist (because there were visits).
+  EXPECT_EQ(row1_id, backend_->db_->GetRowForURL(row1.url(), NULL));
+
+  // There should still be visits.
+  visits.clear();
+  backend_->db_->GetVisitsForURL(row1_id, &visits);
+  EXPECT_EQ(1, visits.size());
+
+  // The favicon should still be valid.
+  EXPECT_EQ(favicon1,
+      backend_->thumbnail_db_->GetFavIconIDForFavIconURL(favicon_url1));
+}
+
 TEST_F(HistoryBackendTest, GetPageThumbnailAfterRedirects) {
   ASSERT_TRUE(backend_.get());
 
diff --git a/chrome/browser/history/history_database.cc b/chrome/browser/history/history_database.cc
index c33dee8..cf49883 100644
--- a/chrome/browser/history/history_database.cc
+++ b/chrome/browser/history/history_database.cc
@@ -127,7 +127,7 @@ void HistoryDatabase::CommitTransaction() {
   }
 }
 
-bool HistoryDatabase::RecreateAllButStarAndURLTables() {
+bool HistoryDatabase::RecreateAllTablesButURL() {
   if (!DropVisitTable())
     return false;
   if (!InitVisitTable())
@@ -143,7 +143,7 @@ bool HistoryDatabase::RecreateAllButStarAndURLTables() {
   if (!InitSegmentTables())
     return false;
 
-  // We also add the supplimentary URL indices at this point. This index is
+  // We also add the supplementary URL indices at this point. This index is
   // over parts of the URL table that weren't automatically created when the
   // temporary URL table was
   CreateSupplimentaryURLIndices();
diff --git a/chrome/browser/history/history_database.h b/chrome/browser/history/history_database.h
index edde249..c69150b 100644
--- a/chrome/browser/history/history_database.h
+++ b/chrome/browser/history/history_database.h
@@ -88,8 +88,8 @@ class HistoryDatabase : public DownloadDatabase,
     return transaction_nesting_;
   }
 
-  // Drops all tables except the URL, starred and download tables, and recreates
-  // them from scratch. This is done to rapidly clean up stuff when deleting all
+  // Drops all tables except the URL, and download tables, and recreates them
+  // from scratch. This is done to rapidly clean up stuff when deleting all
   // history. It is faster and less likely to have problems that deleting all
   // rows in the tables.
   //
@@ -102,10 +102,10 @@ class HistoryDatabase : public DownloadDatabase,
   // This should be treated the same as an init failure, and the database
   // should not be used any more.
   //
-  // This will also recreate the supplimentary URL indices, since these
+  // This will also recreate the supplementary URL indices, since these
   // indices won't be created automatically when using the temporary URL
-  // talbe (what the caller does right before calling this).
-  bool RecreateAllButStarAndURLTables();
+  // table (what the caller does right before calling this).
+  bool RecreateAllTablesButURL();
 
   // Vacuums the database. This will cause sqlite to defragment and collect
   // unused space in the file. It can be VERY SLOW.
diff --git a/chrome/browser/history/history_marshaling.h b/chrome/browser/history/history_marshaling.h
index 146e6d4..fa02095 100644
--- a/chrome/browser/history/history_marshaling.h
+++ b/chrome/browser/history/history_marshaling.h
@@ -117,9 +117,6 @@ typedef CancelableRequest1<HistoryService::HistoryDBTaskCallback,
                            scoped_refptr<HistoryDBTask> >
     HistoryDBTaskRequest;
 
-typedef CancelableRequest<HistoryService::EmptyHistoryCallback>
-    EmptyHistoryRequest;
-
 }  // namespace history
 
 #endif  // CHROME_BROWSER_HISTORY_HISTORY_MARSHALING_H__
diff --git a/chrome/browser/history/history_querying_unittest.cc b/chrome/browser/history/history_querying_unittest.cc
index c62fc40..625bc5b 100644
--- a/chrome/browser/history/history_querying_unittest.cc
+++ b/chrome/browser/history/history_querying_unittest.cc
@@ -86,7 +86,7 @@ class HistoryQueryTest : public testing::Test {
     file_util::CreateDirectory(history_dir_);
 
     history_ = new HistoryService;
-    if (!history_->Init(history_dir_)) {
+    if (!history_->Init(history_dir_, NULL)) {
       history_ = NULL;  // Tests should notice this NULL ptr & fail.
       return;
     }
diff --git a/chrome/browser/history/history_unittest.cc b/chrome/browser/history/history_unittest.cc
index 29f674e..c2ec919 100644
--- a/chrome/browser/history/history_unittest.cc
+++ b/chrome/browser/history/history_unittest.cc
@@ -90,6 +90,7 @@ class BackendDelegate : public HistoryBackend::Delegate {
   virtual void SetInMemoryBackend(InMemoryHistoryBackend* backend);
   virtual void BroadcastNotifications(NotificationType type,
                                       HistoryDetails* details);
+  virtual void DBLoaded() {}
 
  private:
   HistoryTest* history_test_;
@@ -122,7 +123,8 @@ class HistoryTest : public testing::Test {
   // Creates the HistoryBackend and HistoryDatabase on the current thread,
   // assigning the values to backend_ and db_.
   void CreateBackendAndDatabase() {
-    backend_ = new HistoryBackend(history_dir_, new BackendDelegate(this));
+    backend_ =
+        new HistoryBackend(history_dir_, new BackendDelegate(this), NULL);
     backend_->Init();
     db_ = backend_->db_.get();
     DCHECK(in_mem_backend_.get()) << "Mem backend should have been set by "
@@ -373,7 +375,7 @@ TEST_F(HistoryTest, ClearBrowsingData_Downloads) {
 TEST_F(HistoryTest, AddPage) {
   scoped_refptr<HistoryService> history(new HistoryService);
   history_service_ = history;
-  ASSERT_TRUE(history->Init(history_dir_));
+  ASSERT_TRUE(history->Init(history_dir_, NULL));
 
   // Add the page once from a child frame.
   const GURL test_url("http://www.google.com/");
@@ -397,7 +399,7 @@ TEST_F(HistoryTest, AddPage) {
 TEST_F(HistoryTest, AddPageSameTimes) {
   scoped_refptr<HistoryService> history(new HistoryService);
   history_service_ = history;
-  ASSERT_TRUE(history->Init(history_dir_));
+  ASSERT_TRUE(history->Init(history_dir_, NULL));
 
   Time now = Time::Now();
   const GURL test_urls[] = {
@@ -437,7 +439,7 @@ TEST_F(HistoryTest, AddPageSameTimes) {
 TEST_F(HistoryTest, AddRedirect) {
   scoped_refptr<HistoryService> history(new HistoryService);
   history_service_ = history;
-  ASSERT_TRUE(history->Init(history_dir_));
+  ASSERT_TRUE(history->Init(history_dir_, NULL));
 
   const wchar_t* first_sequence[] = {
     L"http://first.page/",
@@ -508,7 +510,7 @@ TEST_F(HistoryTest, AddRedirect) {
 TEST_F(HistoryTest, Typed) {
   scoped_refptr<HistoryService> history(new HistoryService);
   history_service_ = history;
-  ASSERT_TRUE(history->Init(history_dir_));
+  ASSERT_TRUE(history->Init(history_dir_, NULL));
 
   // Add the page once as typed.
   const GURL test_url("http://www.google.com/");
@@ -551,7 +553,7 @@ TEST_F(HistoryTest, Typed) {
 TEST_F(HistoryTest, SetTitle) {
   scoped_refptr<HistoryService> history(new HistoryService);
   history_service_ = history;
-  ASSERT_TRUE(history->Init(history_dir_));
+  ASSERT_TRUE(history->Init(history_dir_, NULL));
 
   // Add a URL.
   const GURL existing_url(L"http://www.google.com/");
@@ -582,7 +584,7 @@ TEST_F(HistoryTest, Segments) {
   scoped_refptr<HistoryService> history(new HistoryService);
   history_service_ = history;
 
-  ASSERT_TRUE(history->Init(history_dir_));
+  ASSERT_TRUE(history->Init(history_dir_, NULL));
 
   static const void* scope = static_cast<void*>(this);
 
@@ -648,7 +650,7 @@ TEST_F(HistoryTest, Segments) {
 TEST_F(HistoryTest, Thumbnails) {
   scoped_refptr<HistoryService> history(new HistoryService);
   history_service_ = history;
-  ASSERT_TRUE(history->Init(history_dir_));
+  ASSERT_TRUE(history->Init(history_dir_, NULL));
 
   scoped_ptr<SkBitmap> thumbnail(
       JPEGCodec::Decode(kGoogleThumbnail, sizeof(kGoogleThumbnail)));
@@ -798,7 +800,7 @@ class HistoryDBTaskImpl : public HistoryDBTask {
 TEST_F(HistoryTest, HistoryDBTask) {
   CancelableRequestConsumerT<int, 0> request_consumer;
   HistoryService* history = new HistoryService();
-  ASSERT_TRUE(history->Init(history_dir_));
+  ASSERT_TRUE(history->Init(history_dir_, NULL));
   scoped_refptr<HistoryDBTaskImpl> task(new HistoryDBTaskImpl());
   history_service_ = history;
   history->ScheduleDBTask(task.get(), &request_consumer);
@@ -816,7 +818,7 @@ TEST_F(HistoryTest, HistoryDBTask) {
 TEST_F(HistoryTest, HistoryDBTaskCanceled) {
   CancelableRequestConsumerT<int, 0> request_consumer;
   HistoryService* history = new HistoryService();
-  ASSERT_TRUE(history->Init(history_dir_));
+  ASSERT_TRUE(history->Init(history_dir_, NULL));
   scoped_refptr<HistoryDBTaskImpl> task(new HistoryDBTaskImpl());
   history_service_ = history;
   history->ScheduleDBTask(task.get(), &request_consumer);
diff --git a/chrome/browser/history/url_database.cc b/chrome/browser/history/url_database.cc
index 27a4994..aeb9ed2 100644
--- a/chrome/browser/history/url_database.cc
+++ b/chrome/browser/history/url_database.cc
@@ -224,8 +224,9 @@ bool URLDatabase::IsFavIconUsed(FavIconID favicon_id) {
 void URLDatabase::AutocompleteForPrefix(const std::wstring& prefix,
                                         size_t max_results,
                                         std::vector<history::URLRow>* results) {
-  // TODO(sky): this query should order by starred as the second order by
-  // clause.
+  // NOTE: this query originally sorted by starred as the second parameter. But
+  // as bookmarks is no longer part of the db we no longer include the order
+  // by clause.
   results->clear();
   SQLITE_UNIQUE_STATEMENT(statement, GetStatementCache(),
       "SELECT" HISTORY_URL_ROW_FIELDS "FROM urls "
diff --git a/chrome/browser/profile.cc b/chrome/browser/profile.cc
index e974e2a..9a64b40 100644
--- a/chrome/browser/profile.cc
+++ b/chrome/browser/profile.cc
@@ -617,6 +617,14 @@ ProfileImpl::~ProfileImpl() {
     request_context_ = NULL;
   }
 
+  // HistoryService may call into the BookmarkBarModel, as such we need to
+  // delete HistoryService before the BookmarkBarModel. The destructor for
+  // HistoryService will join with HistoryService's backend thread so that
+  // by the time the destructor has finished we're sure it will no longer call
+  // into the BookmarkBarModel.
+  history_service_ = NULL;
+  bookmark_bar_model_.reset();
+
   MarkAsCleanShutdown();
 }
 
@@ -721,11 +729,11 @@ URLRequestContext* ProfileImpl::GetRequestContext() {
 
 HistoryService* ProfileImpl::GetHistoryService(ServiceAccessType sat) {
   if (!history_service_created_) {
+    history_service_created_ = true;
     scoped_refptr<HistoryService> history(new HistoryService(this));
-    if (!history->Init(GetPath()))
+    if (!history->Init(GetPath(), GetBookmarkBarModel()))
       return NULL;
     history_service_.swap(history);
-    history_service_created_ = true;
 
     // Send out the notification that the history service was created.
     NotificationService::current()->
@@ -844,8 +852,10 @@ bool ProfileImpl::HasBookmarkBarModel() {
 }
 
 BookmarkBarModel* ProfileImpl::GetBookmarkBarModel() {
-  if (!bookmark_bar_model_.get())
+  if (!bookmark_bar_model_.get()) {
     bookmark_bar_model_.reset(new BookmarkBarModel(this));
+    bookmark_bar_model_->Load();
+  }
   return bookmark_bar_model_.get();
 }
 
diff --git a/chrome/browser/views/bookmark_bar_view_test.cc b/chrome/browser/views/bookmark_bar_view_test.cc
index fb24dbf..b6571c6 100644
--- a/chrome/browser/views/bookmark_bar_view_test.cc
+++ b/chrome/browser/views/bookmark_bar_view_test.cc
@@ -73,7 +73,7 @@ class BookmarkBarViewEventTestBase : public ViewEventTestBase {
 
     profile_.reset(new TestingProfile());
     profile_->set_has_history_service(true);
-    profile_->CreateBookmarkBarModel();
+    profile_->CreateBookmarkBarModel(true);
     profile_->GetPrefs()->SetBoolean(prefs::kShowBookmarkBar, true);
 
     model_ = profile_->GetBookmarkBarModel();
diff --git a/chrome/browser/views/bookmark_editor_view_unittest.cc b/chrome/browser/views/bookmark_editor_view_unittest.cc
index 93a99bf..7838113 100644
--- a/chrome/browser/views/bookmark_editor_view_unittest.cc
+++ b/chrome/browser/views/bookmark_editor_view_unittest.cc
@@ -21,7 +21,7 @@ class BookmarkEditorViewTest : public testing::Test {
   virtual void SetUp() {
     profile_.reset(new TestingProfile());
     profile_->set_has_history_service(true);
-    profile_->CreateBookmarkBarModel();
+    profile_->CreateBookmarkBarModel(true);
 
     model_ = profile_->GetBookmarkBarModel();
 
diff --git a/chrome/browser/visitedlink_unittest.cc b/chrome/browser/visitedlink_unittest.cc
index afad81c..7f082b7 100644
--- a/chrome/browser/visitedlink_unittest.cc
+++ b/chrome/browser/visitedlink_unittest.cc
@@ -49,7 +49,7 @@ class VisitedLinkTest : public testing::Test {
   // Initialize the history system. This should be called before InitVisited().
   bool InitHistory() {
     history_service_ = new HistoryService;
-    return history_service_->Init(history_dir_);
+    return history_service_->Init(history_dir_, NULL);
   }
 
   // Initializes the visited link objects. Pass in the size that you want a