DLP: Add feature flag for new UX

Add Chrome feature flag for Files policy new UX so it's safer to enable/disable and easier to test.

Bug: b/296378836
Change-Id: I91a1eaf5c43658dad4b135797be7a44b69884e59
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4789272
Reviewed-by: Luciano Pacheco <lucmult@chromium.org>
Reviewed-by: Aida Zolic <aidazolic@chromium.org>
Reviewed-by: Sergey Poromov <poromov@chromium.org>
Commit-Queue: Aya Elsayed <ayaelattar@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1185079}

chrome/browser/ash/extensions/file_manager/private_api_file_system.cc

@@ -966,7 +966,7 @@ FileManagerPrivateInternalGetDisallowedTransfersFunction::Run() {
 
   // If the new UX flow is enabled, return an empty list so the copy/move
   // operation can start.
-  if (policy::DlpFilesController::kNewFilesPolicyUXEnabled) {
+  if (base::FeatureList::IsEnabled(features::kNewFilesPolicyUX)) {
     return RespondNow(WithArguments(base::Value::List()));
   }
 

chrome/browser/ash/file_manager/copy_or_move_io_task_policy_impl.cc

@@ -236,7 +236,7 @@ void CopyOrMoveIOTaskPolicyImpl::VerifyTransfer() {
 
   if (auto* files_controller =
           policy::DlpFilesControllerAsh::GetForPrimaryProfile();
-      policy::DlpFilesController::kNewFilesPolicyUXEnabled &&
+      base::FeatureList::IsEnabled(features::kNewFilesPolicyUX) &&
       files_controller) {
     std::vector<storage::FileSystemURL> transferred_urls;
     for (const auto& entry : progress_->sources) {

chrome/browser/ash/file_manager/copy_or_move_io_task_policy_unittest.cc

@@ -210,9 +210,6 @@ class CopyOrMoveIOTaskWithScansTest
             base::BindRepeating(&CopyOrMoveIOTaskWithScansTest::SetupMock,
                                 base::Unretained(this))));
 
-    policy::DlpFilesController::SetNewFilesPolicyUXEnabledForTesting(
-        /*is_enabled=*/UseNewUI());
-
     if (UseNewUI()) {
       // Set FilesPolicyNotificationManager.
       policy::FilesPolicyNotificationManagerFactory::GetInstance()
@@ -1349,8 +1346,7 @@ class CopyOrMoveIOTaskWithDLPTest : public testing::Test {
   }
 
   void SetUp() override {
-    policy::DlpFilesController::SetNewFilesPolicyUXEnabledForTesting(
-        /*is_enabled=*/true);
+    scoped_feature_list_.InitAndEnableFeature(features::kNewFilesPolicyUX);
 
     AccountId account_id = AccountId::FromUserEmailGaiaId(kEmailId, kGaiaId);
     profile_->SetIsNewProfile(true);
@@ -1392,6 +1388,7 @@ class CopyOrMoveIOTaskWithDLPTest : public testing::Test {
         base::FilePath::FromUTF8Unsafe(path));
   }
 
+  base::test::ScopedFeatureList scoped_feature_list_;
   content::BrowserTaskEnvironment task_environment_;
   ash::disks::FakeDiskMountManager disk_mount_manager_;
   raw_ptr<policy::MockDlpRulesManager, DanglingUntriaged | ExperimentalAsh>

chrome/browser/ash/file_manager/file_manager_browsertest.cc

@@ -192,6 +192,11 @@ struct TestCase {
     return *this;
   }
 
+  TestCase& EnableFilesPolicyNewUX() {
+    options.enable_files_policy_new_ux = true;
+    return *this;
+  }
+
   TestCase& EnableDriveTrash() {
     options.enable_drive_trash = true;
     return *this;
@@ -706,10 +711,6 @@ class DlpFilesAppBrowserTest : public FilesAppBrowserTest {
           add_files_cb.Get());
       return true;
     }
-    if (name == "enableNewFilesPolicyUX") {
-      policy::DlpFilesController::SetNewFilesPolicyUXEnabledForTesting(true);
-      return true;
-    }
     if (name == "setCheckFilesTransferMockToPause") {
       base::FilePath download_path =
           file_manager::util::GetDownloadsFolderForProfile(profile());
@@ -1865,8 +1866,8 @@ WRAPPED_INSTANTIATE_TEST_SUITE_P(
 #endif
         TestCase("fileTasksDlpRestricted").EnableDlp(),
         TestCase("zipExtractRestrictedArchiveCheckContent").EnableDlp(),
-        TestCase("blockShowsPanelItem").EnableDlp(),
-        TestCase("warnShowsPanelItem").EnableDlp()));
+        TestCase("blockShowsPanelItem").EnableDlp().EnableFilesPolicyNewUX(),
+        TestCase("warnShowsPanelItem").EnableDlp().EnableFilesPolicyNewUX()));
 
 WRAPPED_INSTANTIATE_TEST_SUITE_P(
     DriveSpecific, /* drive_specific.js */

chrome/browser/ash/file_manager/file_manager_browsertest_base.cc

@@ -2164,6 +2164,12 @@ void FileManagerBrowserTestBase::SetUpCommandLine(
     disabled_features.push_back(features::kDataLeakPreventionFilesRestriction);
   }
 
+  if (options.enable_files_policy_new_ux) {
+    enabled_features.push_back(features::kNewFilesPolicyUX);
+  } else {
+    disabled_features.push_back(features::kNewFilesPolicyUX);
+  }
+
   if (options.enable_mirrorsync) {
     enabled_features.push_back(ash::features::kDriveFsMirroring);
   } else {

chrome/browser/ash/file_manager/file_manager_browsertest_base.h

@@ -147,6 +147,9 @@ class FileManagerBrowserTestBase
     // feature.
     bool enable_dlp_files_restriction = false;
 
+    // Whether test should enable Files policy new UX feature.
+    bool enable_files_policy_new_ux = false;
+
     // Whether test should run with the Upload Office to Cloud feature.
     bool enable_upload_office_to_cloud = false;
 

chrome/browser/ash/policy/dlp/dialogs/files_policy_dialog_browsertest.cc

@@ -17,12 +17,12 @@
 #include "chrome/browser/ash/system_web_apps/system_web_app_manager.h"
 #include "chrome/browser/chromeos/policy/dlp/dlp_confidential_file.h"
 #include "chrome/browser/chromeos/policy/dlp/dlp_file_destination.h"
-#include "chrome/browser/chromeos/policy/dlp/dlp_files_controller.h"
 #include "chrome/browser/chromeos/policy/dlp/dlp_files_utils.h"
 #include "chrome/browser/chromeos/policy/dlp/dlp_policy_constants.h"
 #include "chrome/browser/enterprise/data_controls/component.h"
 #include "chrome/browser/ui/ash/system_web_apps/system_web_app_ui_utils.h"
 #include "chrome/browser/ui/browser_window.h"
+#include "chrome/common/chrome_features.h"
 #include "chrome/test/base/in_process_browser_test.h"
 #include "chrome/test/base/ui_test_utils.h"
 #include "content/public/test/browser_test.h"
@@ -33,16 +33,16 @@ class FilesPolicyDialogBrowserTest
     : public InProcessBrowserTest,
       public ::testing::WithParamInterface<dlp::FileAction> {
  public:
-  FilesPolicyDialogBrowserTest() = default;
+  FilesPolicyDialogBrowserTest() {
+    scoped_feature_list_.InitAndEnableFeature(features::kNewFilesPolicyUX);
+  }
   FilesPolicyDialogBrowserTest(const FilesPolicyDialogBrowserTest&) = delete;
   FilesPolicyDialogBrowserTest& operator=(const FilesPolicyDialogBrowserTest&) =
       delete;
   ~FilesPolicyDialogBrowserTest() override = default;
 
   void SetUpOnMainThread() override {
     InProcessBrowserTest::SetUpOnMainThread();
-    DlpFilesController::SetNewFilesPolicyUXEnabledForTesting(
-        /*is_enabled=*/true);
 
     // Setup the Files app.
     ash::SystemWebAppManager::GetForTest(browser()->profile())
@@ -71,6 +71,8 @@ class FilesPolicyDialogBrowserTest
     run_loop.Run();
     return ui_test_utils::WaitForBrowserToOpen();
   }
+
+  base::test::ScopedFeatureList scoped_feature_list_;
 };
 
 class WarningDialogBrowserTest : public FilesPolicyDialogBrowserTest {

chrome/browser/ash/policy/dlp/dialogs/files_policy_warn_dialog.cc

@@ -14,6 +14,7 @@
 #include "chrome/browser/chromeos/policy/dlp/dlp_files_controller.h"
 #include "chrome/browser/chromeos/policy/dlp/dlp_files_utils.h"
 #include "chrome/browser/enterprise/data_controls/component.h"
+#include "chrome/common/chrome_features.h"
 #include "components/strings/grit/components_strings.h"
 #include "third_party/abseil-cpp/absl/types/optional.h"
 #include "ui/base/l10n/l10n_util.h"
@@ -113,7 +114,7 @@ std::u16string FilesPolicyWarnDialog::GetCancelButton() {
 }
 
 std::u16string FilesPolicyWarnDialog::GetTitle() {
-  if (DlpFilesController::kNewFilesPolicyUXEnabled) {
+  if (base::FeatureList::IsEnabled(features::kNewFilesPolicyUX)) {
     switch (action_) {
       case dlp::FileAction::kDownload:
         return l10n_util::GetStringUTF16(
@@ -167,7 +168,7 @@ std::u16string FilesPolicyWarnDialog::GetTitle() {
 }
 
 std::u16string FilesPolicyWarnDialog::GetMessage() {
-  if (DlpFilesController::kNewFilesPolicyUXEnabled) {
+  if (base::FeatureList::IsEnabled(features::kNewFilesPolicyUX)) {
     return base::ReplaceStringPlaceholders(
         l10n_util::GetPluralStringFUTF16(IDS_POLICY_DLP_FILES_WARN_MESSAGE,
                                          files_.size()),

chrome/browser/ash/policy/dlp/dlp_files_controller_ash.cc

@@ -52,6 +52,7 @@
 #include "chrome/browser/ui/ash/system_web_apps/system_web_app_ui_utils.h"
 #include "chrome/browser/ui/browser_window.h"
 #include "chrome/browser/ui/webui/ash/cloud_upload/cloud_upload_dialog.h"
+#include "chrome/common/chrome_features.h"
 #include "chrome/common/extensions/extension_constants.h"
 #include "chromeos/dbus/dlp/dlp_client.h"
 #include "chromeos/dbus/dlp/dlp_service.pb.h"
@@ -1032,7 +1033,8 @@ void DlpFilesControllerAsh::ReturnDisallowedFiles(
     restricted_files_urls.push_back(files_map.at(file));
     restricted_files_paths.emplace_back(file);
   }
-  if (!restricted_files_paths.empty() && kNewFilesPolicyUXEnabled &&
+  if (!restricted_files_paths.empty() &&
+      base::FeatureList::IsEnabled(features::kNewFilesPolicyUX) &&
       task_id.has_value()) {
     ShowDlpBlockedFiles(std::move(task_id), std::move(restricted_files_paths),
                         file_action);

chrome/browser/ash/policy/dlp/dlp_files_controller_ash_unittest.cc

@@ -48,6 +48,7 @@
 #include "chrome/browser/chromeos/policy/dlp/dlp_reporting_manager.h"
 #include "chrome/browser/chromeos/policy/dlp/test/dlp_files_test_base.h"
 #include "chrome/browser/enterprise/data_controls/component.h"
+#include "chrome/common/chrome_features.h"
 #include "chromeos/dbus/dlp/dlp_client.h"
 #include "chromeos/ui/base/file_icon_util.h"
 #include "components/drive/drive_pref_names.h"
@@ -203,6 +204,9 @@ class DlpFilesControllerAshTest : public DlpFilesTestWithMounts {
 };
 
 TEST_F(DlpFilesControllerAshTest, CheckIfTransferAllowed_DiffFileSystem) {
+  base::test::ScopedFeatureList scoped_feature_list;
+  scoped_feature_list.InitAndEnableFeature(features::kNewFilesPolicyUX);
+
   std::vector<FileDaemonInfo> files{
       FileDaemonInfo(kInode1, kCrtime1, my_files_dir_.AppendASCII(kFilePath1),
                      kExampleUrl1, kReferrerUrl1),
@@ -233,7 +237,6 @@ TEST_F(DlpFilesControllerAshTest, CheckIfTransferAllowed_DiffFileSystem) {
       blink::StorageKey(), "archive",
       base::FilePath("file.rar/path/in/archive"));
 
-  DlpFilesController::SetNewFilesPolicyUXEnabledForTesting(/*is_enabled=*/true);
   EXPECT_CALL(*fpnm_, ShowDlpBlockedFiles(
                           /*task_id=*/{1},
                           std::vector<base::FilePath>{files_urls[0].path(),
@@ -386,6 +389,9 @@ TEST_F(DlpFilesControllerAshTest, CheckIfTransferAllowed_ErrorResponse) {
 }
 
 TEST_F(DlpFilesControllerAshTest, CheckIfTransferAllowed_MultiFolder) {
+  base::test::ScopedFeatureList scoped_feature_list;
+  scoped_feature_list.InitAndEnableFeature(features::kNewFilesPolicyUX);
+
   base::ScopedTempDir sub_dir1;
   ASSERT_TRUE(sub_dir1.CreateUniqueTempDirUnderPath(my_files_dir_));
   base::ScopedTempDir sub_dir2;
@@ -428,7 +434,6 @@ TEST_F(DlpFilesControllerAshTest, CheckIfTransferAllowed_MultiFolder) {
   chromeos::DlpClient::Get()->GetTestInterface()->SetCheckFilesTransferResponse(
       check_files_transfer_response);
 
-  DlpFilesController::SetNewFilesPolicyUXEnabledForTesting(/*is_enabled=*/true);
   EXPECT_CALL(*fpnm_, ShowDlpBlockedFiles(
                           /*task_id=*/{1},
                           std::vector<base::FilePath>{files_urls[1].path(),

chrome/browser/ash/policy/dlp/files_policy_notification_manager.cc

@@ -29,14 +29,14 @@
 #include "chrome/browser/chromeos/policy/dlp/dialogs/policy_dialog_base.h"
 #include "chrome/browser/chromeos/policy/dlp/dlp_confidential_file.h"
 #include "chrome/browser/chromeos/policy/dlp/dlp_file_destination.h"
-#include "chrome/browser/chromeos/policy/dlp/dlp_files_controller.h"
 #include "chrome/browser/chromeos/policy/dlp/dlp_files_utils.h"
 #include "chrome/browser/notifications/notification_display_service.h"
 #include "chrome/browser/notifications/notification_display_service_factory.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/ash/system_web_apps/system_web_app_ui_utils.h"
 #include "chrome/browser/ui/browser_list.h"
 #include "chrome/browser/ui/browser_window.h"
+#include "chrome/common/chrome_features.h"
 #include "components/strings/grit/components_strings.h"
 #include "content/public/browser/browser_context.h"
 #include "third_party/abseil-cpp/absl/types/optional.h"
@@ -1048,7 +1048,7 @@ void FilesPolicyNotificationManager::ShowDlpBlockNotification(
   const std::string notification_id = GetNotificationId(notification_count_++);
   std::unique_ptr<message_center::Notification> notification;
 
-  if (DlpFilesController::kNewFilesPolicyUXEnabled) {
+  if (base::FeatureList::IsEnabled(features::kNewFilesPolicyUX)) {
     // The notification should stay visible until actioned upon.
     message_center::RichNotificationData optional_fields;
     optional_fields.never_timeout = true;
@@ -1123,7 +1123,7 @@ void FilesPolicyNotificationManager::ShowDlpWarningNotification(
     std::vector<base::FilePath> warning_files,
     const DlpFileDestination& destination,
     dlp::FileAction action) {
-  if (DlpFilesController::kNewFilesPolicyUXEnabled) {
+  if (base::FeatureList::IsEnabled(features::kNewFilesPolicyUX)) {
     const std::string& notification_id =
         GetNotificationId(notification_count_++);
 

chrome/browser/ash/policy/dlp/files_policy_notification_manager_browsertest.cc

@@ -33,7 +33,6 @@
 #include "chrome/browser/chromeos/policy/dlp/dialogs/policy_dialog_base.h"
 #include "chrome/browser/chromeos/policy/dlp/dlp_confidential_file.h"
 #include "chrome/browser/chromeos/policy/dlp/dlp_file_destination.h"
-#include "chrome/browser/chromeos/policy/dlp/dlp_files_controller.h"
 #include "chrome/browser/chromeos/policy/dlp/dlp_files_utils.h"
 #include "chrome/browser/chromeos/policy/dlp/dlp_policy_constants.h"
 #include "chrome/browser/chromeos/policy/dlp/dlp_rules_manager.h"
@@ -47,6 +46,7 @@
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/ash/system_web_apps/system_web_app_ui_utils.h"
 #include "chrome/browser/ui/browser.h"
+#include "chrome/common/chrome_features.h"
 #include "chrome/common/chrome_paths.h"
 #include "chrome/test/base/in_process_browser_test.h"
 #include "chrome/test/base/interactive_test_utils.h"
@@ -153,7 +153,9 @@ class TestNotificationPlatformBridgeDelegator
 
 class FilesPolicyNotificationManagerBrowserTest : public InProcessBrowserTest {
  public:
-  FilesPolicyNotificationManagerBrowserTest() = default;
+  FilesPolicyNotificationManagerBrowserTest() {
+    scoped_feature_list_.InitAndEnableFeature(features::kNewFilesPolicyUX);
+  }
   FilesPolicyNotificationManagerBrowserTest(
       const FilesPolicyNotificationManagerBrowserTest&) = delete;
   FilesPolicyNotificationManagerBrowserTest& operator=(
@@ -183,9 +185,6 @@ class FilesPolicyNotificationManagerBrowserTest : public InProcessBrowserTest {
     fpnm_ = FilesPolicyNotificationManagerFactory::GetForBrowserContext(
         browser()->profile());
     ASSERT_TRUE(fpnm_);
-
-    DlpFilesController::SetNewFilesPolicyUXEnabledForTesting(
-        /*is_enabled=*/true);
   }
 
  protected:
@@ -197,6 +196,7 @@ class FilesPolicyNotificationManagerBrowserTest : public InProcessBrowserTest {
                                    ash::SystemWebAppType::FILE_MANAGER);
   }
 
+  base::test::ScopedFeatureList scoped_feature_list_;
   raw_ptr<NotificationDisplayServiceImpl, ExperimentalAsh> display_service_;
   raw_ptr<TestNotificationPlatformBridgeDelegator, ExperimentalAsh> bridge_;
   std::unique_ptr<MockFilesPolicyDialogFactory> factory_;

chrome/browser/ash/policy/dlp/files_policy_notification_manager_unittest.cc

@@ -22,9 +22,9 @@
 #include "chrome/browser/ash/policy/dlp/test/files_policy_notification_manager_test_utils.h"
 #include "chrome/browser/chromeos/policy/dlp/dialogs/policy_dialog_base.h"
 #include "chrome/browser/chromeos/policy/dlp/dlp_confidential_file.h"
-#include "chrome/browser/chromeos/policy/dlp/dlp_files_controller.h"
 #include "chrome/browser/chromeos/policy/dlp/dlp_files_utils.h"
 #include "chrome/browser/notifications/notification_display_service_tester.h"
+#include "chrome/common/chrome_features.h"
 #include "chrome/test/base/testing_browser_process.h"
 #include "chrome/test/base/testing_profile_manager.h"
 #include "chromeos/ash/components/disks/disk_mount_manager.h"
@@ -134,8 +134,9 @@ class FilesPolicyNotificationManagerTest : public testing::Test {
   FilesPolicyNotificationManagerTest& operator=(
       const FilesPolicyNotificationManagerTest&) = delete;
   ~FilesPolicyNotificationManagerTest() override = default;
-
   void SetUp() override {
+    scoped_feature_list_.InitAndEnableFeature(features::kNewFilesPolicyUX);
+
     ASSERT_TRUE(profile_manager_.SetUp());
     profile_ = profile_manager_.CreateTestingProfile("test-user");
     file_manager::VolumeManagerFactory::GetInstance()->SetTestingFactory(
@@ -179,6 +180,7 @@ class FilesPolicyNotificationManagerTest : public testing::Test {
   }
 
  protected:
+  base::test::ScopedFeatureList scoped_feature_list_;
   std::unique_ptr<FilesPolicyNotificationManager> fpnm_;
   scoped_refptr<storage::FileSystemContext> file_system_context_;
   raw_ptr<file_manager::io_task::IOTaskController, DanglingUntriaged>
@@ -970,11 +972,7 @@ INSTANTIATE_TEST_SUITE_P(
 class FPNMShowBlockTest
     : public FilesPolicyNotificationManagerTest,
       public ::testing::WithParamInterface<std::tuple<dlp::FileAction, int>> {
-  void SetUp() override {
-    FilesPolicyNotificationManagerTest::SetUp();
-    DlpFilesController::SetNewFilesPolicyUXEnabledForTesting(
-        /*is_enabled=*/true);
-  }
+  void SetUp() override { FilesPolicyNotificationManagerTest::SetUp(); }
 };
 
 TEST_P(FPNMShowBlockTest, ShowDlpBlockNotification_Single) {
@@ -1050,8 +1048,6 @@ class FPNMShowWarningTest
       public ::testing::WithParamInterface<dlp::FileAction> {
   void SetUp() override {
     FilesPolicyNotificationManagerTest::SetUp();
-    DlpFilesController::SetNewFilesPolicyUXEnabledForTesting(
-        /*is_enabled=*/true);
   }
 };
 

chrome/browser/chromeos/policy/dlp/dlp_files_controller.cc

@@ -131,13 +131,6 @@ DlpFilesController::DlpFilesController(const DlpRulesManager& rules_manager)
 
 DlpFilesController::~DlpFilesController() = default;
 
-bool DlpFilesController::kNewFilesPolicyUXEnabled = false;
-
-// static
-void DlpFilesController::SetNewFilesPolicyUXEnabledForTesting(bool is_enabled) {
-  kNewFilesPolicyUXEnabled = is_enabled;
-}
-
 void DlpFilesController::RequestCopyAccess(
     const storage::FileSystemURL& source_file,
     const storage::FileSystemURL& destination,

chrome/browser/chromeos/policy/dlp/dlp_files_controller.h

@@ -66,10 +66,6 @@ class DlpFilesController {
 
   virtual ~DlpFilesController();
 
-  static bool kNewFilesPolicyUXEnabled;
-
-  static void SetNewFilesPolicyUXEnabledForTesting(bool is_enabled);
-
  protected:
   explicit DlpFilesController(const DlpRulesManager& rules_manager);