Avoid requesting the Drive mount path if Drive is disabled.

Guest mode profiles do not have a valid account ID key so requesting
their DriveFS path is invalid. Such accounts never have Drive enabled
and the downloads path should not be within Drive if disabled.

Update DriveIntegrationService to avoid paths within Drive when
disabling Drive with DriveFS enabled so the above assumption is valid
with DriveFS enabled.

Bug: 904558
Change-Id: Ic8b64609f3657feae4b1f289e3726a51a871b36c
Reviewed-on: https://chromium-review.googlesource.com/c/1333091Reviewed-by: Sergei Datsenko <dats@chromium.org>
Commit-Queue: Sam McNally <sammc@chromium.org>
Cr-Commit-Position: refs/heads/master@{#607501}

chrome/browser/chromeos/drive/drive_integration_service.cc

@@ -758,7 +758,7 @@ void DriveIntegrationService::SetEnabled(bool enabled) {
   // want to run the check for the first SetEnabled() called in the constructor,
   // which may be a change from false to false.
   if (!enabled)
-    AvoidDriveAsDownloadDirecotryPreference();
+    AvoidDriveAsDownloadDirectoryPreference();
 
   // Do nothing if not changed.
   if (enabled_ == enabled)
@@ -1138,7 +1138,7 @@ void DriveIntegrationService::InitializeAfterMetadataInitialized(
     LOG(WARNING) << "Failed to initialize: " << FileErrorToString(error);
 
     // Cannot used Drive. Set the download destination preference out of Drive.
-    AvoidDriveAsDownloadDirecotryPreference();
+    AvoidDriveAsDownloadDirectoryPreference();
 
     // Back to NOT_INITIALIZED state. Then, re-running Initialize() should
     // work if the error is recoverable manually (such as out of disk space).
@@ -1195,16 +1195,26 @@ void DriveIntegrationService::InitializeAfterMetadataInitialized(
     AddDriveMountPoint();
 }
 
-void DriveIntegrationService::AvoidDriveAsDownloadDirecotryPreference() {
-  PrefService* pref_service = profile_->GetPrefs();
-  if (util::IsUnderDriveMountPoint(
-          pref_service->GetFilePath(::prefs::kDownloadDefaultDirectory))) {
-    pref_service->SetFilePath(
+void DriveIntegrationService::AvoidDriveAsDownloadDirectoryPreference() {
+  if (DownloadDirectoryPreferenceIsInDrive()) {
+    profile_->GetPrefs()->SetFilePath(
         ::prefs::kDownloadDefaultDirectory,
         file_manager::util::GetDownloadsFolderForProfile(profile_));
   }
 }
 
+bool DriveIntegrationService::DownloadDirectoryPreferenceIsInDrive() {
+  const auto downloads_path =
+      profile_->GetPrefs()->GetFilePath(::prefs::kDownloadDefaultDirectory);
+
+  if (!drivefs_holder_) {
+    return util::IsUnderDriveMountPoint(downloads_path);
+  }
+  const auto* user = chromeos::ProfileHelper::Get()->GetUserByProfile(profile_);
+  return user && user->GetAccountId().HasAccountIdKey() &&
+         GetMountPointPath().IsParent(downloads_path);
+}
+
 void DriveIntegrationService::Observe(
     int type,
     const content::NotificationSource& source,

chrome/browser/chromeos/drive/drive_integration_service.h

@@ -233,7 +233,9 @@ class DriveIntegrationService : public KeyedService,
 
   // Change the download directory to the local "Downloads" if the download
   // destination is set under Drive. This must be called when disabling Drive.
-  void AvoidDriveAsDownloadDirecotryPreference();
+  void AvoidDriveAsDownloadDirectoryPreference();
+
+  bool DownloadDirectoryPreferenceIsInDrive();
 
   // content::NotificationObserver overrides.
   void Observe(int type,

chrome/browser/chromeos/file_manager/path_util.cc

@@ -409,6 +409,9 @@ std::string GetPathDisplayTextForSettings(Profile* profile,
   std::string result(path);
   auto* drive_integration_service =
       drive::DriveIntegrationServiceFactory::FindForProfile(profile);
+  if (drive_integration_service && !drive_integration_service->is_enabled()) {
+    drive_integration_service = nullptr;
+  }
   if (ReplacePrefix(&result, "/home/chronos/user/Downloads",
                     kFolderNameDownloads)) {
   } else if (ReplacePrefix(&result,

chrome/browser/chromeos/file_manager/path_util_unittest.cc

@@ -138,7 +138,8 @@ TEST(FileManagerPathUtilTest, GetPathDisplayTextForSettings) {
   {
     base::test::ScopedFeatureList features;
     features.InitAndDisableFeature(chromeos::features::kDriveFs);
-    drive::DriveIntegrationServiceFactory::GetForProfile(&profile);
+    drive::DriveIntegrationServiceFactory::GetForProfile(&profile)->SetEnabled(
+        true);
     EXPECT_EQ("Google Drive \u203a My Drive \u203a foo",
               GetPathDisplayTextForSettings(
                   &profile, "/special/drive-0123456789abcdef/root/foo"));
@@ -166,7 +167,8 @@ TEST(FileManagerPathUtilTest, GetPathDisplayTextForSettings) {
     PrefService* prefs = profile2.GetPrefs();
     prefs->SetString(drive::prefs::kDriveFsProfileSalt, "a");
 
-    drive::DriveIntegrationServiceFactory::GetForProfile(&profile2);
+    drive::DriveIntegrationServiceFactory::GetForProfile(&profile2)->SetEnabled(
+        true);
     EXPECT_EQ(
         "Google Drive \u203a My Drive \u203a foo",
         GetPathDisplayTextForSettings(
@@ -183,6 +185,18 @@ TEST(FileManagerPathUtilTest, GetPathDisplayTextForSettings) {
             &profile2,
             "/media/fuse/drivefs-84675c855b63e12f384d45f033826980/"
             "Computers/My Other Computer/bar"));
+
+    TestingProfile guest_profile(base::FilePath("/home/chronos/guest"));
+    guest_profile.SetGuestSession(true);
+    guest_profile.set_profile_name("$guest");
+    ASSERT_TRUE(
+        drive::DriveIntegrationServiceFactory::GetForProfile(&guest_profile));
+
+    EXPECT_EQ("Downloads", GetPathDisplayTextForSettings(
+                               &guest_profile, "/home/chronos/user/Downloads"));
+    // Test that a passthrough path doesn't crash on requesting the Drive mount
+    // path for a guest profile.
+    EXPECT_EQ("foo", GetPathDisplayTextForSettings(&guest_profile, "foo"));
   }
   chromeos::disks::DiskMountManager::Shutdown();
 }