Skip to content

Commit

Permalink
capture_mode_settings: Detect the selection of Play files
Browse files Browse the repository at this point in the history
The path of the user's Android Play files internally can
have a very different user-unfirendly name. This CL fixes
this issue by detecting when this folder gets selected
in the capture mode settings, and name it explicitly
"Play files" similarly to how it's done in the Files
app.

Fixed: 1277570
Test: Manually
Change-Id: I08874700148c6d89a959c94e2affaff3f18b43fe
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3328223
Reviewed-by: Min Chen <minch@chromium.org>
Commit-Queue: Ahmed Fakhry <afakhry@chromium.org>
Cr-Commit-Position: refs/heads/main@{#950182}
  • Loading branch information
Ahmed Fakhry authored and Chromium LUCI CQ committed Dec 9, 2021
1 parent e7f5646 commit 885604e
Show file tree
Hide file tree
Showing 10 changed files with 45 additions and 10 deletions.
3 changes: 3 additions & 0 deletions ash/ash_strings.grd
Expand Up @@ -4017,6 +4017,9 @@ Here are some things you can try to get started.
<message name="IDS_ASH_SCREEN_CAPTURE_SAVE_TO_GOOGLE_DRIVE" desc="The label of the menu item button for selecting the root of Google Drive to store the captured images and videos.">
Google Drive
</message>
<message name="IDS_ASH_SCREEN_CAPTURE_SAVE_TO_ANDROID_FILES" desc="The label of the menu item button for selecting the root of the Android Play files path to store the captured images and videos.">
Play files
</message>

<!-- Switch Between TABLET/LAPTOP MODE-->
<message name="IDS_ASH_SWITCH_TO_TABLET_MODE" desc="Alert of switching to tablet mode.">
Expand Down
@@ -0,0 +1 @@
ca7bb51eacb12e35779993046905821405a596fa
16 changes: 11 additions & 5 deletions ash/capture_mode/capture_mode_advanced_settings_view.cc
Expand Up @@ -130,11 +130,17 @@ void CaptureModeAdvancedSettingsView::OnCaptureFolderMayHaveChanged() {
return;
}

const std::u16string folder_name =
controller->IsRootDriveFsPath(custom_path)
? l10n_util::GetStringUTF16(
IDS_ASH_SCREEN_CAPTURE_SAVE_TO_GOOGLE_DRIVE)
: custom_path.BaseName().AsUTF16Unsafe();
std::u16string folder_name = custom_path.BaseName().AsUTF16Unsafe();
// We explicitly name the folders of Google Drive and Play files, since those
// folders internally may have user-unfriendly names.
if (controller->IsRootDriveFsPath(custom_path)) {
folder_name =
l10n_util::GetStringUTF16(IDS_ASH_SCREEN_CAPTURE_SAVE_TO_GOOGLE_DRIVE);
} else if (controller->IsAndroidFilesPath(custom_path)) {
folder_name =
l10n_util::GetStringUTF16(IDS_ASH_SCREEN_CAPTURE_SAVE_TO_ANDROID_FILES);
}

save_to_menu_group_->AddOrUpdateExistingOption(folder_name, kCustomFolder);

controller->CheckFolderAvailability(
Expand Down
5 changes: 5 additions & 0 deletions ash/capture_mode/capture_mode_controller.cc
Expand Up @@ -710,6 +710,11 @@ bool CaptureModeController::IsRootDriveFsPath(
return false;
}

bool CaptureModeController::IsAndroidFilesPath(
const base::FilePath& path) const {
return path == delegate_->GetAndroidFilesPath();
}

void CaptureModeController::OnRecordingEnded(
recording::mojom::RecordingStatus status,
const gfx::ImageSkia& thumbnail) {
Expand Down
4 changes: 4 additions & 0 deletions ash/capture_mode/capture_mode_controller.h
Expand Up @@ -198,6 +198,10 @@ class ASH_EXPORT CaptureModeController
// otherwise.
bool IsRootDriveFsPath(const base::FilePath& path) const;

// Returns true if the given `path` is the same as the Android Play files
// path, false otherwise.
bool IsAndroidFilesPath(const base::FilePath& path) const;

// recording::mojom::RecordingServiceClient:
void OnRecordingEnded(recording::mojom::RecordingStatus status,
const gfx::ImageSkia& thumbnail) override;
Expand Down
15 changes: 10 additions & 5 deletions ash/capture_mode/test_capture_mode_delegate.cc
Expand Up @@ -29,12 +29,13 @@ class TestRecordingOverlayView : public RecordingOverlayView {

TestCaptureModeDelegate::TestCaptureModeDelegate() {
base::ScopedAllowBlockingForTesting allow_blocking;
const bool created_downloads_dir =
bool created_dir =
base::CreateNewTempDirectory(/*prefix=*/"", &fake_downloads_dir_);
DCHECK(created_downloads_dir);
const bool created_root_drive_dir =
fake_drive_fs_mount_path_.CreateUniqueTempDir();
DCHECK(created_root_drive_dir);
DCHECK(created_dir);
created_dir = fake_drive_fs_mount_path_.CreateUniqueTempDir();
DCHECK(created_dir);
created_dir = fake_android_files_path_.CreateUniqueTempDir();
DCHECK(created_dir);
}

TestCaptureModeDelegate::~TestCaptureModeDelegate() = default;
Expand Down Expand Up @@ -158,6 +159,10 @@ bool TestCaptureModeDelegate::GetDriveFsMountPointPath(
return true;
}

base::FilePath TestCaptureModeDelegate::GetAndroidFilesPath() const {
return fake_android_files_path_.GetPath();
}

std::unique_ptr<RecordingOverlayView>
TestCaptureModeDelegate::CreateRecordingOverlayView() const {
return std::make_unique<TestRecordingOverlayView>();
Expand Down
2 changes: 2 additions & 0 deletions ash/capture_mode/test_capture_mode_delegate.h
Expand Up @@ -93,6 +93,7 @@ class TestCaptureModeDelegate : public CaptureModeDelegate {
void OnSessionStateChanged(bool started) override;
void OnServiceRemoteReset() override;
bool GetDriveFsMountPointPath(base::FilePath* result) const override;
base::FilePath GetAndroidFilesPath() const override;
std::unique_ptr<RecordingOverlayView> CreateRecordingOverlayView()
const override;

Expand All @@ -105,6 +106,7 @@ class TestCaptureModeDelegate : public CaptureModeDelegate {
bool is_allowed_by_policy_ = true;
bool should_save_after_dlp_check_ = true;
base::ScopedTempDir fake_drive_fs_mount_path_;
base::ScopedTempDir fake_android_files_path_;
};

} // namespace ash
Expand Down
3 changes: 3 additions & 0 deletions ash/public/cpp/capture_mode/capture_mode_delegate.h
Expand Up @@ -141,6 +141,9 @@ class ASH_PUBLIC_EXPORT CaptureModeDelegate {
// ash_unittests to reduce the duplication.
virtual bool GetDriveFsMountPointPath(base::FilePath* path) const = 0;

// Returns the absolute path for the user's Android Play files.
virtual base::FilePath GetAndroidFilesPath() const = 0;

// Creates and returns the view that will be used as the contents view of the
// overlay widget, which is added as a child of the recorded surface to host
// contents rendered in a web view that are meant to be part of the recording
Expand Down
Expand Up @@ -15,6 +15,7 @@
#include "chrome/browser/apps/app_service/app_service_proxy_factory.h"
#include "chrome/browser/apps/app_service/launch_utils.h"
#include "chrome/browser/ash/drive/drive_integration_service.h"
#include "chrome/browser/ash/file_manager/path_util.h"
#include "chrome/browser/ash/policy/dlp/dlp_content_manager.h"
#include "chrome/browser/browser_process.h"
#include "chrome/browser/download/download_prefs.h"
Expand Down Expand Up @@ -216,6 +217,10 @@ bool ChromeCaptureModeDelegate::GetDriveFsMountPointPath(
return true;
}

base::FilePath ChromeCaptureModeDelegate::GetAndroidFilesPath() const {
return file_manager::util::GetAndroidFilesPath();
}

std::unique_ptr<ash::RecordingOverlayView>
ChromeCaptureModeDelegate::CreateRecordingOverlayView() const {
return std::make_unique<RecordingOverlayViewImpl>(
Expand Down
Expand Up @@ -60,6 +60,7 @@ class ChromeCaptureModeDelegate : public ash::CaptureModeDelegate {
void OnSessionStateChanged(bool started) override;
void OnServiceRemoteReset() override;
bool GetDriveFsMountPointPath(base::FilePath* path) const override;
base::FilePath GetAndroidFilesPath() const override;
std::unique_ptr<ash::RecordingOverlayView> CreateRecordingOverlayView()
const override;

Expand Down

0 comments on commit 885604e

Please sign in to comment.