Better support OneDrive for ScreenCapture.
Improves the way how OneDrive location is retrieved, changes the string from obfuscated mount folder to `OneDrive`, adds metrics for OneDrive upload. Bug: b/323146997 Change-Id: I4e4167fa0a1a96e0447d9cff7efb4d51647d06fa Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5277131 Reviewed-by: Michele Fan <michelefan@chromium.org> Commit-Queue: Sergey Poromov <poromov@chromium.org> Reviewed-by: Ahmed Fakhry <afakhry@chromium.org> Reviewed-by: Sean Kau <skau@chromium.org> Cr-Commit-Position: refs/heads/main@{#1273935}
This commit is contained in:

committed by
Chromium LUCI CQ

parent
700aff62d4
commit
93163e5c1f
ash
ash_strings.grd
ash_strings_grd
capture_mode
capture_mode_controller.cccapture_mode_controller.hcapture_mode_metrics.hcapture_mode_settings_view.cccapture_mode_unittests.cctest_capture_mode_delegate.cctest_capture_mode_delegate.h
public
cpp
capture_mode
chrome/browser/ui/ash/capture_mode
tools/metrics/histograms/metadata/ash
@ -6332,6 +6332,9 @@ Here are some things you can try to get started.
|
||||
<message name="IDS_ASH_SCREEN_CAPTURE_SAVE_TO_LINUX_FILES" desc="The label of the menu item button for selecting the root of the Linux Files path to store the captured images and videos.">
|
||||
Linux files
|
||||
</message>
|
||||
<message name="IDS_ASH_SCREEN_CAPTURE_SAVE_TO_ONE_DRIVE" desc="The label of the menu item button for selecting the root of the OneDrive path to store the captured images and videos.">
|
||||
OneDrive
|
||||
</message>
|
||||
<message name="IDS_ASH_GAME_CAPTURE_START_RECORDING_BUTTON" desc="The label of the start recording button inside the game capture bar.">
|
||||
Start recording
|
||||
</message>
|
||||
|
@ -0,0 +1 @@
|
||||
2666ea75e6909640c56de8656970a8644eaae9c9
|
@ -909,6 +909,11 @@ bool CaptureModeController::IsLinuxFilesPath(const base::FilePath& path) const {
|
||||
return path == delegate_->GetLinuxFilesPath();
|
||||
}
|
||||
|
||||
bool CaptureModeController::IsRootOneDriveFilesPath(
|
||||
const base::FilePath& path) const {
|
||||
return path == delegate_->GetOneDriveMountPointPath();
|
||||
}
|
||||
|
||||
aura::Window* CaptureModeController::GetOnCaptureSurfaceWidgetParentWindow()
|
||||
const {
|
||||
// Trying to get camera preview's parent from `video_recording_watcher_` first
|
||||
@ -2266,7 +2271,15 @@ CaptureModeSaveToLocation CaptureModeController::GetSaveToOption(
|
||||
if (drive_root_path.IsParent(dir_path))
|
||||
return CaptureModeSaveToLocation::kDriveFolder;
|
||||
}
|
||||
// TODO(b/323146997): Add cases for OneDrive.
|
||||
base::FilePath one_drive_mount_path = delegate_->GetOneDriveMountPointPath();
|
||||
if (!one_drive_mount_path.empty()) {
|
||||
if (dir_path == one_drive_mount_path) {
|
||||
return CaptureModeSaveToLocation::kOneDrive;
|
||||
}
|
||||
if (one_drive_mount_path.IsParent(dir_path)) {
|
||||
return CaptureModeSaveToLocation::kOneDriveFolder;
|
||||
}
|
||||
}
|
||||
return CaptureModeSaveToLocation::kCustomizedFolder;
|
||||
}
|
||||
|
||||
|
@ -293,6 +293,10 @@ class ASH_EXPORT CaptureModeController
|
||||
// otherwise.
|
||||
bool IsLinuxFilesPath(const base::FilePath& path) const;
|
||||
|
||||
// Returns true if the given `path` is the root folder of OneDrive, false
|
||||
// otherwise.
|
||||
bool IsRootOneDriveFilesPath(const base::FilePath& path) const;
|
||||
|
||||
// Returns the current parent window for the on-capture-surface widgets such
|
||||
// as `CaptureModeCameraController::camera_preview_widget_` and
|
||||
// `CaptureModeDemoToolsController::key_combo_widget_`.
|
||||
|
@ -108,7 +108,9 @@ enum class CaptureModeSaveToLocation {
|
||||
kDrive,
|
||||
kDriveFolder,
|
||||
kCustomizedFolder,
|
||||
kMaxValue = kCustomizedFolder,
|
||||
kOneDrive,
|
||||
kOneDriveFolder,
|
||||
kMaxValue = kOneDriveFolder,
|
||||
};
|
||||
|
||||
// Enumeration of reasons for which the capture folder is switched to default
|
||||
|
@ -295,6 +295,9 @@ void CaptureModeSettingsView::OnCaptureFolderMayHaveChanged() {
|
||||
} else if (controller->IsLinuxFilesPath(custom_path)) {
|
||||
folder_name =
|
||||
l10n_util::GetStringUTF16(IDS_ASH_SCREEN_CAPTURE_SAVE_TO_LINUX_FILES);
|
||||
} else if (controller->IsRootOneDriveFilesPath(custom_path)) {
|
||||
folder_name =
|
||||
l10n_util::GetStringUTF16(IDS_ASH_SCREEN_CAPTURE_SAVE_TO_ONE_DRIVE);
|
||||
}
|
||||
|
||||
save_to_menu_group_->AddOrUpdateExistingOption(
|
||||
|
@ -33,6 +33,7 @@
|
||||
#include "ash/capture_mode/test_capture_mode_delegate.h"
|
||||
#include "ash/capture_mode/user_nudge_controller.h"
|
||||
#include "ash/capture_mode/video_recording_watcher.h"
|
||||
#include "ash/constants/ash_pref_names.h"
|
||||
#include "ash/display/cursor_window_controller.h"
|
||||
#include "ash/display/output_protection_delegate.h"
|
||||
#include "ash/display/screen_orientation_controller_test_api.h"
|
||||
@ -2714,6 +2715,13 @@ TEST_P(CaptureModeSaveFileTest, CaptureModeSaveToLocationMetric) {
|
||||
test_delegate->GetDriveFsMountPointPath(&mount_point_path);
|
||||
const auto root_drive_folder = mount_point_path.Append("root");
|
||||
const base::FilePath non_root_drive_folder = CreateFolderOnDriveFS("test");
|
||||
const base::FilePath onedrive_root =
|
||||
test_delegate->GetOneDriveMountPointPath();
|
||||
const base::FilePath onedrive_folder = onedrive_root.Append("test");
|
||||
{
|
||||
base::ScopedAllowBlockingForTesting allow_blocking;
|
||||
ASSERT_TRUE(base::CreateDirectory(onedrive_folder));
|
||||
}
|
||||
struct {
|
||||
base::FilePath set_save_file_folder;
|
||||
CaptureModeSaveToLocation save_location;
|
||||
@ -2722,6 +2730,8 @@ TEST_P(CaptureModeSaveFileTest, CaptureModeSaveToLocationMetric) {
|
||||
{custom_folder, CaptureModeSaveToLocation::kCustomizedFolder},
|
||||
{root_drive_folder, CaptureModeSaveToLocation::kDrive},
|
||||
{non_root_drive_folder, CaptureModeSaveToLocation::kDriveFolder},
|
||||
{onedrive_root, CaptureModeSaveToLocation::kOneDrive},
|
||||
{onedrive_folder, CaptureModeSaveToLocation::kOneDriveFolder},
|
||||
};
|
||||
for (auto test_case : kTestCases) {
|
||||
histogram_tester.ExpectBucketCount(histogram_name, test_case.save_location,
|
||||
@ -7295,6 +7305,74 @@ TEST_F(CaptureModeSettingsTest, KeyboardNavigationForAddingCustomFolderOption) {
|
||||
EXPECT_EQ(0u, session_test_api.GetCurrentFocusIndex());
|
||||
}
|
||||
|
||||
// Tests the folder selection settings when it's recommended by policy.
|
||||
TEST_F(CaptureModeSettingsTest, FolderRecommendedByPolicy) {
|
||||
auto* controller = StartImageRegionCapture();
|
||||
|
||||
// Set the pref to recommended values.
|
||||
const base::FilePath custom_folder(
|
||||
CreateCustomFolderInUserDownloadsPath("test"));
|
||||
auto* test_delegate =
|
||||
static_cast<TestCaptureModeDelegate*>(controller->delegate_for_testing());
|
||||
test_delegate->set_policy_capture_path(
|
||||
{custom_folder,
|
||||
CaptureModeDelegate::CapturePathEnforcement::kRecommended});
|
||||
|
||||
// Open settings.
|
||||
auto* event_generator = GetEventGenerator();
|
||||
ClickOnView(GetSettingsButton(), event_generator);
|
||||
std::unique_ptr<CaptureModeSettingsTestApi> test_api =
|
||||
std::make_unique<CaptureModeSettingsTestApi>();
|
||||
WaitForSettingsMenuToBeRefreshed();
|
||||
|
||||
// Custom folder is set, but Downloads option and select folder is enabled.
|
||||
EXPECT_FALSE(controller->IsCustomFolderManagedByPolicy());
|
||||
CaptureModeMenuGroup* save_to_menu_group = test_api->GetSaveToMenuGroup();
|
||||
EXPECT_FALSE(save_to_menu_group->IsManagedByPolicy());
|
||||
|
||||
EXPECT_TRUE(test_api->GetCustomFolderOptionIfAny()->GetEnabled());
|
||||
EXPECT_TRUE(save_to_menu_group->IsOptionChecked(kCustomFolder));
|
||||
|
||||
EXPECT_TRUE(test_api->GetDefaultDownloadsOption()->GetEnabled());
|
||||
EXPECT_FALSE(save_to_menu_group->IsOptionChecked(kDownloadsFolder));
|
||||
|
||||
EXPECT_TRUE(test_api->GetSelectFolderMenuItem()->GetEnabled());
|
||||
}
|
||||
|
||||
// Tests the folder selection settings when it's enforced by policy.
|
||||
TEST_F(CaptureModeSettingsTest, FolderSetByPolicy) {
|
||||
auto* controller = StartImageRegionCapture();
|
||||
|
||||
// Set the pref to managed values.
|
||||
const base::FilePath custom_folder(
|
||||
CreateCustomFolderInUserDownloadsPath("test"));
|
||||
auto* test_delegate =
|
||||
static_cast<TestCaptureModeDelegate*>(controller->delegate_for_testing());
|
||||
test_delegate->set_policy_capture_path(
|
||||
{custom_folder, CaptureModeDelegate::CapturePathEnforcement::kManaged});
|
||||
|
||||
// Open settings.
|
||||
auto* event_generator = GetEventGenerator();
|
||||
ClickOnView(GetSettingsButton(), event_generator);
|
||||
std::unique_ptr<CaptureModeSettingsTestApi> test_api =
|
||||
std::make_unique<CaptureModeSettingsTestApi>();
|
||||
WaitForSettingsMenuToBeRefreshed();
|
||||
|
||||
// Custom folder is set, but Downloads option and select folder are not
|
||||
// enabled.
|
||||
EXPECT_TRUE(controller->IsCustomFolderManagedByPolicy());
|
||||
CaptureModeMenuGroup* save_to_menu_group = test_api->GetSaveToMenuGroup();
|
||||
EXPECT_TRUE(save_to_menu_group->IsManagedByPolicy());
|
||||
|
||||
EXPECT_TRUE(test_api->GetCustomFolderOptionIfAny()->GetEnabled());
|
||||
EXPECT_TRUE(save_to_menu_group->IsOptionChecked(kCustomFolder));
|
||||
|
||||
EXPECT_FALSE(test_api->GetDefaultDownloadsOption()->GetEnabled());
|
||||
EXPECT_FALSE(save_to_menu_group->IsOptionChecked(kDownloadsFolder));
|
||||
|
||||
EXPECT_FALSE(test_api->GetSelectFolderMenuItem()->GetEnabled());
|
||||
}
|
||||
|
||||
// -----------------------------------------------------------------------------
|
||||
// CaptureModeHistogramTest:
|
||||
|
||||
|
@ -39,6 +39,8 @@ TestCaptureModeDelegate::TestCaptureModeDelegate()
|
||||
DCHECK(created_dir);
|
||||
created_dir = fake_linux_files_path_.CreateUniqueTempDir();
|
||||
DCHECK(created_dir);
|
||||
created_dir = fake_one_drive_mount_path_.CreateUniqueTempDir();
|
||||
DCHECK(created_dir);
|
||||
}
|
||||
|
||||
TestCaptureModeDelegate::~TestCaptureModeDelegate() = default;
|
||||
@ -177,9 +179,13 @@ base::FilePath TestCaptureModeDelegate::GetLinuxFilesPath() const {
|
||||
return fake_linux_files_path_.GetPath();
|
||||
}
|
||||
|
||||
base::FilePath TestCaptureModeDelegate::GetOneDriveMountPointPath() const {
|
||||
return fake_one_drive_mount_path_.GetPath();
|
||||
}
|
||||
|
||||
TestCaptureModeDelegate::PolicyCapturePath
|
||||
TestCaptureModeDelegate::GetPolicyCapturePath() const {
|
||||
return {base::FilePath(), CapturePathEnforcement::kNone};
|
||||
return policy_capture_path_;
|
||||
}
|
||||
|
||||
std::unique_ptr<RecordingOverlayView>
|
||||
|
@ -60,6 +60,9 @@ class TestCaptureModeDelegate : public CaptureModeDelegate {
|
||||
void set_fake_drive_fs_free_bytes(int64_t bytes) {
|
||||
fake_drive_fs_free_bytes_ = bytes;
|
||||
}
|
||||
void set_policy_capture_path(PolicyCapturePath policy_capture_path) {
|
||||
policy_capture_path_ = policy_capture_path;
|
||||
}
|
||||
|
||||
// Resets |is_allowed_by_policy_| and |is_allowed_by_dlp_| back to true.
|
||||
void ResetAllowancesToDefault();
|
||||
@ -118,6 +121,7 @@ class TestCaptureModeDelegate : public CaptureModeDelegate {
|
||||
bool GetDriveFsMountPointPath(base::FilePath* result) const override;
|
||||
base::FilePath GetAndroidFilesPath() const override;
|
||||
base::FilePath GetLinuxFilesPath() const override;
|
||||
base::FilePath GetOneDriveMountPointPath() const override;
|
||||
PolicyCapturePath GetPolicyCapturePath() const override;
|
||||
std::unique_ptr<RecordingOverlayView> CreateRecordingOverlayView()
|
||||
const override;
|
||||
@ -152,7 +156,10 @@ class TestCaptureModeDelegate : public CaptureModeDelegate {
|
||||
base::ScopedTempDir fake_drive_fs_mount_path_;
|
||||
base::ScopedTempDir fake_android_files_path_;
|
||||
base::ScopedTempDir fake_linux_files_path_;
|
||||
base::ScopedTempDir fake_one_drive_mount_path_;
|
||||
int64_t fake_drive_fs_free_bytes_ = std::numeric_limits<int64_t>::max();
|
||||
PolicyCapturePath policy_capture_path_ = {base::FilePath(),
|
||||
CapturePathEnforcement::kNone};
|
||||
};
|
||||
|
||||
} // namespace ash
|
||||
|
@ -168,6 +168,9 @@ class ASH_PUBLIC_EXPORT CaptureModeDelegate {
|
||||
// Returns the absolute path for the user's Linux Files.
|
||||
virtual base::FilePath GetLinuxFilesPath() const = 0;
|
||||
|
||||
// Gets the OneDrive mount point. Returns empty if OneDrive is not mounted.
|
||||
virtual base::FilePath GetOneDriveMountPointPath() const = 0;
|
||||
|
||||
// Returns the path to save files if policy set by admin.
|
||||
virtual PolicyCapturePath GetPolicyCapturePath() const = 0;
|
||||
|
||||
|
@ -32,6 +32,7 @@
|
||||
#include "chrome/browser/ui/ash/capture_mode/recording_overlay_view_impl.h"
|
||||
#include "chrome/browser/ui/ash/screenshot_area.h"
|
||||
#include "chrome/browser/ui/ash/system_web_apps/system_web_app_ui_utils.h"
|
||||
#include "chrome/browser/ui/webui/ash/cloud_upload/cloud_upload_util.h"
|
||||
#include "chrome/browser/web_applications/web_app_id_constants.h"
|
||||
#include "chrome/common/pref_names.h"
|
||||
#include "chromeos/ash/components/login/login_state/login_state.h"
|
||||
@ -247,6 +248,12 @@ base::FilePath ChromeCaptureModeDelegate::GetLinuxFilesPath() const {
|
||||
ProfileManager::GetActiveUserProfile());
|
||||
}
|
||||
|
||||
base::FilePath ChromeCaptureModeDelegate::GetOneDriveMountPointPath() const {
|
||||
Profile* profile = ProfileManager::GetPrimaryUserProfile();
|
||||
return profile ? ash::cloud_upload::GetODFSFuseboxMount(profile)
|
||||
: base::FilePath();
|
||||
}
|
||||
|
||||
ChromeCaptureModeDelegate::PolicyCapturePath
|
||||
ChromeCaptureModeDelegate::GetPolicyCapturePath() const {
|
||||
if (auto* profile = ProfileManager::GetActiveUserProfile()) {
|
||||
|
@ -66,6 +66,7 @@ class ChromeCaptureModeDelegate : public ash::CaptureModeDelegate {
|
||||
bool GetDriveFsMountPointPath(base::FilePath* path) const override;
|
||||
base::FilePath GetAndroidFilesPath() const override;
|
||||
base::FilePath GetLinuxFilesPath() const override;
|
||||
base::FilePath GetOneDriveMountPointPath() const override;
|
||||
PolicyCapturePath GetPolicyCapturePath() const override;
|
||||
std::unique_ptr<ash::RecordingOverlayView> CreateRecordingOverlayView()
|
||||
const override;
|
||||
|
@ -457,6 +457,8 @@ ash/ambient/util/time_of_day_utils.h -->
|
||||
<int value="1" label="Root drive folder"/>
|
||||
<int value="2" label="Customized folder in drive"/>
|
||||
<int value="3" label="Local customized folder"/>
|
||||
<int value="4" label="Root OneDrive folder"/>
|
||||
<int value="5" label="Customized folder in OneDrive"/>
|
||||
</enum>
|
||||
|
||||
<enum name="CaptureModeSwitchToDefaultReason">
|
||||
|
Reference in New Issue
Block a user