Support for importing non default Firefox profiles.

Bug: 1011830
Change-Id: If411450952c0e4018933eba636d168d98ece90cd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2070201
Commit-Queue: Ilya Sherman <isherman@chromium.org>
Reviewed-by: dpapad <dpapad@chromium.org>
Reviewed-by: Ilya Sherman <isherman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#749019}

AUTHORS

@@ -474,6 +474,7 @@ Johnson Lin <johnson.lin@intel.com>
 Jonathan Frazer <listedegarde@gmail.com>
 Jonathan Garbee <jonathan@garbee.me>
 Jonathan Hacker <jhacker@arcanefour.com>
+Jonathan Kingston <kingstonmailbox@gmail.com>
 Jongdeok Kim <jongdeok.kim@navercorp.com>
 Jongheon Kim <sapzape@gmail.com>
 JongKwon Lee <jongkwon.lee@navercorp.com>

chrome/browser/importer/importer_list.cc

@@ -101,41 +101,45 @@ void DetectFirefoxProfiles(const std::string locale,
 #else
   const std::string firefox_install_id;
 #endif  // defined(OS_WIN)
-  base::FilePath profile_path = GetFirefoxProfilePath(firefox_install_id);
-  if (profile_path.empty())
+  std::vector<FirefoxDetail> details = GetFirefoxDetails(firefox_install_id);
+  if (details.empty())
     return;
 
-  // Detects which version of Firefox is installed.
-  importer::ImporterType firefox_type;
+  for (auto detail = details.begin(); detail != details.end(); ++detail) {
     base::FilePath app_path;
+    if (detail->path.empty())
+      continue;
+
     int version = 0;
 #if defined(OS_WIN)
     version = GetCurrentFirefoxMajorVersionFromRegistry();
 #endif
-  if (version < 2)
-    GetFirefoxVersionAndPathFromProfile(profile_path, &version, &app_path);
 
-  if (version >= 3) {
-    firefox_type = importer::TYPE_FIREFOX;
-  } else {
+    if (version < 2) {
+      GetFirefoxVersionAndPathFromProfile(detail->path, &version, &app_path);
+      // Note that |version| is re-assigned above.
+      if (version < 2) {
         // Ignores old versions of firefox.
-    return;
+        continue;
+      }
     }
 
     importer::SourceProfile firefox;
     firefox.importer_name = GetFirefoxImporterName(app_path);
-  firefox.importer_type = firefox_type;
-  firefox.source_path = profile_path;
+    firefox.profile = detail->name;
+    firefox.importer_type = importer::TYPE_FIREFOX;
+    firefox.source_path = detail->path;
 #if defined(OS_WIN)
     firefox.app_path = GetFirefoxInstallPathFromRegistry();
 #endif
     if (firefox.app_path.empty())
       firefox.app_path = app_path;
-  firefox.services_supported = importer::HISTORY | importer::FAVORITES |
-                               importer::PASSWORDS | importer::SEARCH_ENGINES |
-                               importer::AUTOFILL_FORM_DATA;
+    firefox.services_supported =
+        importer::HISTORY | importer::FAVORITES | importer::PASSWORDS |
+        importer::SEARCH_ENGINES | importer::AUTOFILL_FORM_DATA;
     firefox.locale = locale;
     profiles->push_back(firefox);
+  }
 }
 
 std::vector<importer::SourceProfile> DetectSourceProfilesWorker(

chrome/browser/resources/settings/people_page/import_data_browser_proxy.js

@@ -9,6 +9,7 @@ cr.define('settings', function() {
    * @typedef {{
    *   name: string,
    *   index: number,
+   *   profileName: string,
    *   history: boolean,
    *   favorites: boolean,
    *   passwords: boolean,

chrome/browser/resources/settings/people_page/import_data_dialog.html

@@ -64,7 +64,12 @@
               aria-label="$i18n{importFromLabel}"
               on-change="onBrowserProfileSelectionChange_">
             <template is="dom-repeat" items="[[browserProfiles_]]">
-              <option value="[[item.index]]">[[item.name]]</option>
+              <option value="[[item.index]]">
+                [[item.name]]
+                <span hidden$="[[!item.profileName]]">
+                  - [[item.profileName]]
+                </span>
+              </option>
             </template>
           </select>
           <div class="description">$i18n{importDescription}</div>

chrome/browser/ui/webui/settings/import_data_handler.cc

@@ -191,6 +191,7 @@ void ImportDataHandler::SendBrowserProfileData(const std::string& callback_id) {
         new base::DictionaryValue());
     browser_profile->SetString("name", source_profile.importer_name);
     browser_profile->SetInteger("index", i);
+    browser_profile->SetString("profileName", source_profile.profile);
     browser_profile->SetBoolean("history",
                                 (browser_services & importer::HISTORY) != 0);
     browser_profile->SetBoolean("favorites",

chrome/common/importer/firefox_importer_utils.cc

@@ -50,74 +50,22 @@ base::FilePath GetProfilePath(const base::DictionaryValue& root,
   return path;
 }
 
-// Returns a map from Firefox profiles to their corresponding installation ids.
-// The keys are file system paths for Firefox profiles that are the default
-// profile in their installation. The values are the registry keys for the
-// corresponding installation.
-std::map<std::string, std::string> GetDefaultProfilesPerInstall(
-    const base::DictionaryValue& root) {
-  std::map<std::string, std::string> default_profile_to_install_id;
-  static constexpr base::StringPiece kInstallPrefix("Install");
-  // Find the default profiles for each Firefox installation.
-  for (const auto& data : root) {
-    const std::string& dict_key = data.first;
-    if (base::StartsWith(dict_key, kInstallPrefix,
-                         base::CompareCase::SENSITIVE)) {
-      std::string path;
-      if (root.GetStringASCII(dict_key + ".Default", &path)) {
-        default_profile_to_install_id.emplace(
-            std::move(path), dict_key.substr(kInstallPrefix.size()));
-      }
-    }
-  }
-  return default_profile_to_install_id;
-}
-
-base::FilePath GetLegacyDefaultProfilePath(
-    const base::DictionaryValue& root,
-    const std::vector<std::string>& profile_names) {
-  if (profile_names.empty())
-    return base::FilePath();
-
-  // When multiple profiles exist, the path to the default profile is returned.
-  for (const auto& profile_name : profile_names) {
-    // Checks if the named profile is the default profile using the legacy
-    // format of profiles.ini (Firefox version < 67).
-    std::string is_default;
-    if (root.GetStringASCII(profile_name + ".Default", &is_default) &&
-        is_default == "1") {
-      return GetProfilePath(root, profile_name);
-    }
-  }
-
-  // If no default profile is found, the path to Profile0 will be returned.
-  return GetProfilePath(root, profile_names.front());
-}
-
 } // namespace
 
-base::FilePath GetFirefoxProfilePath(const std::string& firefox_install_id) {
+std::vector<FirefoxDetail> GetFirefoxDetails(
+    const std::string& firefox_install_id) {
   base::FilePath ini_file = GetProfilesINI();
   std::string content;
   base::ReadFileToString(ini_file, &content);
   DictionaryValueINIParser ini_parser;
   ini_parser.Parse(content);
-  return GetFirefoxProfilePathFromDictionary(ini_parser.root(),
-                                             firefox_install_id);
+  return GetFirefoxDetailsFromDictionary(ini_parser.root(), firefox_install_id);
 }
 
-base::FilePath GetFirefoxProfilePathFromDictionary(
+std::vector<FirefoxDetail> GetFirefoxDetailsFromDictionary(
     const base::DictionaryValue& root,
     const std::string& firefox_install_id) {
-  // List of profiles linked to a Firefox installation. This will be empty for
-  // Firefox versions older than 67.
-  std::map<std::string, std::string> default_profile_to_install_id =
-      GetDefaultProfilesPerInstall(root);
-  // First profile linked to a Firefox installation (version >= 67).
-  base::Optional<std::string> first_modern_profile;
-
-  // Profiles not linked to a Firefox installation (version < 67).
-  std::vector<std::string> legacy_profiles;
+  std::vector<FirefoxDetail> profile_details;
 
   for (int i = 0; ; ++i) {
     std::string current_profile = base::StringPrintf("Profile%d", i);
@@ -131,26 +79,24 @@ base::FilePath GetFirefoxProfilePathFromDictionary(
     if (!root.GetStringASCII(current_profile + ".Path", &path))
       continue;
 
-    auto install_id_it = default_profile_to_install_id.find(path);
-    if (install_id_it != default_profile_to_install_id.end()) {
-      // If this installation is the default browser, use the associated
-      // profile as default profile.
-      if (install_id_it->second == firefox_install_id)
-        return GetProfilePath(root, current_profile);
-      if (!first_modern_profile)
-        first_modern_profile.emplace(std::move(current_profile));
-    } else {
-      // If no Firefox installation found in profiles.ini, legacy profiles
-      // (Firefox version < 67) are being used.
-      legacy_profiles.push_back(std::move(current_profile));
-    }
+    FirefoxDetail details;
+    details.path = GetProfilePath(root, current_profile);
+    std::string name;
+    root.GetStringASCII(current_profile + ".Name", &name);
+    // Make the profile name more presentable by replacing dashes with spaces.
+    base::ReplaceChars(name, "-", " ", &name);
+    details.name = name;
+    profile_details.push_back(details);
   }
 
-  // Take the first install found as the default install.
-  if (first_modern_profile)
-    return GetProfilePath(root, *first_modern_profile);
+  // If there is only one profile, set the name as a blank string.
+  // The name is only used to disambiguate profiles in the profile selection UI,
+  // which is only useful when there are multiple profiles.
+  if (profile_details.size() == 1) {
+    profile_details[0].name = "";
+  }
 
-  return GetLegacyDefaultProfilePath(root, legacy_profiles);
+  return profile_details;
 }
 
 #if defined(OS_MACOSX)

chrome/common/importer/firefox_importer_utils.h

@@ -8,6 +8,7 @@
 #include <string>
 #include <vector>
 
+#include "base/files/file_util.h"
 #include "base/strings/string16.h"
 #include "build/build_config.h"
 
@@ -38,15 +39,32 @@ base::FilePath GetFirefoxInstallPathFromRegistry();
 base::FilePath GetFirefoxDylibPath();
 #endif  // OS_MACOSX
 
-// Returns the path to the default profile of the Firefox installation with id
-// |firefox_install_id|.
-base::FilePath GetFirefoxProfilePath(const std::string& firefox_install_id);
+struct FirefoxDetail {
+  // |path| represents the Path field in Profiles.ini.
+  // This path is the directory name where all the profile information
+  // in stored.
+  base::FilePath path;
+  // The user specified name of the profile.
+  std::string name;
+};
+
+inline bool operator==(const FirefoxDetail& a1, const FirefoxDetail& a2) {
+  return a1.name == a2.name && a1.path == a2.path;
+}
+
+inline bool operator!=(const FirefoxDetail& a1, const FirefoxDetail& a2) {
+  return !(a1 == a2);
+}
+
+// Returns a vector of FirefoxDetail for available profiles.
+std::vector<FirefoxDetail> GetFirefoxDetails(
+    const std::string& firefox_install_id);
 
 // Returns the path to the Firefox profile, using a custom dictionary.
 // If |firefox_install_id| is not empty returns the default profile associated
 // with that id.
 // Exposed for testing.
-base::FilePath GetFirefoxProfilePathFromDictionary(
+std::vector<FirefoxDetail> GetFirefoxDetailsFromDictionary(
     const base::DictionaryValue& root,
     const std::string& firefox_install_id);
 

chrome/common/importer/firefox_importer_utils_unittest.cc

@@ -11,9 +11,12 @@
 #include "base/stl_util.h"
 #include "base/values.h"
 #include "chrome/grit/generated_resources.h"
+#include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "ui/base/l10n/l10n_util.h"
 
+using testing::UnorderedElementsAre;
+
 namespace {
 
 struct GetPrefsJsValueCase {
@@ -124,46 +127,71 @@ TEST(FirefoxImporterUtilsTest, GetFirefoxImporterName) {
 
 TEST(FirefoxImporterUtilsTest, GetFirefoxProfilePath) {
   base::DictionaryValue no_profiles;
-  EXPECT_EQ(std::string(),
-            GetFirefoxProfilePathFromDictionary(no_profiles, std::string())
-                .MaybeAsASCII());
+  EXPECT_EQ(0u,
+            GetFirefoxDetailsFromDictionary(no_profiles, std::string()).size());
 
   base::DictionaryValue single_profile;
   single_profile.SetString("Profile0.Path", "first");
+  // Ensure that when there is only one profile the profile name shown in the UI
+  // is empty, since there's no need to disambiguate among multiple profiles
+  single_profile.SetString("Profile0.Name", "namey");
   single_profile.SetString("Profile0.IsRelative", "0");
   single_profile.SetString("Profile0.Default", "1");
-  EXPECT_EQ("first",
-            GetFirefoxProfilePathFromDictionary(single_profile, std::string())
-                .MaybeAsASCII());
+
+  std::vector<FirefoxDetail> details =
+      GetFirefoxDetailsFromDictionary(single_profile, std::string());
+  EXPECT_THAT(details, UnorderedElementsAre(FirefoxDetail{
+                           base::FilePath(FILE_PATH_LITERAL("first")), ""}));
 
   base::DictionaryValue no_default;
   no_default.SetString("Profile0.Path", "first");
+  no_default.SetString("Profile0.Name", "namey");
   no_default.SetString("Profile0.IsRelative", "0");
   no_default.SetString("Profile1.Path", "second");
+  no_default.SetString("Profile1.Name", "namey-name");
   no_default.SetString("Profile1.IsRelative", "0");
-  EXPECT_EQ("first",
-            GetFirefoxProfilePathFromDictionary(no_default, std::string())
-                .MaybeAsASCII());
+  std::vector<FirefoxDetail> no_default_details =
+      GetFirefoxDetailsFromDictionary(no_default, std::string());
+  EXPECT_THAT(
+      no_default_details,
+      UnorderedElementsAre(
+          FirefoxDetail{base::FilePath(FILE_PATH_LITERAL("first")), "namey"},
+          FirefoxDetail{base::FilePath(FILE_PATH_LITERAL("second")),
+                        "namey name"}));
 
   base::DictionaryValue default_first;
   default_first.SetString("Profile0.Path", "first");
+  default_first.SetString("Profile0.Name", "namey");
   default_first.SetString("Profile0.IsRelative", "0");
   default_first.SetString("Profile0.Default", "1");
   default_first.SetString("Profile1.Path", "second");
+  default_first.SetString("Profile1.Name", "namey-name");
   default_first.SetString("Profile1.IsRelative", "0");
-  EXPECT_EQ("first",
-            GetFirefoxProfilePathFromDictionary(default_first, std::string())
-                .MaybeAsASCII());
+  std::vector<FirefoxDetail> default_first_details =
+      GetFirefoxDetailsFromDictionary(default_first, std::string());
+  EXPECT_THAT(
+      default_first_details,
+      UnorderedElementsAre(
+          FirefoxDetail{base::FilePath(FILE_PATH_LITERAL("first")), "namey"},
+          FirefoxDetail{base::FilePath(FILE_PATH_LITERAL("second")),
+                        "namey name"}));
 
   base::DictionaryValue default_second;
   default_second.SetString("Profile0.Path", "first");
+  default_second.SetString("Profile0.Name", "namey");
   default_second.SetString("Profile0.IsRelative", "0");
   default_second.SetString("Profile1.Path", "second");
+  default_second.SetString("Profile1.Name", "namey-name");
   default_second.SetString("Profile1.IsRelative", "0");
   default_second.SetString("Profile1.Default", "1");
-  EXPECT_EQ("second",
-            GetFirefoxProfilePathFromDictionary(default_second, std::string())
-                .MaybeAsASCII());
+  std::vector<FirefoxDetail> default_second_details =
+      GetFirefoxDetailsFromDictionary(default_second, std::string());
+  EXPECT_THAT(
+      default_second_details,
+      UnorderedElementsAre(
+          FirefoxDetail{base::FilePath(FILE_PATH_LITERAL("first")), "namey"},
+          FirefoxDetail{base::FilePath(FILE_PATH_LITERAL("second")),
+                        "namey name"}));
 
   // Firefox format from version 67
   base::DictionaryValue default_single_install;
@@ -172,9 +200,9 @@ TEST(FirefoxImporterUtilsTest, GetFirefoxProfilePath) {
   default_single_install.SetString("Profile0.Default", "1");
   default_single_install.SetString("Profile1.Path", "second");
   default_single_install.SetString("Profile1.IsRelative", "0");
-  EXPECT_EQ("second", GetFirefoxProfilePathFromDictionary(
-                          default_single_install, std::string())
-                          .MaybeAsASCII());
+  std::vector<FirefoxDetail> default_single_install_details =
+      GetFirefoxDetailsFromDictionary(default_single_install, std::string());
+  EXPECT_EQ("second", default_single_install_details[0].path.MaybeAsASCII());
 
   base::DictionaryValue default_single_install_unknown_profile;
   default_single_install_unknown_profile.SetString("Install01.Default",
@@ -184,11 +212,15 @@ TEST(FirefoxImporterUtilsTest, GetFirefoxProfilePath) {
   default_single_install_unknown_profile.SetString("Profile0.Default", "1");
   default_single_install_unknown_profile.SetString("Profile1.Path", "second");
   default_single_install_unknown_profile.SetString("Profile1.IsRelative", "0");
-  EXPECT_EQ("first", GetFirefoxProfilePathFromDictionary(
-                         default_single_install_unknown_profile, std::string())
-                         .MaybeAsASCII());
+  std::vector<FirefoxDetail> default_single_install_unknown_profile_details =
+      GetFirefoxDetailsFromDictionary(default_single_install_unknown_profile,
+                                      std::string());
+  EXPECT_THAT(
+      default_single_install_unknown_profile_details,
+      UnorderedElementsAre(
+          FirefoxDetail{base::FilePath(FILE_PATH_LITERAL("first")), ""},
+          FirefoxDetail{base::FilePath(FILE_PATH_LITERAL("second")), ""}));
 
-  base::DictionaryValue default_multiple_install;
   default_single_install_unknown_profile.SetString("Install01.Default",
                                                    "first");
   default_single_install_unknown_profile.SetString("Install02.Default",
@@ -198,7 +230,12 @@ TEST(FirefoxImporterUtilsTest, GetFirefoxProfilePath) {
   default_single_install_unknown_profile.SetString("Profile0.Default", "1");
   default_single_install_unknown_profile.SetString("Profile1.Path", "second");
   default_single_install_unknown_profile.SetString("Profile1.IsRelative", "0");
-  EXPECT_EQ("second", GetFirefoxProfilePathFromDictionary(
-                          default_single_install_unknown_profile, "02")
-                          .MaybeAsASCII());
+  std::vector<FirefoxDetail> default_multiple_install_details =
+      GetFirefoxDetailsFromDictionary(default_single_install_unknown_profile,
+                                      std::string());
+  EXPECT_THAT(
+      default_multiple_install_details,
+      UnorderedElementsAre(
+          FirefoxDetail{base::FilePath(FILE_PATH_LITERAL("first")), ""},
+          FirefoxDetail{base::FilePath(FILE_PATH_LITERAL("second")), ""}));
 }

chrome/common/importer/importer_data_types.h

@@ -48,6 +48,7 @@ struct SourceProfile {
   // The application locale. Stored because we can only access it from the UI
   // thread on the browser process. This is only used by the Firefox importer.
   std::string locale;
+  std::string profile;
 };
 
 // Contains information needed for importing search engine urls.