Rearrange GetPrimaryUserProfile() to work in unit tests

Rearranged ProfileManager::GetPrimaryUserProfile() to be able to run in
unit tests.  The early check of g_browser_process->profile_manager() is
not relevant for IS_CHROMEOS_ASH with a logged in user and it causes
unit tests to fail unnecessarily.

This has resulted in a few places where the code is duplicated which can
now be removed and use ProfileManager::GetPrimaryUserProfile().

Change-Id: I164b5c20dbc54cbd59c22bf7f84b5376af1b88c2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2596903
Auto-Submit: Joel Hockey <joelhockey@chromium.org>
Reviewed-by: David Roger <droger@chromium.org>
Commit-Queue: Joel Hockey <joelhockey@chromium.org>
Cr-Commit-Position: refs/heads/master@{#837968}

chrome/browser/chromeos/dbus/plugin_vm_service_provider.cc

@@ -13,8 +13,8 @@
 #include "chrome/browser/chromeos/plugin_vm/plugin_vm_pref_names.h"
 #include "chrome/browser/chromeos/plugin_vm/plugin_vm_util.h"
 #include "chrome/browser/chromeos/policy/browser_policy_connector_chromeos.h"
-#include "chrome/browser/chromeos/profiles/profile_helper.h"
 #include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/ui/chrome_pages.h"
 #include "chrome/browser/ui/settings_window_manager_chromeos.h"
 #include "chrome/browser/ui/webui/settings/chromeos/app_management/app_management_uma.h"
@@ -22,7 +22,6 @@
 #include "chrome/common/webui_url_constants.h"
 #include "chromeos/dbus/plugin_vm_service/plugin_vm_service.pb.h"
 #include "components/prefs/pref_service.h"
-#include "components/user_manager/user_manager.h"
 #include "dbus/message.h"
 #include "third_party/cros_system_api/dbus/service_constants.h"
 
@@ -33,14 +32,6 @@ constexpr char kFakeUUID[] = "74657374-4444-4444-8888-888888888888";
 constexpr char kShowSettingsPageDetails[] = "pluginVm/details";
 constexpr char kShowSettingsPageSharedPaths[] = "pluginVm/sharedPaths";
 
-namespace {
-Profile* GetPrimaryProfile() {
-  const user_manager::User* primary_user =
-      user_manager::UserManager::Get()->GetPrimaryUser();
-  return chromeos::ProfileHelper::Get()->GetProfileByUser(primary_user);
-}
-}  // namespace
-
 namespace chromeos {
 
 PluginVmServiceProvider::PluginVmServiceProvider() {}
@@ -118,7 +109,7 @@ void PluginVmServiceProvider::ShowSettingsPage(
     return;
   }
 
-  Profile* primary_profile = GetPrimaryProfile();
+  Profile* primary_profile = ProfileManager::GetPrimaryUserProfile();
   if (request.subpage_path() == kShowSettingsPageDetails) {
     chrome::ShowAppManagementPage(
         primary_profile, plugin_vm::kPluginVmShelfAppId,
@@ -145,8 +136,8 @@ void PluginVmServiceProvider::GetUserId(
   std::unique_ptr<dbus::Response> response =
       dbus::Response::FromMethodCall(method_call);
   plugin_vm_service::GetAppLicenseUserIdResponse payload;
-  payload.set_user_id(
-      plugin_vm::GetPluginVmUserIdForProfile(GetPrimaryProfile()));
+  payload.set_user_id(plugin_vm::GetPluginVmUserIdForProfile(
+      ProfileManager::GetPrimaryUserProfile()));
   dbus::MessageWriter writer(response.get());
   writer.AppendProtoAsArrayOfBytes(payload);
   std::move(response_sender).Run(std::move(response));
@@ -159,7 +150,7 @@ void PluginVmServiceProvider::GetPermissions(
       dbus::Response::FromMethodCall(method_call);
   plugin_vm_service::GetPermissionsResponse payload;
   payload.set_data_collection_enabled(
-      GetPrimaryProfile()->GetPrefs()->GetBoolean(
+      ProfileManager::GetPrimaryUserProfile()->GetPrefs()->GetBoolean(
           plugin_vm::prefs::kPluginVmDataCollectionAllowed));
   dbus::MessageWriter writer(response.get());
   writer.AppendProtoAsArrayOfBytes(payload);

chrome/browser/chromeos/exo/chrome_data_exchange_delegate.cc

@@ -27,9 +27,8 @@
 #include "chrome/browser/chromeos/guest_os/guest_os_share_path.h"
 #include "chrome/browser/chromeos/plugin_vm/plugin_vm_files.h"
 #include "chrome/browser/chromeos/plugin_vm/plugin_vm_util.h"
-#include "chrome/browser/chromeos/profiles/profile_helper.h"
 #include "chrome/browser/profiles/profile.h"
-#include "components/user_manager/user_manager.h"
+#include "chrome/browser/profiles/profile_manager.h"
 #include "content/public/common/drop_data.h"
 #include "storage/browser/file_system/external_mount_points.h"
 #include "storage/browser/file_system/file_system_context.h"
@@ -50,17 +49,6 @@ constexpr char kFileSchemePrefix[] = "file:";
 constexpr char kUriListSeparator[] = "\r\n";
 constexpr char kVmFileScheme[] = "vmfile";
 
-// We are using our own GetPrimaryProfile() rather than
-// ProfileManager::GetPrimaryUserProfile() to help unittest.
-Profile* GetPrimaryProfile() {
-  if (!user_manager::UserManager::IsInitialized())
-    return nullptr;
-  const auto* primary_user = user_manager::UserManager::Get()->GetPrimaryUser();
-  if (!primary_user)
-    return nullptr;
-  return chromeos::ProfileHelper::Get()->GetProfileByUser(primary_user);
-}
-
 // We implement our own URLToPath() and PathToURL() rather than use
 // net::FileUrlToFilePath() or net::FilePathToFileURL() since //net code does
 // not support Windows network paths such as //ChromeOS/MyFiles on OS_CHROMEOS.
@@ -114,7 +102,7 @@ std::string PathToURL(const std::string& path) {
 }
 
 storage::FileSystemContext* GetFileSystemContext() {
-  Profile* primary_profile = GetPrimaryProfile();
+  Profile* primary_profile = ProfileManager::GetPrimaryUserProfile();
   if (!primary_profile)
     return nullptr;
 
@@ -175,7 +163,7 @@ struct FileInfo {
 void ShareAndSend(aura::Window* target,
                   std::vector<FileInfo> files,
                   exo::DataExchangeDelegate::SendDataCallback callback) {
-  Profile* primary_profile = GetPrimaryProfile();
+  Profile* primary_profile = ProfileManager::GetPrimaryUserProfile();
   aura::Window* toplevel = target->GetToplevelWindow();
   bool is_crostini = crostini::IsCrostiniWindow(toplevel);
   bool is_plugin_vm = plugin_vm::IsPluginVmAppWindow(toplevel);
@@ -279,7 +267,7 @@ void ChromeDataExchangeDelegate::SetSourceOnOSExchangeData(
 std::vector<ui::FileInfo> ChromeDataExchangeDelegate::GetFilenames(
     aura::Window* source,
     const std::vector<uint8_t>& data) const {
-  Profile* primary_profile = GetPrimaryProfile();
+  Profile* primary_profile = ProfileManager::GetPrimaryUserProfile();
   aura::Window* toplevel = source->GetToplevelWindow();
   bool is_crostini = crostini::IsCrostiniWindow(toplevel);
   bool is_plugin_vm = plugin_vm::IsPluginVmAppWindow(toplevel);

chrome/browser/chromeos/file_manager/path_util.cc

@@ -32,6 +32,7 @@
 #include "chrome/browser/download/download_dir_util.h"
 #include "chrome/browser/download/download_prefs.h"
 #include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/profiles/profile_manager.h"
 #include "chromeos/constants/chromeos_features.h"
 #include "chromeos/disks/disk.h"
 #include "chromeos/disks/disk_mount_manager.h"
@@ -83,15 +84,6 @@ constexpr char kArcMyFilesContentUrlPrefix[] =
 constexpr char kArcDriveContentUrlPrefix[] =
     "content://org.chromium.arc.volumeprovider/MyDrive/";
 
-Profile* GetPrimaryProfile() {
-  if (!user_manager::UserManager::IsInitialized())
-    return nullptr;
-  const auto* primary_user = user_manager::UserManager::Get()->GetPrimaryUser();
-  if (!primary_user)
-    return nullptr;
-  return chromeos::ProfileHelper::Get()->GetProfileByUser(primary_user);
-}
-
 // Helper function for |ConvertToContentUrls|.
 void OnSingleContentUrlResolved(const base::RepeatingClosure& barrier_closure,
                                 std::vector<GURL>* out_urls,
@@ -561,7 +553,7 @@ bool ConvertPathToArcUrl(const base::FilePath& path, GURL* arc_url_out) {
 
   // Obtain the primary profile. This information is required because currently
   // only the file systems for the primary profile is exposed to ARC.
-  Profile* primary_profile = GetPrimaryProfile();
+  Profile* primary_profile = ProfileManager::GetPrimaryUserProfile();
   if (!primary_profile)
     return false;
 
@@ -734,8 +726,8 @@ void ConvertToContentUrls(
 void ConvertToContentUrls(
     const std::vector<storage::FileSystemURL>& file_system_urls,
     ConvertToContentUrlsCallback callback) {
-  ConvertToContentUrls(GetPrimaryProfile(), file_system_urls,
-                       std::move(callback));
+  ConvertToContentUrls(ProfileManager::GetPrimaryUserProfile(),
+                       file_system_urls, std::move(callback));
 }
 
 bool ReplacePrefix(std::string* s,

chrome/browser/profiles/profile_manager.cc

@@ -442,15 +442,8 @@ std::vector<Profile*> ProfileManager::GetLastOpenedProfiles() {
 
 // static
 Profile* ProfileManager::GetPrimaryUserProfile() {
-  ProfileManager* profile_manager = g_browser_process->profile_manager();
-  if (!profile_manager)  // Can be null in unit tests.
-    return nullptr;
 #if BUILDFLAG(IS_CHROMEOS_ASH)
-  if (!IsLoggedIn()) {
-    return profile_manager->GetActiveUserOrOffTheRecordProfileFromPath(
-        profile_manager->user_data_dir());
-  }
-
+  if (IsLoggedIn()) {
     user_manager::UserManager* manager = user_manager::UserManager::Get();
     const user_manager::User* user = manager->GetPrimaryUser();
     if (!user)  // Can be null in unit tests.
@@ -458,10 +451,15 @@ Profile* ProfileManager::GetPrimaryUserProfile() {
 
     // Note: The ProfileHelper will take care of guest profiles.
     return chromeos::ProfileHelper::Get()->GetProfileByUserUnsafe(user);
-#else
+  }
+#endif
+
+  ProfileManager* profile_manager = g_browser_process->profile_manager();
+  if (!profile_manager)  // Can be null in unit tests.
+    return nullptr;
+
   return profile_manager->GetActiveUserOrOffTheRecordProfileFromPath(
       profile_manager->user_data_dir());
-#endif
 }
 
 // static