[Office upload] Add IsEligibleAndEnabledUploadOfficeToCloud

Replace most checks for the UploadOfficeToCloud feature with IsEligibleAndEnabledUploadOfficeToCloud. IsEligibleAndEnabledUploadOfficeToCloud returns the result of IsUploadOfficeToCloudEnabled only if the user is non-managed or a Google employee, otherwise false is returned. Checks for the UploadOfficeToCloud feature were left untouched in chrome/browser/ash/file_system_provider/* because they will be removed. Tests: - FileHandlerDialogBrowserTest.* - FixUpFlowBrowserTest.* - CloudUploadDialogTest.* - FileHandlerPageTest.* - OfficeFallbackAppBrowserTest.* were updated to use a non-managed user email address so IsEligibleAndEnabledUploadOfficeToCloud will return true. Tests: - DriveTest.* - OneDriveTest.* - FileManagerFileTaskWithAppServiceTest.OfficePwaHandlerHidden (became NonManagedAccount.OfficePwaHandlerHidden) could not be updated the same way to ensure IsEligibleAndEnabledUploadOfficeToCloud will return true. A fake non- managed user with an account set up in LoggedInUserMixin is needed to ensure both the DriveFS and ODFS file systems could be mounted. Tests: - NonManagedAccount.IsEligibleAndEnabledUploadOfficeToCloud - ManagedAccount.IsEligibleAndEnabledUploadOfficeToCloud - GoogleAccount.IsEligibleAndEnabledUploadOfficeToCloud - NonManagedAccountNoFlag.IsEligibleAndEnabledUploadOfficeToCloud were added to test IsEligibleAndEnabledUploadOfficeToCloud. Bug: b:261353127 Change-Id: Ia1978fc67763f9d88eb4719b35467175e71e5e0a Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4230452 Commit-Queue: Cassy Chun-Crogan <cassycc@google.com> Reviewed-by: Kyle Horimoto <khorimoto@chromium.org> Reviewed-by: Luciano Pacheco <lucmult@chromium.org> Reviewed-by: Sam McNally <sammc@chromium.org> Cr-Commit-Position: refs/heads/main@{#1108719}
chromium · Feb 23, 2023 · 5b6d90f · 5b6d90f
1 parent 871ff77
commit 5b6d90f
Show file tree

Hide file tree

Showing 15 changed files with 301 additions and 70 deletions.
diff --git a/chrome/browser/ash/file_manager/file_tasks.cc b/chrome/browser/ash/file_manager/file_tasks.cc
@@ -333,7 +333,7 @@ void PostProcessFoundTasks(Profile* profile,
   disabled_actions.emplace("view-pdf");
 #endif  // !BUILDFLAG(ENABLE_PDF)
 
-  if (!ash::features::IsUploadOfficeToCloudEnabled()) {
+  if (!ash::cloud_upload::IsEligibleAndEnabledUploadOfficeToCloud()) {
     disabled_actions.emplace(kActionIdWebDriveOfficeWord);
     disabled_actions.emplace(kActionIdWebDriveOfficeExcel);
     disabled_actions.emplace(kActionIdWebDriveOfficePowerPoint);

diff --git a/chrome/browser/ash/file_manager/file_tasks_browsertest.cc b/chrome/browser/ash/file_manager/file_tasks_browsertest.cc
@@ -23,10 +23,12 @@
 #include "chrome/browser/apps/app_service/app_launch_params.h"
 #include "chrome/browser/apps/app_service/app_service_proxy.h"
 #include "chrome/browser/apps/app_service/app_service_proxy_factory.h"
+#include "chrome/browser/apps/app_service/app_service_test.h"
 #include "chrome/browser/apps/app_service/publishers/app_publisher.h"
 #include "chrome/browser/ash/arc/fileapi/arc_documents_provider_util.h"
 #include "chrome/browser/ash/drive/drivefs_test_support.h"
 #include "chrome/browser/ash/file_manager/app_id.h"
+#include "chrome/browser/ash/file_manager/file_manager_browsertest_base.h"
 #include "chrome/browser/ash/file_manager/file_manager_test_util.h"
 #include "chrome/browser/ash/file_manager/file_tasks.h"
 #include "chrome/browser/ash/file_manager/fileapi_util.h"
@@ -38,13 +40,15 @@
 #include "chrome/browser/ash/file_system_provider/provided_file_system_info.h"
 #include "chrome/browser/ash/file_system_provider/provider_interface.h"
 #include "chrome/browser/ash/file_system_provider/service.h"
+#include "chrome/browser/ash/login/test/logged_in_user_mixin.h"
 #include "chrome/browser/ash/policy/dlp/dlp_files_controller.h"
 #include "chrome/browser/ash/system_web_apps/system_web_app_manager.h"
 #include "chrome/browser/chromeos/policy/dlp/dlp_rules_manager.h"
 #include "chrome/browser/chromeos/policy/dlp/dlp_rules_manager_factory.h"
 #include "chrome/browser/chromeos/policy/dlp/mock_dlp_rules_manager.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/web_applications/test/web_app_browsertest_util.h"
 #include "chrome/browser/ui/web_applications/web_app_launch_manager.h"
 #include "chrome/browser/ui/webui/ash/cloud_upload/cloud_upload_dialog.h"
 #include "chrome/browser/ui/webui/ash/cloud_upload/cloud_upload_util.h"
@@ -58,6 +62,7 @@
 #include "chrome/common/chrome_paths.h"
 #include "chrome/common/webui_url_constants.h"
 #include "chrome/test/base/in_process_browser_test.h"
+#include "chrome/test/base/mixin_based_in_process_browser_test.h"
 #include "chromeos/ash/components/drivefs/fake_drivefs.h"
 #include "chromeos/ash/components/drivefs/mojom/drivefs.mojom.h"
 #include "components/drive/file_errors.h"
@@ -740,6 +745,159 @@ IN_PROC_BROWSER_TEST_P(FileTasksPolicyBrowserTest, TasksMarkedAsBlocked) {
   TestExpectationsAgainstDlp(expectations);
 }
 
+// |InProcessBrowserTest| which allows a fake user to login. Login a non-managed
+// or Google user to ensure |IsEligibleAndEnabledUploadOfficeToCloud| returns
+// the result of |IsUploadOfficeToCloudEnabled|.
+class TestAccountBrowserTest : public MixinBasedInProcessBrowserTest {
+ public:
+  TestAccountBrowserTest(TestAccountType test_account_type,
+                         bool is_google_account) {
+    ash::LoggedInUserMixin::LogInType log_in_type;
+    absl::optional<AccountId> account_id;
+    if (is_google_account) {
+      log_in_type = ash::LoggedInUserMixin::LogInType::kRegular;
+      account_id = AccountId::FromUserEmailGaiaId("user@google.com", "12345");
+    } else {
+      log_in_type = LogInTypeFor(test_account_type);
+      account_id = AccountIdFor(test_account_type);
+    }
+
+    logged_in_user_mixin_ = std::make_unique<ash::LoggedInUserMixin>(
+        &mixin_host_, log_in_type, embedded_test_server(), this,
+        /*should_launch_browser=*/true, account_id);
+  }
+
+ protected:
+  void SetUpOnMainThread() override {
+    MixinBasedInProcessBrowserTest::SetUpOnMainThread();
+    logged_in_user_mixin_->LogInUser();
+  }
+
+ private:
+  std::unique_ptr<ash::LoggedInUserMixin> logged_in_user_mixin_;
+};
+
+class NonManagedAccount : public TestAccountBrowserTest {
+ public:
+  NonManagedAccount()
+      : TestAccountBrowserTest(kNonManaged, /*is_google_account=*/false) {
+    feature_list_.InitAndEnableFeature(ash::features::kUploadOfficeToCloud);
+  }
+
+  void SetUpOnMainThread() override {
+    TestAccountBrowserTest::SetUpOnMainThread();
+    app_service_test_.SetUp(browser()->profile());
+  }
+  apps::AppServiceProxy* app_service_proxy() {
+    apps::AppServiceProxy* app_service_proxy =
+        apps::AppServiceProxyFactory::GetForProfile(browser()->profile());
+    CHECK(app_service_proxy);
+    return app_service_proxy;
+  }
+
+ private:
+  base::test::ScopedFeatureList feature_list_;
+  apps::AppServiceTest app_service_test_;
+};
+
+// Tests that a |IsEligibleAndEnabledUploadOfficeToCloud| returns true when a
+// non-managed user is logged in and |kUploadOfficeToCloud| is enabled.
+IN_PROC_BROWSER_TEST_F(NonManagedAccount,
+                       IsEligibleAndEnabledUploadOfficeToCloud) {
+  ASSERT_TRUE(ash::cloud_upload::IsEligibleAndEnabledUploadOfficeToCloud());
+}
+
+// Test that the office PWA file handler is hidden from the available file
+// handlers when opening an office file and the |kUploadOfficeToCloud| flag is
+// enabled.
+IN_PROC_BROWSER_TEST_F(NonManagedAccount, OfficePwaHandlerHidden) {
+  struct FakeOfficeFileType {
+    std::string file_extension;
+    std::string mime_type;
+  };
+
+  std::vector<FakeOfficeFileType> fake_office_file_types = {
+      {"ppt", "application/vnd.ms-powerpoint"},
+      {"pptx",
+       "application/"
+       "vnd.openxmlformats-officedocument.presentationml.presentation"},
+      {"xls", "application/vnd.ms-excel"},
+      {"xlsx",
+       "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet"},
+      {"doc", "application/msword"},
+      {"docx",
+       "application/"
+       "vnd.openxmlformats-officedocument.wordprocessingml.document"}};
+
+  for (FakeOfficeFileType& fake_office_file_type : fake_office_file_types) {
+    file_manager::test::AddFakeWebApp(extension_misc::kOfficePwaAppId,
+                                      fake_office_file_type.mime_type,
+                                      fake_office_file_type.file_extension,
+                                      "something", true, app_service_proxy());
+
+    base::FilePath test_file_path = web_app::CreateTestFileWithExtension(
+        fake_office_file_type.file_extension);
+
+    std::vector<file_manager::file_tasks::FullTaskDescriptor> tasks =
+        file_manager::test::GetTasksForFile(browser()->profile(),
+                                            test_file_path);
+
+    for (FullTaskDescriptor& task : tasks) {
+      EXPECT_NE(extension_misc::kOfficePwaAppId, task.task_descriptor.app_id)
+          << " for extension: " << fake_office_file_type.file_extension;
+    }
+  }
+}
+
+class ManagedAccount : public TestAccountBrowserTest {
+ public:
+  ManagedAccount()
+      : TestAccountBrowserTest(kEnterprise, /*is_google_account=*/false) {
+    feature_list_.InitAndEnableFeature(ash::features::kUploadOfficeToCloud);
+  }
+
+ private:
+  base::test::ScopedFeatureList feature_list_;
+};
+
+// Tests that a |IsEligibleAndEnabledUploadOfficeToCloud| returns false when a
+// managed user is logged in and |kUploadOfficeToCloud| is enabled.
+IN_PROC_BROWSER_TEST_F(ManagedAccount,
+                       IsEligibleAndEnabledUploadOfficeToCloud) {
+  ASSERT_FALSE(ash::cloud_upload::IsEligibleAndEnabledUploadOfficeToCloud());
+}
+
+class GoogleAccount : public TestAccountBrowserTest {
+ public:
+  GoogleAccount()
+      : TestAccountBrowserTest(kTestAccountTypeNotSet,
+                               /*is_google_account=*/true) {
+    feature_list_.InitAndEnableFeature(ash::features::kUploadOfficeToCloud);
+  }
+
+ private:
+  base::test::ScopedFeatureList feature_list_;
+};
+
+// Tests that a |IsEligibleAndEnabledUploadOfficeToCloud| returns true when a
+// google user is logged in and |kUploadOfficeToCloud| is enabled.
+IN_PROC_BROWSER_TEST_F(GoogleAccount, IsEligibleAndEnabledUploadOfficeToCloud) {
+  ASSERT_TRUE(ash::cloud_upload::IsEligibleAndEnabledUploadOfficeToCloud());
+}
+
+class NonManagedAccountNoFlag : public TestAccountBrowserTest {
+ public:
+  NonManagedAccountNoFlag()
+      : TestAccountBrowserTest(kNonManaged, /*is_google_account=*/false) {}
+};
+
+// Tests that a |IsEligibleAndEnabledUploadOfficeToCloud| returns false when a
+// non-managed user is logged in but |kUploadOfficeToCloud| is disabled.
+IN_PROC_BROWSER_TEST_F(NonManagedAccountNoFlag,
+                       IsEligibleAndEnabledUploadOfficeToCloud) {
+  ASSERT_FALSE(ash::cloud_upload::IsEligibleAndEnabledUploadOfficeToCloud());
+}
+
 // TODO(cassycc): move this class to a more appropriate spot.
 // Fake DriveFs specific to the `DriveTest`. Allows a test file to
 // be "added" to the DriveFs via `SetMetadata()`. The `alternate_url` of the
@@ -807,9 +965,10 @@ class FakeSimpleDriveFsHelper : public drive::FakeDriveFsHelper {
 // testing with a fake DriveFs.
 // Tests the office fallback flow that occurs when
 // a user fails to open an office file from Drive.
-class DriveTest : public InProcessBrowserTest {
+class DriveTest : public TestAccountBrowserTest {
  public:
-  DriveTest() {
+  DriveTest()
+      : TestAccountBrowserTest(kNonManaged, /*is_google_account=*/false) {
     feature_list_.InitAndEnableFeature(ash::features::kUploadOfficeToCloud);
     EXPECT_TRUE(temp_dir_.CreateUniqueTempDir());
     drive_mount_point_ = temp_dir_.GetPath();
@@ -831,7 +990,7 @@ class DriveTest : public InProcessBrowserTest {
   }
 
   void TearDown() override {
-    InProcessBrowserTest::TearDown();
+    TestAccountBrowserTest::TearDown();
     storage::ExternalMountPoints::GetSystemInstance()->RevokeAllFileSystems();
   }
 
@@ -1202,9 +1361,10 @@ class FakeWebAppPublisher : public apps::AppPublisher {
 // testing with a fake ODFS.
 // Tests the office fallback flow that occurs when a
 // user fails to open an office file from ODFS.
-class OneDriveTest : public InProcessBrowserTest {
+class OneDriveTest : public TestAccountBrowserTest {
  public:
-  OneDriveTest() {
+  OneDriveTest()
+      : TestAccountBrowserTest(kNonManaged, /*is_google_account=*/false) {
     feature_list_.InitAndEnableFeature(ash::features::kUploadOfficeToCloud);
     test_file_name_ = "text.docx";
     // Relative path for a file on ODFS and Android OneDrive.
@@ -1219,7 +1379,7 @@ class OneDriveTest : public InProcessBrowserTest {
   OneDriveTest& operator=(const OneDriveTest&) = delete;
 
   void TearDown() override {
-    InProcessBrowserTest::TearDown();
+    TestAccountBrowserTest::TearDown();
     storage::ExternalMountPoints::GetSystemInstance()->RevokeAllFileSystems();
   }
 

diff --git a/chrome/browser/ash/file_manager/file_tasks_unittest.cc b/chrome/browser/ash/file_manager/file_tasks_unittest.cc
@@ -800,51 +800,6 @@ TEST_F(FileManagerFileTaskPreferencesTest, SetOfficeFileHandlersToFilesSWA) {
   ASSERT_EQ(task, default_task);
 }
 
-// Test that the office PWA file handler is hidden from the available file
-// handlers when opening an office file.
-TEST_F(FileManagerFileTaskWithAppServiceTest, OfficePwaHandlerHidden) {
-  struct FakeOfficeFileType {
-    std::string file_extension;
-    std::string mime_type;
-  };
-
-  // Enable `kUploadOfficeToCloud` flag as the hiding happens behind this
-  // flag.
-  base::test::ScopedFeatureList scoped_feature_list{
-      ash::features::kUploadOfficeToCloud};
-
-  std::vector<FakeOfficeFileType> fake_office_file_types = {
-      {"ppt", "application/vnd.ms-powerpoint"},
-      {"pptx",
-       "application/"
-       "vnd.openxmlformats-officedocument.presentationml.presentation"},
-      {"xls", "application/vnd.ms-excel"},
-      {"xlsx",
-       "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet"},
-      {"doc", "application/msword"},
-      {"docx",
-       "application/"
-       "vnd.openxmlformats-officedocument.wordprocessingml.document"}};
-
-  for (FakeOfficeFileType& fake_office_file_type : fake_office_file_types) {
-    file_manager::test::AddFakeWebApp(extension_misc::kOfficePwaAppId,
-                                      fake_office_file_type.mime_type,
-                                      fake_office_file_type.file_extension,
-                                      "something", true, app_service_proxy());
-
-    base::FilePath test_file_path = web_app::CreateTestFileWithExtension(
-        fake_office_file_type.file_extension);
-
-    std::vector<file_manager::file_tasks::FullTaskDescriptor> tasks =
-        file_manager::test::GetTasksForFile(profile(), test_file_path);
-
-    for (FullTaskDescriptor& task : tasks) {
-      EXPECT_NE(extension_misc::kOfficePwaAppId, task.task_descriptor.app_id)
-          << " for extension: " << fake_office_file_type.file_extension;
-    }
-  }
-}
-
 // Test using the test extension system, which needs lots of setup.
 class FileManagerFileTasksComplexTest : public testing::Test {
  protected:

diff --git a/chrome/browser/chrome_browser_interface_binders.cc b/chrome/browser/chrome_browser_interface_binders.cc
@@ -267,6 +267,7 @@
 #include "chrome/browser/ui/webui/ash/audio/audio_ui.h"
 #include "chrome/browser/ui/webui/ash/bluetooth_pairing_dialog.h"
 #include "chrome/browser/ui/webui/ash/cloud_upload/cloud_upload.mojom.h"
+#include "chrome/browser/ui/webui/ash/cloud_upload/cloud_upload_dialog.h"
 #include "chrome/browser/ui/webui/ash/cloud_upload/cloud_upload_ui.h"
 #include "chrome/browser/ui/webui/ash/crostini_installer/crostini_installer.mojom.h"
 #include "chrome/browser/ui/webui/ash/crostini_installer/crostini_installer_ui.h"
@@ -1358,13 +1359,13 @@ void PopulateChromeWebUIFrameBinders(
         ash::ManageMirrorSyncUI>(map);
   }
 
-  if (ash::features::IsUploadOfficeToCloudEnabled()) {
+  if (ash::cloud_upload::IsEligibleAndEnabledUploadOfficeToCloud()) {
     RegisterWebUIControllerInterfaceBinder<
         ash::cloud_upload::mojom::PageHandlerFactory,
         ash::cloud_upload::CloudUploadUI>(map);
   }
 
-  if (ash::features::IsUploadOfficeToCloudEnabled()) {
+  if (ash::cloud_upload::IsEligibleAndEnabledUploadOfficeToCloud()) {
     RegisterWebUIControllerInterfaceBinder<
         ash::office_fallback::mojom::PageHandlerFactory,
         ash::office_fallback::OfficeFallbackUI>(map);

diff --git a/chrome/browser/ui/webui/ash/cloud_upload/cloud_upload_dialog.cc b/chrome/browser/ui/webui/ash/cloud_upload/cloud_upload_dialog.cc
@@ -4,6 +4,7 @@
 
 #include "chrome/browser/ui/webui/ash/cloud_upload/cloud_upload_dialog.h"
 
+#include "ash/constants/ash_features.h"
 #include "base/files/file_path.h"
 #include "base/functional/bind.h"
 #include "base/functional/callback.h"
@@ -17,16 +18,21 @@
 #include "chrome/browser/ash/arc/fileapi/arc_documents_provider_util.h"
 #include "chrome/browser/ash/file_manager/file_tasks.h"
 #include "chrome/browser/ash/file_manager/open_with_browser.h"
+#include "chrome/browser/ash/file_system_provider/mount_path_util.h"
+#include "chrome/browser/browser_process.h"
 #include "chrome/browser/ui/webui/ash/cloud_upload/cloud_upload.mojom-forward.h"
 #include "chrome/browser/ui/webui/ash/cloud_upload/cloud_upload.mojom-shared.h"
 #include "chrome/browser/ui/webui/ash/cloud_upload/cloud_upload_ui.h"
 #include "chrome/browser/ui/webui/ash/cloud_upload/drive_upload_handler.h"
 #include "chrome/browser/ui/webui/ash/cloud_upload/one_drive_upload_handler.h"
 #include "chrome/browser/web_applications/web_app_id_constants.h"
 #include "chrome/common/webui_url_constants.h"
+#include "chromeos/ash/components/browser_context_helper/browser_context_helper.h"
 #include "components/services/app_service/public/cpp/types_util.h"
+#include "components/user_manager/user_manager.h"
 #include "extensions/browser/api/file_handlers/mime_util.h"
 #include "extensions/browser/entry_info.h"
+#include "google_apis/gaia/gaia_auth_util.h"
 #include "storage/browser/file_system/file_system_url.h"
 #include "ui/gfx/geometry/size.h"
 
@@ -315,6 +321,43 @@ void LaunchLocalFileTask(
 }
 }  // namespace
 
+bool IsEligibleAndEnabledUploadOfficeToCloud() {
+  user_manager::UserManager* user_manager = user_manager::UserManager::Get();
+  if (!user_manager) {
+    return false;
+  }
+
+  user_manager::User* user = user_manager->GetActiveUser();
+  if (!user) {
+    return false;
+  }
+
+  // |profile_manager| can be null in unit tests, even though a user was
+  // created. If it is null, `GetBrowserContextByUser` call will cause crash.
+  auto* profile_manager = g_browser_process->profile_manager();
+  if (!profile_manager) {
+    return false;
+  }
+
+  Profile* profile = Profile::FromBrowserContext(
+      BrowserContextHelper::Get()->GetBrowserContextByUser(user));
+  if (!profile) {
+    return false;
+  }
+
+  // Managed users, e.g. enterprise account, child account, are not eligible
+  // with the exception of Google employees. `GetUserCloudPolicyManagerAsh`
+  // returns non-nullptr if a profile is a managed account. This approach is
+  // taken in `UserTypeByDeviceTypeMetricsProvider::GetUserSegment`.
+  if (profile->GetUserCloudPolicyManagerAsh() &&
+      !gaia::IsGoogleInternalAccountEmail(
+          user->GetAccountId().GetUserEmail())) {
+    return false;
+  }
+
+  return features::IsUploadOfficeToCloudEnabled();
+}
+
 void OnDialogComplete(
     Profile* profile,
     const std::vector<storage::FileSystemURL>& file_urls,