Skip to content

Commit

Permalink
[CloudOpenMetrics] Log TaskResult when user exits cloud upload\open flow
Browse files Browse the repository at this point in the history
When the user tries to open a file with Google Docs or MS365, they will
be taken to the setup/fixup flow if extra setup is required. Previously,
if the user cancelled at the setup/fixup flow or instead chose a local
file task (not Google Docs or MS365) as the default task, no TaskResult
was logged, leading to inconsistent metrics [1].

Expand OfficeTaskResult with kCancelledAtSetup and kLocalFileTask to
use in these cases. Update ~CloudOpenMetrics() to check for these
consistencies.

[1] https://bugs.chromium.org/p/chromium/issues/detail?id=1496488

Bug: chromium:1496488 b/308078642
Change-Id: Idba990559a898190c5d43ce115e27812fbbac13b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4983114
Reviewed-by: Peter Marshall <petermarshall@chromium.org>
Commit-Queue: Cassy Chun-Crogan <cassycc@google.com>
Cr-Commit-Position: refs/heads/main@{#1216746}
  • Loading branch information
cassycc authored and Chromium LUCI CQ committed Oct 30, 2023
1 parent 5bf0924 commit 27e10aa
Show file tree
Hide file tree
Showing 5 changed files with 73 additions and 10 deletions.
21 changes: 13 additions & 8 deletions chrome/browser/ui/webui/ash/cloud_upload/cloud_open_metrics.cc
Original file line number Diff line number Diff line change
Expand Up @@ -86,16 +86,21 @@ CloudOpenMetrics::~CloudOpenMetrics() {
ExpectLogged(task_result_);
if (task_result_.logged()) {
if (task_result_.value == OfficeTaskResult::kFallbackQuickOffice ||
task_result_.value == OfficeTaskResult::kCancelledAtFallback) {
task_result_.value == OfficeTaskResult::kCancelledAtFallback ||
task_result_.value == OfficeTaskResult::kCancelledAtSetup ||
task_result_.value == OfficeTaskResult::kLocalFileTask) {
ExpectNotLogged(transfer_required_);
ExpectNotLogged(upload_result_);
if (google_drive) {
ExpectLoggedWith(drive_open_error_,
{OfficeDriveOpenErrors::kOffline,
OfficeDriveOpenErrors::kDriveFsInterface});
} else {
ExpectLoggedWith(one_drive_open_error_,
{OfficeOneDriveOpenErrors::kOffline});
if (task_result_.value == OfficeTaskResult::kFallbackQuickOffice ||
task_result_.value == OfficeTaskResult::kCancelledAtFallback) {
if (google_drive) {
ExpectLoggedWith(drive_open_error_,
{OfficeDriveOpenErrors::kOffline,
OfficeDriveOpenErrors::kDriveFsInterface});
} else {
ExpectLoggedWith(one_drive_open_error_,
{OfficeOneDriveOpenErrors::kOffline});
}
}
} else {
ExpectLogged(source_volume_);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1109,6 +1109,7 @@ void CloudOpenTask::OnDialogComplete(const std::string& user_response) {
cloud_provider_ = CloudProvider::kOneDrive;
InitAndShowDialog(mojom::DialogPage::kOneDriveSetup);
} else if (user_response == kUserActionCancel) {
cloud_open_metrics_->LogTaskResult(OfficeTaskResult::kCancelledAtSetup);
// Do nothing.
} else if (user_response == kUserActionCancelGoogleDrive) {
cloud_open_metrics_->LogTaskResult(
Expand All @@ -1117,6 +1118,7 @@ void CloudOpenTask::OnDialogComplete(const std::string& user_response) {
cloud_open_metrics_->LogTaskResult(
OfficeTaskResult::kCancelledAtConfirmation);
} else {
cloud_open_metrics_->LogTaskResult(OfficeTaskResult::kLocalFileTask);
LaunchLocalFileTask(user_response);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -252,6 +252,7 @@ class FileHandlerDialogBrowserTest : public InProcessBrowserTest {
std::vector<std::string> urls_;
std::vector<file_manager::file_tasks::TaskDescriptor> tasks_;
std::vector<storage::FileSystemURL> files_;
base::HistogramTester histogram_;

private:
base::test::ScopedFeatureList feature_list_;
Expand Down Expand Up @@ -378,10 +379,57 @@ IN_PROC_BROWSER_TEST_F(FileHandlerDialogBrowserTest, ModalParentProvided) {
FindSystemWebAppBrowser(profile(), SystemWebAppType::FILE_MANAGER));
}

// Test which launches a `CloudUploadDialog` which in turn creates a
// `FileHandlerPageElement`. Tests that the cancel button works and the correct
// TaskResult is logged.
IN_PROC_BROWSER_TEST_F(FileHandlerDialogBrowserTest, CancelFileHandlerDialog) {
// Watch for File Handler dialog URL chrome://cloud-upload.
content::TestNavigationObserver navigation_observer_dialog(
(GURL(chrome::kChromeUICloudUploadURL)));
navigation_observer_dialog.StartWatchingNewWebContents();

auto cloud_open_metrics =
std::make_unique<CloudOpenMetrics>(CloudProvider::kGoogleDrive);
auto cloud_open_metrics_weak_ptr = cloud_open_metrics->GetWeakPtr();

// Check that the Setup flow has never run and so the File Handler dialog will
// be launched when CloudOpenTask::Execute() is called.
ASSERT_FALSE(file_manager::file_tasks::HasExplicitDefaultFileHandler(
profile(), ".docx"));

// Launch File Handler dialog.
ASSERT_TRUE(CloudOpenTask::Execute(profile(), files_,
CloudProvider::kGoogleDrive, nullptr,
std::move(cloud_open_metrics)));

// Wait for File Handler dialog to open at chrome://cloud-upload.
navigation_observer_dialog.Wait();
ASSERT_TRUE(navigation_observer_dialog.last_navigation_succeeded());

// Get the web contents of the dialog to be able to query
// `FileHandlerPageElement`.
content::WebContents* web_contents = GetWebContentsFromCloudUploadDialog();

// Click the close button and wait for the dialog to close.
content::WebContentsDestroyedWatcher watcher(web_contents);
EXPECT_TRUE(content::ExecJs(web_contents,
"document.querySelector('file-handler-page')"
".$('.cancel-button').click()"));
watcher.Wait();

histogram_.ExpectUniqueSample(
ash::cloud_upload::kGoogleDriveTaskResultMetricName,
ash::cloud_upload::OfficeTaskResult::kCancelledAtSetup, 1);

// cloud_open_metrics should have been destroyed by the end of the test.
ASSERT_TRUE(cloud_open_metrics_weak_ptr.WasInvalidated());
}

// Test which launches a `CloudUploadDialog` which in turn creates a
// `FileHandlerPageElement`. Tests that the `FileHandlerPageElement` observes
// all of the fake file tasks and that a file task can be launched by clicking
// on its button before clicking the open button.
// on its button before clicking the open button. Tests that the correct
// TaskResult is logged
IN_PROC_BROWSER_TEST_F(FileHandlerDialogBrowserTest, OpenFileTaskFromDialog) {
// Install QuickOffice.
file_manager::test::AddDefaultComponentExtensionsOnMainThread(profile());
Expand Down Expand Up @@ -511,6 +559,10 @@ IN_PROC_BROWSER_TEST_F(FileHandlerDialogBrowserTest, OpenFileTaskFromDialog) {
ASSERT_FALSE(file_manager::file_tasks::GetDefaultTaskFromPrefs(
*profile()->GetPrefs(), kXlsxMimeType, kXlsxFileExtension));

histogram_.ExpectUniqueSample(
ash::cloud_upload::kGoogleDriveTaskResultMetricName,
ash::cloud_upload::OfficeTaskResult::kLocalFileTask, 1);

// cloud_open_metrics should have been destroyed by the end of the test.
ASSERT_TRUE(cloud_open_metrics_weak_ptr.WasInvalidated());
}
Expand Down
4 changes: 3 additions & 1 deletion chrome/browser/ui/webui/ash/cloud_upload/cloud_upload_util.h
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,9 @@ enum class OfficeTaskResult {
kFailedToOpen = 6,
kCopied = 7,
kCancelledAtFallback = 8,
kMaxValue = kCancelledAtFallback,
kCancelledAtSetup = 9,
kLocalFileTask = 10,
kMaxValue = kLocalFileTask,
};

// The result of the "Upload to cloud" workflow for Office files.
Expand Down
2 changes: 2 additions & 0 deletions tools/metrics/histograms/enums.xml
Original file line number Diff line number Diff line change
Expand Up @@ -80028,6 +80028,8 @@ Called by update_net_trust_anchors.py.-->
<int value="6" label="Failed to open"/>
<int value="7" label="Copied"/>
<int value="8" label="Cancelled at fallback"/>
<int value="9" label="Cancelled at setup"/>
<int value="10" label="Local file task"/>
</enum>

<enum name="OfflineCapableReason">
Expand Down

0 comments on commit 27e10aa

Please sign in to comment.