Importer: Fix ImporterList::DetectSourceProfiles to run on the FILE thread, as

it access the file system.  Fix up a few call sites.

BUG=60825
TEST=none

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

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

4 files changed, 277 insertions, 137 deletions

diff --git a/chrome/browser/importer/importer.cc b/chrome/browser/importer/importer.cc
index c1dc196..4286f4e 100644
--- a/chrome/browser/importer/importer.cc
+++ b/chrome/browser/importer/importer.cc
@@ -85,8 +85,23 @@ ImporterHost::ImporterHost()
       installed_bookmark_observer_(false),
       is_source_readable_(true),
       headless_(false),
-      parent_window_(NULL) {
-  importer_list_.DetectSourceProfiles();
+      parent_window_(NULL),
+      importer_list_(new ImporterList) {
+  importer_list_->DetectSourceProfilesHack();
+}
+
+ImporterHost::ImporterHost(ImporterList::Observer* observer)
+    : profile_(NULL),
+      observer_(NULL),
+      task_(NULL),
+      importer_(NULL),
+      waiting_for_bookmarkbar_model_(false),
+      installed_bookmark_observer_(false),
+      is_source_readable_(true),
+      headless_(false),
+      parent_window_(NULL),
+      importer_list_(new ImporterList) {
+  importer_list_->DetectSourceProfiles(observer);
 }
 
 ImporterHost::~ImporterHost() {
@@ -171,7 +186,7 @@ void ImporterHost::StartImportSettings(
   // so that it doesn't block the UI. When the import is complete, observer
   // will be notified.
   writer_ = writer;
-  importer_ = importer_list_.CreateImporterByType(profile_info.browser_type);
+  importer_ = ImporterList::CreateImporterByType(profile_info.browser_type);
   // If we fail to create Importer, exit as we cannot do anything.
   if (!importer_) {
     ImportEnded();
@@ -307,6 +322,15 @@ ExternalProcessImporterHost::ExternalProcessImporterHost()
       import_process_launched_(false) {
 }
 
+ExternalProcessImporterHost::ExternalProcessImporterHost(
+    ImporterList::Observer* observer)
+    : ImporterHost(observer),
+      items_(0),
+      import_to_bookmark_bar_(false),
+      cancelled_(false),
+      import_process_launched_(false) {
+}
+
 void ExternalProcessImporterHost::Loaded(BookmarkModel* model) {
   DCHECK(model->IsLoaded());
   model->RemoveObserver(this);
diff --git a/chrome/browser/importer/importer.h b/chrome/browser/importer/importer.h
index 1cb9db7..d8cecac 100644
--- a/chrome/browser/importer/importer.h
+++ b/chrome/browser/importer/importer.h
@@ -55,9 +55,37 @@ class ImporterHost : public base::RefCountedThreadSafe<ImporterHost>,
                      public BookmarkModelObserver,
                      public NotificationObserver {
  public:
+  // An interface which an object can implement to be notified of events during
+  // the import process.
+  class Observer {
+   public:
+    // Invoked when data for the specified item is about to be collected.
+    virtual void ImportItemStarted(importer::ImportItem item) = 0;
+
+    // Invoked when data for the specified item has been collected from the
+    // source profile and is now ready for further processing.
+    virtual void ImportItemEnded(importer::ImportItem item) = 0;
+
+    // Invoked when the import begins.
+    virtual void ImportStarted() = 0;
+
+    // Invoked when the source profile has been imported.
+    virtual void ImportEnded() = 0;
+
+   protected:
+    virtual ~Observer() {}
+  };
+
+  // DEPRECATED: Calls the synchronous version of
+  // ImporterList::DetectSourceProfiles.
+  // TODO(jhawkins): Remove this constructor once all callers are fixed.
+  // See http://crbug.com/65633 and http://crbug.com/65638.
   ImporterHost();
 
-  // BookmarkModelObserver methods.
+  // |observer| must not be NULL.
+  explicit ImporterHost(ImporterList::Observer* observer);
+
+  // BookmarkModelObserver implementation.
   virtual void Loaded(BookmarkModel* model);
   virtual void BookmarkNodeMoved(BookmarkModel* model,
                                  const BookmarkNode* old_parent,
@@ -79,7 +107,8 @@ class ImporterHost : public base::RefCountedThreadSafe<ImporterHost>,
                                          const BookmarkNode* node) {}
   virtual void BookmarkModelBeingDeleted(BookmarkModel* model);
 
-  // NotificationObserver method. Called when TemplateURLModel has been loaded.
+  // NotificationObserver implementation. Called when TemplateURLModel has been
+  // loaded.
   void Observe(NotificationType type,
                const NotificationSource& source,
                const NotificationDetails& details);
@@ -122,24 +151,6 @@ class ImporterHost : public base::RefCountedThreadSafe<ImporterHost>,
     parent_window_ = parent_window;
   }
 
-  // An interface which an object can implement to be notified of events during
-  // the import process.
-  class Observer {
-   public:
-     virtual ~Observer() {}
-    // Invoked when data for the specified item is about to be collected.
-    virtual void ImportItemStarted(importer::ImportItem item) = 0;
-
-    // Invoked when data for the specified item has been collected from the
-    // source profile and is now ready for further processing.
-    virtual void ImportItemEnded(importer::ImportItem item) = 0;
-
-    // Invoked when the import begins.
-    virtual void ImportStarted() = 0;
-
-    // Invoked when the source profile has been imported.
-    virtual void ImportEnded() = 0;
-  };
   void SetObserver(Observer* observer);
 
   // A series of functions invoked at the start, during and end of the end
@@ -152,25 +163,25 @@ class ImporterHost : public base::RefCountedThreadSafe<ImporterHost>,
   virtual void ImportEnded();
 
   int GetAvailableProfileCount() const {
-    return importer_list_.GetAvailableProfileCount();
+    return importer_list_->GetAvailableProfileCount();
   }
 
   // Returns the name of the profile at the 'index' slot. The profiles are
   // ordered such that the profile at index 0 is the likely default browser.
   std::wstring GetSourceProfileNameAt(int index) const {
-    return importer_list_.GetSourceProfileNameAt(index);
+    return importer_list_->GetSourceProfileNameAt(index);
   }
 
   // Returns the ProfileInfo at the specified index.  The ProfileInfo should be
   // passed to StartImportSettings().
   const importer::ProfileInfo& GetSourceProfileInfoAt(int index) const {
-    return importer_list_.GetSourceProfileInfoAt(index);
+    return importer_list_->GetSourceProfileInfoAt(index);
   }
 
   // Returns the ProfileInfo with the given browser type.
   const importer::ProfileInfo& GetSourceProfileInfoForBrowserType(
       int browser_type) const {
-    return importer_list_.GetSourceProfileInfoForBrowserType(browser_type);
+    return importer_list_->GetSourceProfileInfoForBrowserType(browser_type);
   }
 
  protected:
@@ -240,7 +251,7 @@ class ImporterHost : public base::RefCountedThreadSafe<ImporterHost>,
   virtual void InvokeTaskIfDone();
 
   // Used to create an importer of the appropriate type.
-  ImporterList importer_list_;
+  scoped_refptr<ImporterList> importer_list_;
 
   DISALLOW_COPY_AND_ASSIGN(ImporterHost);
 };
@@ -249,8 +260,14 @@ class ImporterHost : public base::RefCountedThreadSafe<ImporterHost>,
 // the importer bridge and the external process importer client.
 class ExternalProcessImporterHost : public ImporterHost {
  public:
+  // DEPRECATED: Calls the deprecated ImporterHost constructor.
+  // TODO(jhawkins): Remove this constructor once all callers are fixed.
+  // See http://crbug.com/65633 and http://crbug.com/65638.
   ExternalProcessImporterHost();
 
+  // |observer| must not be NULL.
+  explicit ExternalProcessImporterHost(ImporterList::Observer* observer);
+
   // Called when the BookmarkModel has finished loading. Calls InvokeTaskIfDone
   // to start importing.
   virtual void Loaded(BookmarkModel* model);
diff --git a/chrome/browser/importer/importer_list.cc b/chrome/browser/importer/importer_list.cc
index 6ac6517..e38cc5e 100644
--- a/chrome/browser/importer/importer_list.cc
+++ b/chrome/browser/importer/importer_list.cc
@@ -27,39 +27,94 @@
 #include "chrome/browser/importer/safari_importer.h"
 #endif
 
-ImporterList::ImporterList() {
+namespace {
+
+#if defined(OS_WIN)
+void DetectIEProfiles(std::vector<importer::ProfileInfo*>* profiles) {
+    // IE always exists and doesn't have multiple profiles.
+  ProfileInfo* ie = new ProfileInfo();
+  ie->description = l10n_util::GetString(IDS_IMPORT_FROM_IE);
+  ie->browser_type = importer::MS_IE;
+  ie->source_path.clear();
+  ie->app_path.clear();
+  ie->services_supported = importer::HISTORY | importer::FAVORITES |
+      importer::COOKIES | importer::PASSWORDS | importer::SEARCH_ENGINES;
+  profiles->push_back(ie);
 }
+#endif  // defined(OS_WIN)
 
-ImporterList::~ImporterList() {
-  STLDeleteContainerPointers(source_profiles_.begin(), source_profiles_.end());
+#if defined(OS_MACOSX)
+void DetectSafariProfiles(std::vector<importer::ProfileInfo*>* profiles) {
+  uint16 items = importer::NONE;
+  if (!SafariImporter::CanImport(mac_util::GetUserLibraryPath(), &items))
+    return;
+
+  importer::ProfileInfo* safari = new importer::ProfileInfo();
+  safari->browser_type = importer::SAFARI;
+  safari->description = l10n_util::GetString(IDS_IMPORT_FROM_SAFARI);
+  safari->source_path.clear();
+  safari->app_path.clear();
+  safari->services_supported = items;
+  profiles->push_back(safari);
 }
+#endif  // defined(OS_MACOSX)
 
-void ImporterList::DetectSourceProfiles() {
-// The first run import will automatically take settings from the first
-// profile detected, which should be the user's current default.
+void DetectFirefoxProfiles(std::vector<importer::ProfileInfo*>* profiles) {
+  FilePath profile_path = GetFirefoxProfilePath();
+  if (profile_path.empty())
+    return;
+
+  // Detects which version of Firefox is installed.
+  importer::ProfileType firefox_type;
+  FilePath app_path;
+  int version = 0;
 #if defined(OS_WIN)
-  if (ShellIntegration::IsFirefoxDefaultBrowser()) {
-    DetectFirefoxProfiles();
-    DetectIEProfiles();
-  } else {
-    DetectIEProfiles();
-    DetectFirefoxProfiles();
-  }
-  // TODO(brg) : Current UI requires win_util.
-  DetectGoogleToolbarProfiles();
-#elif defined(OS_MACOSX)
-  if (ShellIntegration::IsFirefoxDefaultBrowser()) {
-    DetectFirefoxProfiles();
-    DetectSafariProfiles();
+  version = GetCurrentFirefoxMajorVersionFromRegistry();
+#endif
+  if (version < 2)
+    GetFirefoxVersionAndPathFromProfile(profile_path, &version, &app_path);
+
+  if (version == 2) {
+    firefox_type = importer::FIREFOX2;
+  } else if (version >= 3) {
+    firefox_type = importer::FIREFOX3;
   } else {
-    DetectSafariProfiles();
-    DetectFirefoxProfiles();
+    // Ignores other versions of firefox.
+    return;
   }
-#else
-  DetectFirefoxProfiles();
+
+  importer::ProfileInfo* firefox = new importer::ProfileInfo();
+  firefox->description = l10n_util::GetString(IDS_IMPORT_FROM_FIREFOX);
+  firefox->browser_type = firefox_type;
+  firefox->source_path = profile_path;
+#if defined(OS_WIN)
+  firefox->app_path = FilePath::FromWStringHack(
+      GetFirefoxInstallPathFromRegistry());
 #endif
+  if (firefox->app_path.empty())
+    firefox->app_path = app_path;
+  firefox->services_supported = importer::HISTORY | importer::FAVORITES |
+      importer::PASSWORDS | importer::SEARCH_ENGINES;
+  profiles->push_back(firefox);
+}
+
+void DetectGoogleToolbarProfiles(std::vector<importer::ProfileInfo*>* profiles) {
+  if (FirstRun::IsChromeFirstRun())
+    return;
+
+  importer::ProfileInfo* google_toolbar = new importer::ProfileInfo();
+  google_toolbar->browser_type = importer::GOOGLE_TOOLBAR5;
+  google_toolbar->description = l10n_util::GetString(
+                                IDS_IMPORT_FROM_GOOGLE_TOOLBAR);
+  google_toolbar->source_path.clear();
+  google_toolbar->app_path.clear();
+  google_toolbar->services_supported = importer::FAVORITES;
+  profiles->push_back(google_toolbar);
 }
 
+}  // namespace
+
+// static
 Importer* ImporterList::CreateImporterByType(importer::ProfileType type) {
   switch (type) {
 #if defined(OS_WIN)
@@ -85,23 +140,53 @@ Importer* ImporterList::CreateImporterByType(importer::ProfileType type) {
   return NULL;
 }
 
+ImporterList::ImporterList()
+    : source_thread_id_(BrowserThread::UI),
+      observer_(NULL),
+      source_profiles_loaded_(false) {
+}
+
+ImporterList::~ImporterList() {
+}
+
+void ImporterList::DetectSourceProfiles(Observer* observer) {
+  DCHECK(observer);
+  observer_ = observer;
+
+  BrowserThread::GetCurrentThreadIdentifier(&source_thread_id_);
+
+  BrowserThread::PostTask(
+      BrowserThread::FILE,
+      FROM_HERE,
+      NewRunnableMethod(this, &ImporterList::DetectSourceProfilesWorker));
+}
+
+void ImporterList::DetectSourceProfilesHack() {
+  DetectSourceProfilesWorker();
+}
+
 int ImporterList::GetAvailableProfileCount() const {
+  DCHECK(source_profiles_loaded_);
   return static_cast<int>(source_profiles_.size());
 }
 
 std::wstring ImporterList::GetSourceProfileNameAt(int index) const {
+  DCHECK(source_profiles_loaded_);
   DCHECK(index >=0 && index < GetAvailableProfileCount());
   return source_profiles_[index]->description;
 }
 
 const importer::ProfileInfo& ImporterList::GetSourceProfileInfoAt(
     int index) const {
+  DCHECK(source_profiles_loaded_);
   DCHECK(index >=0 && index < GetAvailableProfileCount());
   return *source_profiles_[index];
 }
 
 const importer::ProfileInfo& ImporterList::GetSourceProfileInfoForBrowserType(
     int browser_type) const {
+  DCHECK(source_profiles_loaded_);
+
   int count = GetAvailableProfileCount();
   for (int i = 0; i < count; ++i) {
     if (source_profiles_[i]->browser_type == browser_type)
@@ -111,83 +196,62 @@ const importer::ProfileInfo& ImporterList::GetSourceProfileInfoForBrowserType(
   return *(new importer::ProfileInfo());
 }
 
-#if defined(OS_WIN)
-void ImporterList::DetectIEProfiles() {
-  // IE always exists and don't have multiple profiles.
-  ProfileInfo* ie = new ProfileInfo();
-  ie->description = l10n_util::GetString(IDS_IMPORT_FROM_IE);
-  ie->browser_type = importer::MS_IE;
-  ie->source_path.clear();
-  ie->app_path.clear();
-  ie->services_supported = importer::HISTORY | importer::FAVORITES |
-      importer::COOKIES | importer::PASSWORDS | importer::SEARCH_ENGINES;
-  source_profiles_.push_back(ie);
-}
-#endif
+void ImporterList::DetectSourceProfilesWorker() {
+  // TODO(jhawkins): Remove this condition once DetectSourceProfileHack is
+  // removed. |observer_| is NULL when said method is called.
+  if (observer_)
+    DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
 
-void ImporterList::DetectFirefoxProfiles() {
-  FilePath profile_path = GetFirefoxProfilePath();
-  if (profile_path.empty())
-    return;
+  std::vector<importer::ProfileInfo*> profiles;
 
-  // Detects which version of Firefox is installed.
-  importer::ProfileType firefox_type;
-  FilePath app_path;
-  int version = 0;
+// The first run import will automatically take settings from the first
+// profile detected, which should be the user's current default.
 #if defined(OS_WIN)
-  version = GetCurrentFirefoxMajorVersionFromRegistry();
-#endif
-  if (version < 2)
-    GetFirefoxVersionAndPathFromProfile(profile_path, &version, &app_path);
-
-  if (version == 2) {
-    firefox_type = importer::FIREFOX2;
-  } else if (version >= 3) {
-    firefox_type = importer::FIREFOX3;
+  if (ShellIntegration::IsFirefoxDefaultBrowser()) {
+    DetectFirefoxProfiles(&profiles);
+    DetectIEProfiles(&profiles);
   } else {
-    // Ignores other versions of firefox.
-    return;
+    DetectIEProfiles(&profiles);
+    DetectFirefoxProfiles(&profiles);
   }
-
-  importer::ProfileInfo* firefox = new importer::ProfileInfo();
-  firefox->description = l10n_util::GetString(IDS_IMPORT_FROM_FIREFOX);
-  firefox->browser_type = firefox_type;
-  firefox->source_path = profile_path;
-#if defined(OS_WIN)
-  firefox->app_path = FilePath::FromWStringHack(
-      GetFirefoxInstallPathFromRegistry());
+  // TODO(brg) : Current UI requires win_util.
+  DetectGoogleToolbarProfiles(&profiles);
+#elif defined(OS_MACOSX)
+  if (ShellIntegration::IsFirefoxDefaultBrowser()) {
+    DetectFirefoxProfiles(&profiles);
+    DetectSafariProfiles(&profiles);
+  } else {
+    DetectSafariProfiles(&profiles);
+    DetectFirefoxProfiles(&profiles);
+  }
+#else
+  DetectFirefoxProfiles(&profiles);
 #endif
-  if (firefox->app_path.empty())
-    firefox->app_path = app_path;
-  firefox->services_supported = importer::HISTORY | importer::FAVORITES |
-      importer::PASSWORDS | importer::SEARCH_ENGINES;
-  source_profiles_.push_back(firefox);
-}
 
-void ImporterList::DetectGoogleToolbarProfiles() {
-  if (!FirstRun::IsChromeFirstRun()) {
-    importer::ProfileInfo* google_toolbar = new importer::ProfileInfo();
-    google_toolbar->browser_type = importer::GOOGLE_TOOLBAR5;
-    google_toolbar->description = l10n_util::GetString(
-                                  IDS_IMPORT_FROM_GOOGLE_TOOLBAR);
-    google_toolbar->source_path.clear();
-    google_toolbar->app_path.clear();
-    google_toolbar->services_supported = importer::FAVORITES;
-    source_profiles_.push_back(google_toolbar);
+  // TODO(jhawkins): Remove this condition once DetectSourceProfileHack is
+  // removed. |observer_| is NULL when said method is called.
+  if (observer_) {
+    BrowserThread::PostTask(
+        source_thread_id_,
+        FROM_HERE,
+        NewRunnableMethod(this, &ImporterList::SourceProfilesLoaded, profiles));
+  } else {
+    source_profiles_->assign(profiles.begin(), profiles.end());
+    source_profiles_loaded_ = true;
   }
 }
 
-#if defined(OS_MACOSX)
-void ImporterList::DetectSafariProfiles() {
-  uint16 items = importer::NONE;
-  if (SafariImporter::CanImport(mac_util::GetUserLibraryPath(), &items)) {
-    importer::ProfileInfo* safari = new importer::ProfileInfo();
-    safari->browser_type = importer::SAFARI;
-    safari->description = l10n_util::GetString(IDS_IMPORT_FROM_SAFARI);
-    safari->source_path.clear();
-    safari->app_path.clear();
-    safari->services_supported = items;
-    source_profiles_.push_back(safari);
-  }
+void ImporterList::SourceProfilesLoaded(
+    const std::vector<importer::ProfileInfo*>& profiles) {
+  DCHECK_NE(static_cast<Observer*>(NULL), observer_);
+
+  BrowserThread::ID current_thread_id;
+  BrowserThread::GetCurrentThreadIdentifier(&current_thread_id);
+  DCHECK_EQ(current_thread_id, source_thread_id_);
+
+  source_profiles_->assign(profiles.begin(), profiles.end());
+  source_profiles_loaded_ = true;
+  observer_->SourceProfilesLoaded();
+  observer_ = NULL;
+  source_thread_id_ = BrowserThread::UI;
 }
-#endif  // OS_MACOSX
diff --git a/chrome/browser/importer/importer_list.h b/chrome/browser/importer/importer_list.h
index 6f98046..fc801fc 100644
--- a/chrome/browser/importer/importer_list.h
+++ b/chrome/browser/importer/importer_list.h
@@ -9,23 +9,42 @@
 #include <string>
 #include <vector>
 
-#include "build/build_config.h"
 #include "base/basictypes.h"
+#include "base/platform_thread.h"
+#include "base/ref_counted.h"
+#include "base/scoped_vector.h"
+#include "build/build_config.h"
+#include "chrome/browser/browser_thread.h"
 #include "chrome/browser/importer/importer_data_types.h"
 
 class Importer;
 
-class ImporterList {
+class ImporterList : public base::RefCountedThreadSafe<ImporterList> {
  public:
+  // Any class calling DetectSourceProfiles() must implement this interface in
+  // order to be called back when the source profiles are loaded.
+  class Observer {
+   public:
+    virtual void SourceProfilesLoaded() = 0;
+
+   protected:
+    virtual ~Observer() {}
+  };
+
+  static Importer* CreateImporterByType(importer::ProfileType type);
+
   ImporterList();
-  ~ImporterList();
 
   // Detects the installed browsers and their associated profiles, then
   // stores their information in a list. It returns the list of description
-  // of all profiles.
-  void DetectSourceProfiles();
+  // of all profiles. Calls into DetectSourceProfilesWorker() on the FILE thread
+  // to do the real work of detecting source profiles. |observer| must be
+  // non-NULL.
+  void DetectSourceProfiles(Observer* observer);
 
-  Importer* CreateImporterByType(importer::ProfileType type);
+  // DEPRECATED: This method is synchronous and performs file operations which
+  // may end up blocking the current thread, which is usually the UI thread.
+  void DetectSourceProfilesHack();
 
   // Returns the number of different browser profiles you can import from.
   int GetAvailableProfileCount() const;
@@ -42,19 +61,35 @@ class ImporterList {
   const importer::ProfileInfo& GetSourceProfileInfoForBrowserType(
       int browser_type) const;
 
-  // Helper methods for detecting available profiles.
-#if defined(OS_WIN)
-  void DetectIEProfiles();
-#endif
-  void DetectFirefoxProfiles();
-  void DetectGoogleToolbarProfiles();
-#if defined(OS_MACOSX)
-  void DetectSafariProfiles();
-#endif
-
  private:
+  friend class base::RefCountedThreadSafe<ImporterList>;
+
+  ~ImporterList();
+
+  // The worker method for DetectSourceProfiles(). Must be called on the FILE
+  // thread.
+  void DetectSourceProfilesWorker();
+
+  // Called by DetectSourceProfilesWorker() on the source thread. This method
+  // notifies |observer_| that the source profiles are loaded. |profiles| is
+  // the vector of loaded profiles.
+  void SourceProfilesLoaded(
+      const std::vector<importer::ProfileInfo*>& profiles);
+
   // The list of profiles with the default one first.
-  std::vector<importer::ProfileInfo*> source_profiles_;
+  ScopedVector<importer::ProfileInfo> source_profiles_;
+
+  // The ID of the thread DetectSourceProfiles() is called on. Only valid after
+  // DetectSourceProfiles() is called and until SourceProfilesLoaded() has
+  // returned.
+  BrowserThread::ID source_thread_id_;
+
+  // Weak reference. Only valid after DetectSourceProfiles() is called and until
+  // SourceProfilesLoaded() has returned.
+  Observer* observer_;
+
+  // True if source profiles are loaded.
+  bool source_profiles_loaded_;
 
   DISALLOW_COPY_AND_ASSIGN(ImporterList);
 };