Attempt at fixing crash during importing. As far as I can tell from

the crash the BookmarkModel is trying to notifying observers it's
going away and one of those observers has been deleted. This is during
first run import, so the only observer should be the importer.

This may be possible if someone prematurely exits the nested loop
started during first run so that the importerhost isn't done when the
profile is deleted.

BUG=16143
TEST=none

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

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

2 files changed, 27 insertions, 2 deletions

diff --git a/chrome/browser/importer/importer.cc b/chrome/browser/importer/importer.cc
index 6f36d19..cf9e278 100644
--- a/chrome/browser/importer/importer.cc
+++ b/chrome/browser/importer/importer.cc
@@ -397,11 +397,13 @@ bool Importer::ReencodeFavicon(const unsigned char* src_data, size_t src_len,
 // ImporterHost.
 
 ImporterHost::ImporterHost()
-    : observer_(NULL),
+    : profile_(NULL),
+      observer_(NULL),
       task_(NULL),
       importer_(NULL),
       file_loop_(g_browser_process->file_thread()->message_loop()),
       waiting_for_bookmarkbar_model_(false),
+      installed_bookmark_observer_(false),
       is_source_readable_(true),
       headless_(false),
       parent_window_(NULL) {
@@ -409,11 +411,13 @@ ImporterHost::ImporterHost()
 }
 
 ImporterHost::ImporterHost(MessageLoop* file_loop)
-    : observer_(NULL),
+    : profile_(NULL),
+      observer_(NULL),
       task_(NULL),
       importer_(NULL),
       file_loop_(file_loop),
       waiting_for_bookmarkbar_model_(false),
+      installed_bookmark_observer_(false),
       is_source_readable_(true),
       headless_(false),
       parent_window_(NULL) {
@@ -423,12 +427,18 @@ ImporterHost::ImporterHost(MessageLoop* file_loop)
 ImporterHost::~ImporterHost() {
   if (NULL != importer_)
     importer_->Release();
+  if (installed_bookmark_observer_) {
+    DCHECK(profile_);  // Only way for waiting_for_bookmarkbar_model_ to be true
+                       // is if we have a profile.
+    profile_->GetBookmarkModel()->RemoveObserver(this);
+  }
 }
 
 void ImporterHost::Loaded(BookmarkModel* model) {
   DCHECK(model->IsLoaded());
   model->RemoveObserver(this);
   waiting_for_bookmarkbar_model_ = false;
+  installed_bookmark_observer_ = false;
 
   std::vector<GURL> starred_urls;
   model->GetBookmarks(&starred_urls);
@@ -436,6 +446,10 @@ void ImporterHost::Loaded(BookmarkModel* model) {
   InvokeTaskIfDone();
 }
 
+void ImporterHost::BookmarkModelBeingDeleted(BookmarkModel* model) {
+  installed_bookmark_observer_ = false;
+}
+
 void ImporterHost::Observe(NotificationType type,
                            const NotificationSource& source,
                            const NotificationDetails& details) {
@@ -487,6 +501,9 @@ void ImporterHost::StartImportSettings(const ProfileInfo& profile_info,
                                        uint16 items,
                                        ProfileWriter* writer,
                                        bool first_run) {
+  DCHECK(!profile_);  // We really only support importing from one host at a
+                      // time.
+  profile_ = target_profile;
   // Preserves the observer and creates a task, since we do async import
   // so that it doesn't block the UI. When the import is complete, observer
   // will be notified.
@@ -564,6 +581,7 @@ void ImporterHost::StartImportSettings(const ProfileInfo& profile_info,
   if ((items & FAVORITES) && !writer_->BookmarkModelIsLoaded()) {
     target_profile->GetBookmarkModel()->AddObserver(this);
     waiting_for_bookmarkbar_model_ = true;
+    installed_bookmark_observer_ = true;
   }
 
   // Observes the TemplateURLModel if needed to import search engines from the
diff --git a/chrome/browser/importer/importer.h b/chrome/browser/importer/importer.h
index 4b8739e..8fc3587 100644
--- a/chrome/browser/importer/importer.h
+++ b/chrome/browser/importer/importer.h
@@ -182,6 +182,7 @@ class ImporterHost : public base::RefCountedThreadSafe<ImporterHost>,
                                              const BookmarkNode* node) {}
   virtual void BookmarkNodeFavIconLoaded(BookmarkModel* model,
                                          const BookmarkNode* node) {}
+  virtual void BookmarkModelBeingDeleted(BookmarkModel* model);
 
   // NotificationObserver method. Called when TemplateURLModel has been loaded.
   void Observe(NotificationType type,
@@ -280,6 +281,9 @@ class ImporterHost : public base::RefCountedThreadSafe<ImporterHost>,
   NotificationRegistrar registrar_;
   ImporterList importer_list_;
 
+  // Profile we're importing from.
+  Profile* profile_;
+
   Observer* observer_;
   scoped_refptr<ProfileWriter> writer_;
 
@@ -295,6 +299,9 @@ class ImporterHost : public base::RefCountedThreadSafe<ImporterHost>,
   // True if we're waiting for the model to finish loading.
   bool waiting_for_bookmarkbar_model_;
 
+  // Have we installed a listener on the bookmark model?
+  bool installed_bookmark_observer_;
+
   // True if source profile is readable.
   bool is_source_readable_;