Remove #file-notification-revamp flag and associated code
This flag was turned on by default in M126, this CL tidies up the old code paths for M127. Bug: b/340347289 Change-Id: Ib3dcdd7f1c1368d6b173a91a53a6bcf3cbbfd8f9 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5553530 Reviewed-by: Shakti Sahu <shaktisahu@chromium.org> Reviewed-by: Ahmed Fakhry <afakhry@chromium.org> Commit-Queue: Timothy Loh <timloh@chromium.org> Cr-Commit-Position: refs/heads/main@{#1320802}
This commit is contained in:

committed by
Chromium LUCI CQ

parent
70a8816d07
commit
204ff5cb9f
ash
chrome/browser
about_flags.cc
download
flag-metadata.jsonflag_descriptions.ccflag_descriptions.hui
ash
capture_mode
download_status
tools/metrics/histograms
@ -6258,9 +6258,6 @@ Here are some things you can try to get started.
|
||||
<message name="IDS_ASH_SCREEN_CAPTURE_GIF_RECORDING_TITLE" desc="The titile of the notification which shows after a screen recording is taken.">
|
||||
GIF recording taken
|
||||
</message>
|
||||
<message name="IDS_ASH_SCREEN_CAPTURE_MESSAGE" desc="The message shows in the screenshot or screen recording notification.">
|
||||
Show in folder
|
||||
</message>
|
||||
<message name="IDS_ASH_SCREEN_CAPTURE_LOW_STORAGE_SPACE_MESSAGE" desc="The message shows in the screen recording notification when recording ends due to low storage space.">
|
||||
Recording ended due to critically low storage space
|
||||
</message>
|
||||
|
@ -1 +0,0 @@
|
||||
ac998dc1c50f46552c283f30c1d784b214f3f799
|
@ -1672,14 +1672,9 @@ void CaptureModeController::ShowPreviewNotification(
|
||||
const CaptureModeBehavior* behavior) {
|
||||
const bool for_video = type == CaptureModeType::kVideo;
|
||||
const int title_id = GetNotificationTitleIdForFile(screen_capture_path);
|
||||
int message_id;
|
||||
if (for_video && low_disk_space_threshold_reached_) {
|
||||
message_id = IDS_ASH_SCREEN_CAPTURE_LOW_STORAGE_SPACE_MESSAGE;
|
||||
} else {
|
||||
message_id = base::FeatureList::IsEnabled(features::kFileNotificationRevamp)
|
||||
? kNoMessage
|
||||
: IDS_ASH_SCREEN_CAPTURE_MESSAGE;
|
||||
}
|
||||
const int message_id = for_video && low_disk_space_threshold_reached_
|
||||
? IDS_ASH_SCREEN_CAPTURE_LOW_STORAGE_SPACE_MESSAGE
|
||||
: kNoMessage;
|
||||
|
||||
message_center::RichNotificationData optional_fields;
|
||||
optional_fields.buttons = behavior->GetNotificationButtonsInfo(for_video);
|
||||
@ -1704,15 +1699,9 @@ void CaptureModeController::HandleNotificationClicked(
|
||||
const BehaviorType behavior_type,
|
||||
std::optional<int> button_index) {
|
||||
if (!button_index.has_value()) {
|
||||
if (base::FeatureList::IsEnabled(features::kFileNotificationRevamp)) {
|
||||
// Open the item with the default handler.
|
||||
delegate_->OpenScreenCaptureItem(screen_capture_path);
|
||||
RecordScreenshotNotificationQuickAction(CaptureQuickAction::kOpenDefault);
|
||||
} else {
|
||||
// Show the item in the folder.
|
||||
delegate_->ShowScreenCaptureItemInFolder(screen_capture_path);
|
||||
RecordScreenshotNotificationQuickAction(CaptureQuickAction::kFiles);
|
||||
}
|
||||
// Open the item with the default handler.
|
||||
delegate_->OpenScreenCaptureItem(screen_capture_path);
|
||||
RecordScreenshotNotificationQuickAction(CaptureQuickAction::kOpenDefault);
|
||||
} else {
|
||||
const int button_index_value = button_index.value();
|
||||
if (type == CaptureModeType::kVideo) {
|
||||
|
@ -92,9 +92,6 @@ base::FilePath TestCaptureModeDelegate::GetUserDefaultDownloadsFolder() const {
|
||||
return fake_downloads_dir_.GetPath();
|
||||
}
|
||||
|
||||
void TestCaptureModeDelegate::ShowScreenCaptureItemInFolder(
|
||||
const base::FilePath& file_path) {}
|
||||
|
||||
void TestCaptureModeDelegate::OpenScreenCaptureItem(
|
||||
const base::FilePath& file_path) {}
|
||||
|
||||
|
@ -90,7 +90,6 @@ class TestCaptureModeDelegate : public CaptureModeDelegate {
|
||||
|
||||
// CaptureModeDelegate:
|
||||
base::FilePath GetUserDefaultDownloadsFolder() const override;
|
||||
void ShowScreenCaptureItemInFolder(const base::FilePath& file_path) override;
|
||||
void OpenScreenCaptureItem(const base::FilePath& file_path) override;
|
||||
void OpenScreenshotInImageEditor(const base::FilePath& file_path) override;
|
||||
bool Uses24HourFormat() const override;
|
||||
|
@ -1116,11 +1116,6 @@ BASE_FEATURE(kFederatedLauncherQueryAnalyticsVersion2Task,
|
||||
"FederatedLauncherQueryAnalyticsVersion2Task",
|
||||
base::FEATURE_DISABLED_BY_DEFAULT);
|
||||
|
||||
// Enable the new notifications for downloaded files and screen captures.
|
||||
BASE_FEATURE(kFileNotificationRevamp,
|
||||
"kFileNotificationRevamp",
|
||||
base::FEATURE_ENABLED_BY_DEFAULT);
|
||||
|
||||
// Enables the files transfer conflict dialog in Files app.
|
||||
BASE_FEATURE(kFilesConflictDialog,
|
||||
"FilesConflictDialog",
|
||||
|
@ -352,7 +352,6 @@ COMPONENT_EXPORT(ASH_CONSTANTS)
|
||||
BASE_DECLARE_FEATURE(kFederatedLauncherQueryAnalyticsTask);
|
||||
COMPONENT_EXPORT(ASH_CONSTANTS)
|
||||
BASE_DECLARE_FEATURE(kFederatedLauncherQueryAnalyticsVersion2Task);
|
||||
COMPONENT_EXPORT(ASH_CONSTANTS) BASE_DECLARE_FEATURE(kFileNotificationRevamp);
|
||||
COMPONENT_EXPORT(ASH_CONSTANTS) BASE_DECLARE_FEATURE(kFilesConflictDialog);
|
||||
COMPONENT_EXPORT(ASH_CONSTANTS) BASE_DECLARE_FEATURE(kFilesLocalImageSearch);
|
||||
COMPONENT_EXPORT(ASH_CONSTANTS) BASE_DECLARE_FEATURE(kFilesMaterializedViews);
|
||||
|
@ -80,10 +80,6 @@ class ASH_PUBLIC_EXPORT CaptureModeDelegate {
|
||||
// user. This function can only be called if the user is logged in.
|
||||
virtual base::FilePath GetUserDefaultDownloadsFolder() const = 0;
|
||||
|
||||
// Shows the screenshot or screen recording item in the screen capture folder.
|
||||
virtual void ShowScreenCaptureItemInFolder(
|
||||
const base::FilePath& file_path) = 0;
|
||||
|
||||
// Opens the screenshot or screen recording item with the default handler.
|
||||
virtual void OpenScreenCaptureItem(const base::FilePath& file_path) = 0;
|
||||
|
||||
|
@ -6026,9 +6026,6 @@ const FeatureEntry kFeatureEntries[] = {
|
||||
flag_descriptions::kDisableCameraFrameRotationAtSourceName,
|
||||
flag_descriptions::kDisableCameraFrameRotationAtSourceDescription, kOsCrOS,
|
||||
FEATURE_VALUE_TYPE(media::features::kDisableCameraFrameRotationAtSource)},
|
||||
{"file-notification-revamp", flag_descriptions::kFileNotificationRevampName,
|
||||
flag_descriptions::kFileNotificationRevampDescription, kOsCrOS,
|
||||
FEATURE_VALUE_TYPE(ash::features::kFileNotificationRevamp)},
|
||||
{"file-transfer-enterprise-connector",
|
||||
flag_descriptions::kFileTransferEnterpriseConnectorName,
|
||||
flag_descriptions::kFileTransferEnterpriseConnectorDescription, kOsCrOS,
|
||||
|
@ -64,7 +64,6 @@
|
||||
#include "ui/color/color_id.h"
|
||||
|
||||
#if BUILDFLAG(IS_CHROMEOS_ASH)
|
||||
#include "ash/constants/ash_features.h"
|
||||
#include "chrome/browser/ui/ash/system_web_apps/system_web_app_ui_utils.h"
|
||||
#endif
|
||||
|
||||
@ -724,10 +723,6 @@ void DownloadItemModel::OpenUsingPlatformHandler() {
|
||||
#if BUILDFLAG(IS_CHROMEOS_ASH)
|
||||
std::optional<DownloadCommands::Command>
|
||||
DownloadItemModel::MaybeGetMediaAppAction() const {
|
||||
if (!base::FeatureList::IsEnabled(ash::features::kFileNotificationRevamp)) {
|
||||
return std::nullopt;
|
||||
}
|
||||
|
||||
std::string mime_type = GetMimeType();
|
||||
|
||||
if (mime_type == "application/pdf") {
|
||||
|
@ -16,7 +16,6 @@
|
||||
#include "base/memory/raw_ptr.h"
|
||||
#include "base/run_loop.h"
|
||||
#include "base/test/metrics/histogram_tester.h"
|
||||
#include "base/test/scoped_feature_list.h"
|
||||
#include "base/test/test_simple_task_runner.h"
|
||||
#include "base/uuid.h"
|
||||
#include "build/chromeos_buildflags.h"
|
||||
@ -44,7 +43,6 @@
|
||||
#include "testing/gtest/include/gtest/gtest.h"
|
||||
|
||||
#if BUILDFLAG(IS_CHROMEOS_ASH)
|
||||
#include "ash/constants/ash_features.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"
|
||||
@ -188,7 +186,6 @@ class DownloadItemNotificationTest : public testing::Test {
|
||||
}
|
||||
#endif
|
||||
|
||||
base::test::ScopedFeatureList scoped_feature_list_;
|
||||
content::BrowserTaskEnvironment task_environment_;
|
||||
|
||||
std::unique_ptr<TestingProfileManager> profile_manager_;
|
||||
@ -414,15 +411,9 @@ TEST_F(DownloadItemNotificationTest, DeepScanning) {
|
||||
download_item_notification_->Click(std::nullopt, std::nullopt);
|
||||
}
|
||||
|
||||
// Test that EDIT_WITH_MEDIA_APP is added for pdf file if
|
||||
// kFileNotificationRevamp feature is enabled on CHROMEOS_ASH. It should not
|
||||
// be added for other build configs.
|
||||
// Test that EDIT_WITH_MEDIA_APP is added for pdf file on CHROMEOS_ASH.
|
||||
// It should not be added for other build configs.
|
||||
TEST_F(DownloadItemNotificationTest, NotificationActionsForPdf) {
|
||||
#if BUILDFLAG(IS_CHROMEOS_ASH)
|
||||
scoped_feature_list_.InitAndEnableFeature(
|
||||
ash::features::kFileNotificationRevamp);
|
||||
#endif
|
||||
|
||||
ON_CALL(*download_item_, GetState)
|
||||
.WillByDefault(Return(download::DownloadItem::COMPLETE));
|
||||
ON_CALL(*download_item_, IsDone).WillByDefault(Return(true));
|
||||
@ -440,15 +431,9 @@ TEST_F(DownloadItemNotificationTest, NotificationActionsForPdf) {
|
||||
#endif
|
||||
}
|
||||
|
||||
// Test that OPEN_WITH_MEDIA_APP is added for audio file if
|
||||
// kFileNotificationRevamp feature is enabled on CHROMEOS_ASH. It should not
|
||||
// be added for other build configs.
|
||||
// Test that OPEN_WITH_MEDIA_APP is added for audio file on CHROMEOS_ASH.
|
||||
// It should not be added for other build configs.
|
||||
TEST_F(DownloadItemNotificationTest, NotificationActionsForAudio) {
|
||||
#if BUILDFLAG(IS_CHROMEOS_ASH)
|
||||
scoped_feature_list_.InitAndEnableFeature(
|
||||
ash::features::kFileNotificationRevamp);
|
||||
#endif
|
||||
|
||||
ON_CALL(*download_item_, GetState)
|
||||
.WillByDefault(Return(download::DownloadItem::COMPLETE));
|
||||
ON_CALL(*download_item_, IsDone).WillByDefault(Return(true));
|
||||
|
@ -4593,11 +4593,6 @@
|
||||
"owners": [ "dmurph@chromium.org", "mgiuca@chromium.org", "cmp@chromium.org" ],
|
||||
"expiry_milestone": 110
|
||||
},
|
||||
{
|
||||
"name": "file-notification-revamp",
|
||||
"owners": [ "timloh@chromium.org", "backlight-swe@google.com" ],
|
||||
"expiry_milestone": 127
|
||||
},
|
||||
{
|
||||
"name": "file-system-access-locking-scheme",
|
||||
"owners": [ "memmott@chromium.org", "chrome-owp-storage@google.com" ],
|
||||
|
@ -6744,10 +6744,6 @@ const char kExposeOutOfProcessVideoDecodingToLacrosDescription[] =
|
||||
"Accept media.stable.mojom.StableVideoDecoderFactory connection requests "
|
||||
"from LaCrOS and host said factories in utility processes.";
|
||||
|
||||
const char kFileNotificationRevampName[] = "File notification revamp";
|
||||
const char kFileNotificationRevampDescription[] =
|
||||
"Enable the new notifications for downloaded files and screen captures.";
|
||||
|
||||
const char kFileTransferEnterpriseConnectorName[] =
|
||||
"Enable Files Transfer Enterprise Connector.";
|
||||
const char kFileTransferEnterpriseConnectorDescription[] =
|
||||
|
@ -3908,9 +3908,6 @@ extern const char kExperimentalAccessibilitySwitchAccessTextDescription[];
|
||||
extern const char kExposeOutOfProcessVideoDecodingToLacrosName[];
|
||||
extern const char kExposeOutOfProcessVideoDecodingToLacrosDescription[];
|
||||
|
||||
extern const char kFileNotificationRevampName[];
|
||||
extern const char kFileNotificationRevampDescription[];
|
||||
|
||||
extern const char kFileTransferEnterpriseConnectorName[];
|
||||
extern const char kFileTransferEnterpriseConnectorDescription[];
|
||||
|
||||
|
@ -120,12 +120,6 @@ base::FilePath ChromeCaptureModeDelegate::GetUserDefaultDownloadsFolder()
|
||||
return download_prefs->GetDefaultDownloadDirectoryForProfile();
|
||||
}
|
||||
|
||||
void ChromeCaptureModeDelegate::ShowScreenCaptureItemInFolder(
|
||||
const base::FilePath& file_path) {
|
||||
platform_util::ShowItemInFolder(ProfileManager::GetActiveUserProfile(),
|
||||
file_path);
|
||||
}
|
||||
|
||||
void ChromeCaptureModeDelegate::OpenScreenCaptureItem(
|
||||
const base::FilePath& file_path) {
|
||||
Profile* profile = ProfileManager::GetActiveUserProfile();
|
||||
|
@ -37,7 +37,6 @@ class ChromeCaptureModeDelegate : public ash::CaptureModeDelegate {
|
||||
|
||||
// ash::CaptureModeDelegate:
|
||||
base::FilePath GetUserDefaultDownloadsFolder() const override;
|
||||
void ShowScreenCaptureItemInFolder(const base::FilePath& file_path) override;
|
||||
void OpenScreenCaptureItem(const base::FilePath& file_path) override;
|
||||
void OpenScreenshotInImageEditor(const base::FilePath& file_path) override;
|
||||
bool Uses24HourFormat() const override;
|
||||
|
@ -280,31 +280,29 @@ DisplayMetadata DisplayManager::CalculateDisplayMetadata(
|
||||
CommandType::kOpenFile, full_path),
|
||||
/*icon=*/nullptr, /*text_id=*/-1, CommandType::kOpenFile);
|
||||
|
||||
if (base::FeatureList::IsEnabled(features::kFileNotificationRevamp)) {
|
||||
std::optional<std::pair<CommandType, /*text_id=*/int>>
|
||||
media_app_command_metadata;
|
||||
std::optional<std::pair<CommandType, /*text_id=*/int>>
|
||||
media_app_command_metadata;
|
||||
|
||||
if (mime_type == "application/pdf") {
|
||||
media_app_command_metadata =
|
||||
std::make_pair(CommandType::kEditWithMediaApp,
|
||||
IDS_DOWNLOAD_NOTIFICATION_LABEL_OPEN_AND_EDIT);
|
||||
} else if (base::StartsWith(mime_type, "audio/",
|
||||
base::CompareCase::SENSITIVE) ||
|
||||
base::StartsWith(mime_type, "video/",
|
||||
base::CompareCase::SENSITIVE)) {
|
||||
media_app_command_metadata =
|
||||
std::make_pair(CommandType::kOpenWithMediaApp,
|
||||
IDS_DOWNLOAD_NOTIFICATION_LABEL_OPEN);
|
||||
}
|
||||
if (mime_type == "application/pdf") {
|
||||
media_app_command_metadata =
|
||||
std::make_pair(CommandType::kEditWithMediaApp,
|
||||
IDS_DOWNLOAD_NOTIFICATION_LABEL_OPEN_AND_EDIT);
|
||||
} else if (base::StartsWith(mime_type, "audio/",
|
||||
base::CompareCase::SENSITIVE) ||
|
||||
base::StartsWith(mime_type, "video/",
|
||||
base::CompareCase::SENSITIVE)) {
|
||||
media_app_command_metadata =
|
||||
std::make_pair(CommandType::kOpenWithMediaApp,
|
||||
IDS_DOWNLOAD_NOTIFICATION_LABEL_OPEN);
|
||||
}
|
||||
|
||||
if (media_app_command_metadata) {
|
||||
command_infos.emplace_back(
|
||||
base::BindRepeating(&DisplayManager::PerformCommand,
|
||||
weak_ptr_factory_.GetWeakPtr(),
|
||||
media_app_command_metadata->first, full_path),
|
||||
/*icon=*/nullptr, media_app_command_metadata->second,
|
||||
media_app_command_metadata->first);
|
||||
}
|
||||
if (media_app_command_metadata) {
|
||||
command_infos.emplace_back(
|
||||
base::BindRepeating(&DisplayManager::PerformCommand,
|
||||
weak_ptr_factory_.GetWeakPtr(),
|
||||
media_app_command_metadata->first, full_path),
|
||||
/*icon=*/nullptr, media_app_command_metadata->second,
|
||||
media_app_command_metadata->first);
|
||||
}
|
||||
|
||||
// NOTE: The `kShowInFolder` button does not have an icon.
|
||||
|
@ -15646,7 +15646,6 @@ from previous Chrome versions.
|
||||
<int value="-2101338272" label="EnablePciguardUi:disabled"/>
|
||||
<int value="-2101337189" label="AutofillOffNoServerData:disabled"/>
|
||||
<int value="-2101091929" label="MessagesForAndroidAdsBlocked:disabled"/>
|
||||
<int value="-2100915973" label="kFileNotificationRevamp:enabled"/>
|
||||
<int value="-2100134911" label="SystemColorChooser:disabled"/>
|
||||
<int value="-2099608999" label="CrOSEnforceSystemAecNs:enabled"/>
|
||||
<int value="-2099486626" label="DownloadLater:enabled"/>
|
||||
@ -17626,7 +17625,6 @@ from previous Chrome versions.
|
||||
<int value="-1265627803" label="WebAppEnableManifestId:enabled"/>
|
||||
<int value="-1264634670" label="HideTabOnTabSwitcher:disabled"/>
|
||||
<int value="-1262730949" label="EnableDspHotword:enabled"/>
|
||||
<int value="-1262726737" label="kFileNotificationRevamp:disabled"/>
|
||||
<int value="-1262303946" label="SubresourceRedirectPreviews:disabled"/>
|
||||
<int value="-1262302650" label="StorageBuckets:disabled"/>
|
||||
<int value="-1262152606" label="disable-lock-screen-apps"/>
|
||||
|
Reference in New Issue
Block a user