Fixes crash in BookmarkBarView. If the HistoryService fails to load it

was possible for BookmarkBarView to deref NULL (the model_
field). There is no point in having the BookmarkBarView wait for
history to load now though, so I've taken this out which makes it
impossible to get in this situation.

BUG=1356168
TEST=make sure bookmarks still work correctly

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

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

5 files changed, 27 insertions, 60 deletions

diff --git a/chrome/browser/profile.cc b/chrome/browser/profile.cc
index b55a065..3ceef3a 100644
--- a/chrome/browser/profile.cc
+++ b/chrome/browser/profile.cc
@@ -380,10 +380,6 @@ class OffTheRecordProfileImpl : public Profile,
     }
   }
 
-  virtual bool HasHistoryService() const {
-    return profile_->HasHistoryService();
-  }
-
   virtual WebDataService* GetWebDataService(ServiceAccessType sat) {
     if (sat == EXPLICIT_ACCESS) {
       return profile_->GetWebDataService(sat);
@@ -740,10 +736,6 @@ HistoryService* ProfileImpl::GetHistoryService(ServiceAccessType sat) {
   return history_service_.get();
 }
 
-bool ProfileImpl::HasHistoryService() const {
-  return !!history_service_.get();
-}
-
 TemplateURLModel* ProfileImpl::GetTemplateURLModel() {
   if (!template_url_model_.get())
     template_url_model_.reset(new TemplateURLModel(this));
diff --git a/chrome/browser/profile.h b/chrome/browser/profile.h
index 238ce41..9e5adf2 100644
--- a/chrome/browser/profile.h
+++ b/chrome/browser/profile.h
@@ -110,14 +110,6 @@ class Profile {
   // the ServiceAccessType definition above.
   virtual HistoryService* GetHistoryService(ServiceAccessType access) = 0;
 
-  // Returns true if the history service has been initialized. Some callers may
-  // want to use the history service for not very important things, and do not
-  // want to be the ones who cause lazy initialization of the service (can cause
-  // startup regressions).
-  //
-  // If this returns true, history_service() is guaranteed to return non-NULL.
-  virtual bool HasHistoryService() const = 0;
-
   // Returns the WebDataService for this profile. This is owned by
   // the Profile. Callers that outlive the life of this profile need to be
   // sure they refcount the returned value.
@@ -253,7 +245,6 @@ class ProfileImpl : public Profile {
   virtual Profile* GetOriginalProfile();
   virtual VisitedLinkMaster* GetVisitedLinkMaster();
   virtual HistoryService* GetHistoryService(ServiceAccessType sat);
-  virtual bool HasHistoryService() const;
   virtual WebDataService* GetWebDataService(ServiceAccessType sat);
   virtual PrefService* GetPrefs();
   virtual TemplateURLModel* GetTemplateURLModel();
diff --git a/chrome/browser/views/bookmark_bar_view.cc b/chrome/browser/views/bookmark_bar_view.cc
index 64e4489..b5a952f 100644
--- a/chrome/browser/views/bookmark_bar_view.cc
+++ b/chrome/browser/views/bookmark_bar_view.cc
@@ -672,20 +672,17 @@ void BookmarkBarView::SetProfile(Profile* profile) {
 
   NotificationService* ns = NotificationService::current();
   Source<Profile> ns_source(profile_->GetOriginalProfile());
-  ns->AddObserver(this, NOTIFY_HISTORY_CREATED, ns_source);
   ns->AddObserver(this, NOTIFY_BOOKMARK_BUBBLE_SHOWN, ns_source);
   ns->AddObserver(this, NOTIFY_BOOKMARK_BUBBLE_HIDDEN, ns_source);
   ns->AddObserver(this, NOTIFY_BOOKMARK_BAR_VISIBILITY_PREF_CHANGED,
                   NotificationService::AllSources());
 
-  if (!profile->HasHistoryService()) {
-    // The history service hasn't been loaded yet. We don't want to trigger
-    // loading it. Instead we install an observer that is notified when the
-    // history service has loaded.
-    model_ = NULL;
-  } else {
-    ProfileHasValidHistoryService();
-  }
+  model_ = profile_->GetBookmarkBarModel();
+  model_->AddObserver(this);
+  if (model_->IsLoaded())
+    Loaded(model_);
+  // else case: we'll receive notification back from the BookmarkBarModel when
+  // done loading, then we'll populate the bar.
 }
 
 void BookmarkBarView::SetPageNavigator(PageNavigator* navigator) {
@@ -1121,6 +1118,7 @@ void BookmarkBarView::Init() {
   SetContextMenuController(this);
 
   size_animation_.reset(new SlideAnimation(this));
+  size_animation_->SetSlideDuration(4000);
 }
 
 MenuButton* BookmarkBarView::CreateOtherBookmarkedButton() {
@@ -1439,37 +1437,31 @@ void BookmarkBarView::Observe(NotificationType type,
                               const NotificationSource& source,
                               const NotificationDetails& details) {
   DCHECK(profile_);
-  if (type == NOTIFY_BOOKMARK_BAR_VISIBILITY_PREF_CHANGED) {
-    if (IsAlwaysShown()) {
-      size_animation_->Show();
-    } else {
-      size_animation_->Hide();
-    }
-  } else if (type == NOTIFY_HISTORY_CREATED) {
-    ProfileHasValidHistoryService();
-  } else if (type == NOTIFY_BOOKMARK_BUBBLE_SHOWN) {
-    StopThrobbing(true);
-    bubble_url_ = *(Details<GURL>(details).ptr());
-    StartThrobbing();
-  } else if (type == NOTIFY_BOOKMARK_BUBBLE_HIDDEN) {
-    StopThrobbing(false);
-    bubble_url_ = GURL();
-  }
-}
+  switch (type) {
+    case NOTIFY_BOOKMARK_BAR_VISIBILITY_PREF_CHANGED:
+      if (IsAlwaysShown()) {
+        size_animation_->Show();
+      } else {
+        size_animation_->Hide();
+      }
+      break;
 
-void BookmarkBarView::ProfileHasValidHistoryService() {
-  DCHECK(profile_);
-  model_ = profile_->GetBookmarkBarModel();
-  DCHECK(model_);
-  model_->AddObserver(this);
-  if (model_->IsLoaded())
-    Loaded(model_);
+    case NOTIFY_BOOKMARK_BUBBLE_SHOWN:
+      StopThrobbing(true);
+      bubble_url_ = *(Details<GURL>(details).ptr());
+      StartThrobbing();
+      break;
+
+    case NOTIFY_BOOKMARK_BUBBLE_HIDDEN:
+      StopThrobbing(false);
+      bubble_url_ = GURL();
+      break;
+  }
 }
 
 void BookmarkBarView::RemoveNotificationObservers() {
   NotificationService* ns = NotificationService::current();
   Source<Profile> ns_source(profile_->GetOriginalProfile());
-  ns->RemoveObserver(this, NOTIFY_HISTORY_CREATED, ns_source);
   ns->RemoveObserver(this, NOTIFY_BOOKMARK_BUBBLE_SHOWN, ns_source);
   ns->RemoveObserver(this, NOTIFY_BOOKMARK_BUBBLE_HIDDEN, ns_source);
   ns->RemoveObserver(this, NOTIFY_BOOKMARK_BAR_VISIBILITY_PREF_CHANGED,
diff --git a/chrome/browser/views/bookmark_bar_view.h b/chrome/browser/views/bookmark_bar_view.h
index 1f3e124..507da5d 100644
--- a/chrome/browser/views/bookmark_bar_view.h
+++ b/chrome/browser/views/bookmark_bar_view.h
@@ -309,16 +309,11 @@ class BookmarkBarView : public ChromeViews::View,
   // visible. Updates the preferences to match the users choice as appropriate.
   virtual void ExecuteCommand(int id);
 
-  // Notification that the HistoryService is up an running. Removes us as
-  // a listener on the notification service and invokes
-  // ProfileHasValidHistoryService.
+  // NotificationService method.
   virtual void Observe(NotificationType type,
                        const NotificationSource& source,
                        const NotificationDetails& details);
 
-  // Invoked when the profile has a history service. Recreates the models.
-  void ProfileHasValidHistoryService();
-
   // If we have registered an observer on the notification service, this
   // unregisters it. This does nothing if we have not installed ourself as an
   // observer.
diff --git a/chrome/test/testing_profile.h b/chrome/test/testing_profile.h
index 4ff9dbb..e33a423 100644
--- a/chrome/test/testing_profile.h
+++ b/chrome/test/testing_profile.h
@@ -64,9 +64,6 @@ class TestingProfile : public Profile {
   void set_has_history_service(bool has_history_service) {
     has_history_service_ = has_history_service;
   }
-  virtual bool HasHistoryService() const {
-    return (history_service_.get() != NULL || has_history_service_);
-  }
   virtual WebDataService* GetWebDataService(ServiceAccessType access) {
     return NULL;
   }