Commit c928ccfb authored by Eugene But's avatar Eugene But Committed by Commit Bot

[ios] Add User Actions to Download Manager UI

These use actions will show up in Breadcrumbs logs, which are the steps
to reproduce attached to the crash logs.

Bug: 1046223
Change-Id: I49d6a19fb22eebce2d939a7378179148913cc85f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2138166Reviewed-by: default avatarSylvain Defresne <sdefresne@chromium.org>
Reviewed-by: default avatarBrian White <bcwhite@chromium.org>
Commit-Queue: Brian White <bcwhite@chromium.org>
Auto-Submit: Eugene But <eugenebut@chromium.org>
Cr-Commit-Position: refs/heads/master@{#757130}
parent f47be67c
...@@ -271,6 +271,14 @@ class UnopenedDownloadsTracker : public web::DownloadTaskObserver, ...@@ -271,6 +271,14 @@ class UnopenedDownloadsTracker : public web::DownloadTaskObserver,
confirmTitle:IDS_OK confirmTitle:IDS_OK
cancelTitle:IDS_CANCEL cancelTitle:IDS_CANCEL
completionHandler:^(BOOL confirmed) { completionHandler:^(BOOL confirmed) {
if (confirmed) {
base::RecordAction(base::UserMetricsAction(
"IOSDownloadConfirmReplace"));
} else {
base::RecordAction(base::UserMetricsAction(
"IOSDownloadDoNotReplace"));
}
base::UmaHistogramBoolean("Download.IOSDownloadReplaced", base::UmaHistogramBoolean("Download.IOSDownloadReplaced",
confirmed); confirmed);
handler(confirmed ? kNewDownloadPolicyReplace handler(confirmed ? kNewDownloadPolicyReplace
...@@ -313,10 +321,12 @@ class UnopenedDownloadsTracker : public web::DownloadTaskObserver, ...@@ -313,10 +321,12 @@ class UnopenedDownloadsTracker : public web::DownloadTaskObserver,
base::UmaHistogramEnumeration("Download.IOSDownloadFileResult", base::UmaHistogramEnumeration("Download.IOSDownloadFileResult",
DownloadFileResult::NotStarted, DownloadFileResult::NotStarted,
DownloadFileResult::Count); DownloadFileResult::Count);
base::RecordAction(base::UserMetricsAction("IOSDownloadClose"));
[self cancelDownload]; [self cancelDownload];
return; return;
} }
base::RecordAction(
base::UserMetricsAction("IOSDownloadTryCloseWhenInProgress"));
__weak DownloadManagerCoordinator* weakSelf = self; __weak DownloadManagerCoordinator* weakSelf = self;
int title = IDS_IOS_DOWNLOAD_MANAGER_CANCEL_CONFIRMATION; int title = IDS_IOS_DOWNLOAD_MANAGER_CANCEL_CONFIRMATION;
[self runConfirmationDialogWithTitle:title [self runConfirmationDialogWithTitle:title
...@@ -330,13 +340,19 @@ class UnopenedDownloadsTracker : public web::DownloadTaskObserver, ...@@ -330,13 +340,19 @@ class UnopenedDownloadsTracker : public web::DownloadTaskObserver,
DownloadFileResult::Cancelled, DownloadFileResult::Cancelled,
DownloadFileResult::Count); DownloadFileResult::Count);
base::RecordAction(base::UserMetricsAction(
"IOSDownloadConfirmClose"));
[weakSelf cancelDownload]; [weakSelf cancelDownload];
} else {
base::RecordAction(
base::UserMetricsAction("IOSDownloadDoNotClose"));
} }
}]; }];
} }
- (void)installDriveForDownloadManagerViewController: - (void)installDriveForDownloadManagerViewController:
(DownloadManagerViewController*)controller { (DownloadManagerViewController*)controller {
base::RecordAction(base::UserMetricsAction("IOSDownloadInstallGoogleDrive"));
[self presentStoreKitForGoogleDriveApp]; [self presentStoreKitForGoogleDriveApp];
} }
...@@ -345,6 +361,7 @@ class UnopenedDownloadsTracker : public web::DownloadTaskObserver, ...@@ -345,6 +361,7 @@ class UnopenedDownloadsTracker : public web::DownloadTaskObserver,
if (_downloadTask->GetErrorCode() != net::OK) { if (_downloadTask->GetErrorCode() != net::OK) {
base::RecordAction(base::UserMetricsAction("MobileDownloadRetryDownload")); base::RecordAction(base::UserMetricsAction("MobileDownloadRetryDownload"));
} else { } else {
base::RecordAction(base::UserMetricsAction("IOSDownloadStartDownload"));
_unopenedDownloads.Add(_downloadTask); _unopenedDownloads.Add(_downloadTask);
} }
_mediator.StartDowloading(); _mediator.StartDowloading();
...@@ -352,6 +369,7 @@ class UnopenedDownloadsTracker : public web::DownloadTaskObserver, ...@@ -352,6 +369,7 @@ class UnopenedDownloadsTracker : public web::DownloadTaskObserver,
- (void)presentOpenInForDownloadManagerViewController: - (void)presentOpenInForDownloadManagerViewController:
(DownloadManagerViewController*)controller { (DownloadManagerViewController*)controller {
base::RecordAction(base::UserMetricsAction("IOSDownloadOpenIn"));
base::FilePath path = _mediator.GetDownloadPath(); base::FilePath path = _mediator.GetDownloadPath();
NSURL* URL = [NSURL fileURLWithPath:base::SysUTF8ToNSString(path.value())]; NSURL* URL = [NSURL fileURLWithPath:base::SysUTF8ToNSString(path.value())];
......
...@@ -340,6 +340,7 @@ TEST_F(DownloadManagerCoordinatorTest, Close) { ...@@ -340,6 +340,7 @@ TEST_F(DownloadManagerCoordinatorTest, Close) {
DownloadManagerViewController* viewController = DownloadManagerViewController* viewController =
base_view_controller_.childViewControllers.firstObject; base_view_controller_.childViewControllers.firstObject;
ASSERT_EQ([DownloadManagerViewController class], [viewController class]); ASSERT_EQ([DownloadManagerViewController class], [viewController class]);
ASSERT_EQ(0, user_action_tester_.GetActionCount("IOSDownloadClose"));
@autoreleasepool { @autoreleasepool {
// This call will retain coordinator, which should outlive thread bundle. // This call will retain coordinator, which should outlive thread bundle.
[viewController.delegate [viewController.delegate
...@@ -357,6 +358,7 @@ TEST_F(DownloadManagerCoordinatorTest, Close) { ...@@ -357,6 +358,7 @@ TEST_F(DownloadManagerCoordinatorTest, Close) {
1); 1);
histogram_tester_.ExpectTotalCount("Download.IOSDownloadFileUIGoogleDrive", histogram_tester_.ExpectTotalCount("Download.IOSDownloadFileUIGoogleDrive",
0); 0);
EXPECT_EQ(1, user_action_tester_.GetActionCount("IOSDownloadClose"));
} }
// Tests presenting Install Google Drive dialog. Coordinator presents StoreKit // Tests presenting Install Google Drive dialog. Coordinator presents StoreKit
...@@ -377,6 +379,8 @@ TEST_F(DownloadManagerCoordinatorTest, InstallDrive) { ...@@ -377,6 +379,8 @@ TEST_F(DownloadManagerCoordinatorTest, InstallDrive) {
// button changes it's alpha. // button changes it's alpha.
ASSERT_EQ(1.0f, viewController.installDriveButton.superview.alpha); ASSERT_EQ(1.0f, viewController.installDriveButton.superview.alpha);
ASSERT_EQ(
0, user_action_tester_.GetActionCount("IOSDownloadInstallGoogleDrive"));
@autoreleasepool { @autoreleasepool {
// This call will retain coordinator, which should outlive thread bundle. // This call will retain coordinator, which should outlive thread bundle.
[viewController.delegate [viewController.delegate
...@@ -393,8 +397,10 @@ TEST_F(DownloadManagerCoordinatorTest, InstallDrive) { ...@@ -393,8 +397,10 @@ TEST_F(DownloadManagerCoordinatorTest, InstallDrive) {
return viewController.installDriveButton.superview.alpha == 0.0f; return viewController.installDriveButton.superview.alpha == 0.0f;
})); }));
// Simulate Google Drive app installation and verify that expected user action // Simulate Google Drive app installation and verify that expected histograms
// has been recorded. // have been recorded.
EXPECT_EQ(
1, user_action_tester_.GetActionCount("IOSDownloadInstallGoogleDrive"));
histogram_tester_.ExpectTotalCount("Download.IOSDownloadFileUIGoogleDrive", histogram_tester_.ExpectTotalCount("Download.IOSDownloadFileUIGoogleDrive",
0); 0);
// SKStoreProductViewController uses UIApplication, so it's not possible to // SKStoreProductViewController uses UIApplication, so it's not possible to
...@@ -438,6 +444,8 @@ TEST_F(DownloadManagerCoordinatorTest, OpenIn) { ...@@ -438,6 +444,8 @@ TEST_F(DownloadManagerCoordinatorTest, OpenIn) {
EXPECT_EQ(open_in_controller.excludedActivityTypes.count, 2.0); EXPECT_EQ(open_in_controller.excludedActivityTypes.count, 2.0);
}); });
ASSERT_EQ(0, user_action_tester_.GetActionCount("IOSDownloadOpenIn"));
// Present Open In... menu. // Present Open In... menu.
@autoreleasepool { @autoreleasepool {
// These calls will retain coordinator, which should outlive thread bundle. // These calls will retain coordinator, which should outlive thread bundle.
...@@ -470,6 +478,7 @@ TEST_F(DownloadManagerCoordinatorTest, OpenIn) { ...@@ -470,6 +478,7 @@ TEST_F(DownloadManagerCoordinatorTest, OpenIn) {
1); 1);
histogram_tester_.ExpectTotalCount("Download.IOSDownloadFileUIGoogleDrive", histogram_tester_.ExpectTotalCount("Download.IOSDownloadFileUIGoogleDrive",
1); 1);
EXPECT_EQ(1, user_action_tester_.GetActionCount("IOSDownloadOpenIn"));
} }
// Tests destroying download task for in progress download. // Tests destroying download task for in progress download.
...@@ -570,6 +579,8 @@ TEST_F(DownloadManagerCoordinatorTest, CloseInProgressDownload) { ...@@ -570,6 +579,8 @@ TEST_F(DownloadManagerCoordinatorTest, CloseInProgressDownload) {
DownloadManagerViewController* viewController = DownloadManagerViewController* viewController =
base_view_controller_.childViewControllers.firstObject; base_view_controller_.childViewControllers.firstObject;
ASSERT_EQ([DownloadManagerViewController class], [viewController class]); ASSERT_EQ([DownloadManagerViewController class], [viewController class]);
ASSERT_EQ(0, user_action_tester_.GetActionCount(
"IOSDownloadTryCloseWhenInProgress"));
@autoreleasepool { @autoreleasepool {
// This call will retain coordinator, which should outlive thread bundle. // This call will retain coordinator, which should outlive thread bundle.
[viewController.delegate [viewController.delegate
...@@ -595,6 +606,10 @@ TEST_F(DownloadManagerCoordinatorTest, CloseInProgressDownload) { ...@@ -595,6 +606,10 @@ TEST_F(DownloadManagerCoordinatorTest, CloseInProgressDownload) {
WaitUntilConditionOrTimeout(base::test::ios::kWaitForUIElementTimeout, ^{ WaitUntilConditionOrTimeout(base::test::ios::kWaitForUIElementTimeout, ^{
return !base_view_controller_.presentedViewController; return !base_view_controller_.presentedViewController;
})); }));
EXPECT_EQ(1, user_action_tester_.GetActionCount(
"IOSDownloadTryCloseWhenInProgress"));
EXPECT_EQ(0, user_action_tester_.GetActionCount("IOSDownloadConfirmClose"));
EXPECT_EQ(0, user_action_tester_.GetActionCount("IOSDownloadDoNotClose"));
} }
// Tests downloadManagerTabHelper:decidePolicyForDownload:completionHandler:. // Tests downloadManagerTabHelper:decidePolicyForDownload:completionHandler:.
...@@ -660,8 +675,9 @@ TEST_F(DownloadManagerCoordinatorTest, StartDownload) { ...@@ -660,8 +675,9 @@ TEST_F(DownloadManagerCoordinatorTest, StartDownload) {
EXPECT_TRUE(download_dir.IsParent(file)); EXPECT_TRUE(download_dir.IsParent(file));
histogram_tester_.ExpectTotalCount("Download.IOSDownloadFileInBackground", 0); histogram_tester_.ExpectTotalCount("Download.IOSDownloadFileInBackground", 0);
ASSERT_EQ(0, EXPECT_EQ(0,
user_action_tester_.GetActionCount("MobileDownloadRetryDownload")); user_action_tester_.GetActionCount("MobileDownloadRetryDownload"));
EXPECT_EQ(1, user_action_tester_.GetActionCount("IOSDownloadStartDownload"));
} }
// Tests retrying the download. Verifies that kDownloadManagerRetryDownload UMA // Tests retrying the download. Verifies that kDownloadManagerRetryDownload UMA
...@@ -677,6 +693,7 @@ TEST_F(DownloadManagerCoordinatorTest, RetryingDownload) { ...@@ -677,6 +693,7 @@ TEST_F(DownloadManagerCoordinatorTest, RetryingDownload) {
DownloadManagerViewController* viewController = DownloadManagerViewController* viewController =
base_view_controller_.childViewControllers.firstObject; base_view_controller_.childViewControllers.firstObject;
ASSERT_EQ([DownloadManagerViewController class], [viewController class]); ASSERT_EQ([DownloadManagerViewController class], [viewController class]);
ASSERT_EQ(0, user_action_tester_.GetActionCount("IOSDownloadStartDownload"));
@autoreleasepool { @autoreleasepool {
// This call will retain coordinator, which should outlive thread bundle. // This call will retain coordinator, which should outlive thread bundle.
[viewController.delegate [viewController.delegate
...@@ -684,6 +701,7 @@ TEST_F(DownloadManagerCoordinatorTest, RetryingDownload) { ...@@ -684,6 +701,7 @@ TEST_F(DownloadManagerCoordinatorTest, RetryingDownload) {
} }
task.SetErrorCode(net::ERR_INTERNET_DISCONNECTED); task.SetErrorCode(net::ERR_INTERNET_DISCONNECTED);
task.SetDone(true); task.SetDone(true);
ASSERT_EQ(1, user_action_tester_.GetActionCount("IOSDownloadStartDownload"));
@autoreleasepool { @autoreleasepool {
// This call will retain coordinator, which should outlive thread bundle. // This call will retain coordinator, which should outlive thread bundle.
...@@ -708,7 +726,7 @@ TEST_F(DownloadManagerCoordinatorTest, RetryingDownload) { ...@@ -708,7 +726,7 @@ TEST_F(DownloadManagerCoordinatorTest, RetryingDownload) {
static_cast<base::HistogramBase::Sample>( static_cast<base::HistogramBase::Sample>(
DownloadFileInBackground::FailedWithoutBackgrounding), DownloadFileInBackground::FailedWithoutBackgrounding),
1); 1);
ASSERT_EQ(1, EXPECT_EQ(1,
user_action_tester_.GetActionCount("MobileDownloadRetryDownload")); user_action_tester_.GetActionCount("MobileDownloadRetryDownload"));
} }
......
...@@ -8465,6 +8465,83 @@ should be able to be added at any place in this file. ...@@ -8465,6 +8465,83 @@ should be able to be added at any place in this file.
</description> </description>
</action> </action>
<action name="IOSDownloadClose">
<owner>eugenebut@chromium.org</owner>
<owner>sdefresne@chromium.org</owner>
<description>
User closed Download Manager UI by tapping x button when download was not
in-progress (either not started or failed).
</description>
</action>
<action name="IOSDownloadConfirmClose">
<owner>eugenebut@chromium.org</owner>
<owner>sdefresne@chromium.org</owner>
<description>
User confirmed download closure by tapping &quot;Stop&quot; on &quot;Stop
Download?&quot; dialog.
</description>
</action>
<action name="IOSDownloadConfirmReplace">
<owner>eugenebut@chromium.org</owner>
<owner>sdefresne@chromium.org</owner>
<description>
User confirmed download replacement by tapping &quot;OK&quot; on &quot;Start
New Download&quot; dialog.
</description>
</action>
<action name="IOSDownloadDoNotClose">
<owner>eugenebut@chromium.org</owner>
<owner>sdefresne@chromium.org</owner>
<description>
User rejected download closure by tapping &quot;Continue&quot; on &quot;Stop
Download?&quot; dialog.
</description>
</action>
<action name="IOSDownloadDoNotReplace">
<owner>eugenebut@chromium.org</owner>
<owner>sdefresne@chromium.org</owner>
<description>
User rejected download replacement by tapping &quot;Cancel&quot; on
&quot;Start New Download?&quot; dialog.
</description>
</action>
<action name="IOSDownloadInstallGoogleDrive">
<owner>eugenebut@chromium.org</owner>
<owner>sdefresne@chromium.org</owner>
<description>User tapped on Install Google Drive promo.</description>
</action>
<action name="IOSDownloadOpenIn">
<owner>eugenebut@chromium.org</owner>
<owner>sdefresne@chromium.org</owner>
<description>
User tapped on Open In... button after in Download Manager after download
succeeded.
</description>
</action>
<action name="IOSDownloadStartDownload">
<owner>eugenebut@chromium.org</owner>
<owner>sdefresne@chromium.org</owner>
<description>User started the download in Download Manager.</description>
</action>
<action name="IOSDownloadTryCloseWhenInProgress">
<owner>eugenebut@chromium.org</owner>
<owner>sdefresne@chromium.org</owner>
<description>
User tapped x button when download was in-progress. The Download Manager did
not close, but presented the confirmation dialog. See
IOSDownloadConfirmClose and IOSDownloadDoNotClose actions for recording use
choice.
</description>
</action>
<action name="IOSGoogleServicesSettingsCloseWithSwipe"> <action name="IOSGoogleServicesSettingsCloseWithSwipe">
<owner>msarda@chromium.org</owner> <owner>msarda@chromium.org</owner>
<owner>jlebel@chromium.org</owner> <owner>jlebel@chromium.org</owner>
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment