Commit 59deb360 authored by Eugene But's avatar Eugene But Committed by Commit Bot

Report CanceledAfterAppQuit UMA histogram.

This value is logged when the user quits the app during in-progress
downloads.

Bug: 981327
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: If19c9e16045ab0af2af1a77a941df843cc6fdeb8
Reviewed-on: https://chromium-review.googlesource.com/987200
Commit-Queue: Eugene But <eugenebut@chromium.org>
Reviewed-by: default avatarSylvain Defresne <sdefresne@chromium.org>
Cr-Commit-Position: refs/heads/master@{#547986}
parent 08df566d
......@@ -1021,11 +1021,6 @@ applicationCommandEndpoint:(id<ApplicationCommands>)applicationCommandEndpoint {
_snackbarCoordinator.dispatcher = _dispatcher;
[_snackbarCoordinator start];
_downloadManagerCoordinator =
[[DownloadManagerCoordinator alloc] initWithBaseViewController:self];
_downloadManagerCoordinator.presenter =
[[VerticalAnimationContainer alloc] init];
_storeKitCoordinator =
[[StoreKitCoordinator alloc] initWithBaseViewController:self];
......@@ -1560,6 +1555,8 @@ applicationCommandEndpoint:(id<ApplicationCommands>)applicationCommandEndpoint {
self.primaryToolbarCoordinator = nil;
[self.secondaryToolbarCoordinator stop];
self.secondaryToolbarCoordinator = nil;
[_downloadManagerCoordinator stop];
_downloadManagerCoordinator = nil;
self.toolbarInterface = nil;
self.tabStripView = nil;
_infoBarContainer = nil;
......@@ -2237,6 +2234,12 @@ applicationCommandEndpoint:(id<ApplicationCommands>)applicationCommandEndpoint {
[self.primaryToolbarCoordinator QRScannerResultLoader];
_qrScannerCoordinator.presentationProvider = self;
_downloadManagerCoordinator =
[[DownloadManagerCoordinator alloc] initWithBaseViewController:self];
_downloadManagerCoordinator.webStateList = [_model webStateList];
_downloadManagerCoordinator.presenter =
[[VerticalAnimationContainer alloc] init];
if (IsUIRefreshPhase1Enabled()) {
self.popupMenuCoordinator = [[PopupMenuCoordinator alloc]
initWithBaseViewController:self
......
......@@ -49,6 +49,7 @@ source_set("download") {
"//ios/chrome/browser/ui/presenters",
"//ios/chrome/browser/ui/util",
"//ios/chrome/browser/web:web_internal",
"//ios/chrome/browser/web_state_list:web_state_list",
"//ios/public/provider/chrome/browser",
"//ios/public/provider/chrome/browser/images",
"//ios/third_party/material_components_ios",
......@@ -83,6 +84,8 @@ source_set("unit_tests") {
"//ios/chrome/browser/store_kit",
"//ios/chrome/browser/ui:ui_util",
"//ios/chrome/browser/web:test_support",
"//ios/chrome/browser/web_state_list:test_support",
"//ios/chrome/browser/web_state_list:web_state_list",
"//ios/chrome/test:test_support",
"//ios/chrome/test/app:test_support",
"//ios/chrome/test/fakes",
......
......@@ -11,6 +11,7 @@
namespace web {
class DownloadTask;
} // namespace web
class WebStateList;
@protocol ContainedPresenter;
......@@ -28,6 +29,9 @@ class DownloadTask;
// stop method is called.
@property(nonatomic) web::DownloadTask* downloadTask;
// Needed to observe web state closing. Set to null when stop method is called.
@property(nonatomic) WebStateList* webStateList;
// Underlying UIViewController presented by this coordinator.
@property(nonatomic, readonly) UIViewController* viewController;
......
......@@ -23,6 +23,8 @@
#import "ios/chrome/browser/ui/download/download_manager_view_controller.h"
#import "ios/chrome/browser/ui/presenters/contained_presenter.h"
#import "ios/chrome/browser/ui/presenters/contained_presenter_delegate.h"
#import "ios/chrome/browser/web_state_list/web_state_list.h"
#import "ios/chrome/browser/web_state_list/web_state_list_observer.h"
#include "ios/chrome/grit/ios_strings.h"
#import "ios/web/public/download/download_task.h"
#include "net/base/net_errors.h"
......@@ -36,7 +38,8 @@
namespace {
// Tracks download tasks which were not opened by the user yet. Reports various
// metrics in DownloadTaskObserver callbacks.
class UnopenedDownloadsTracker : public web::DownloadTaskObserver {
class UnopenedDownloadsTracker : public web::DownloadTaskObserver,
public WebStateListObserver {
public:
// Starts tracking this download task.
void Add(web::DownloadTask* task) { task->AddObserver(this); }
......@@ -77,6 +80,15 @@ class UnopenedDownloadsTracker : public web::DownloadTaskObserver {
UMA_HISTOGRAM_ENUMERATION("Download.IOSDownloadFileResult",
DownloadFileResult::Other,
DownloadFileResult::Count);
if (did_close_web_state_without_user_action) {
// web state can be closed without user action only during the app
// shutdown.
UMA_HISTOGRAM_ENUMERATION(
"Download.IOSDownloadFileInBackground",
DownloadFileInBackground::CanceledAfterAppQuit,
DownloadFileInBackground::Count);
}
}
if (task->IsDone() && task->GetErrorCode() == net::OK) {
......@@ -86,6 +98,19 @@ class UnopenedDownloadsTracker : public web::DownloadTaskObserver {
DownloadedFileAction::Count);
}
}
// WebStateListObserver overrides:
void WillCloseWebStateAt(WebStateList* web_state_list,
web::WebState* web_state,
int index,
bool user_action) override {
if (!user_action) {
did_close_web_state_without_user_action = true;
}
}
private:
// True if a web state was closed without user action.
bool did_close_web_state_without_user_action = false;
};
} // namespace
......@@ -114,6 +139,7 @@ class UnopenedDownloadsTracker : public web::DownloadTaskObserver {
@synthesize presenter = _presenter;
@synthesize animatesPresentation = _animatesPresentation;
@synthesize downloadTask = _downloadTask;
@synthesize webStateList = _webStateList;
- (void)dealloc {
[[InstallationNotifier sharedInstance] unregisterForNotifications:self];
......@@ -147,6 +173,7 @@ class UnopenedDownloadsTracker : public web::DownloadTaskObserver {
completion:nil];
_confirmationDialog = nil;
_downloadTask = nullptr;
self.webStateList = nullptr;
[_storeKitCoordinator stop];
_storeKitCoordinator = nil;
......@@ -154,6 +181,19 @@ class UnopenedDownloadsTracker : public web::DownloadTaskObserver {
_installDriveAlertCoordinator = nil;
}
- (void)setWebStateList:(WebStateList*)webStateList {
if (_webStateList == webStateList)
return;
if (_webStateList)
_webStateList->RemoveObserver(&_unopenedDownloads);
_webStateList = webStateList;
if (_webStateList)
_webStateList->AddObserver(&_unopenedDownloads);
}
- (UIViewController*)viewController {
return _viewController;
}
......
......@@ -18,6 +18,9 @@
#import "ios/chrome/browser/download/download_manager_tab_helper.h"
#import "ios/chrome/browser/download/google_drive_app_util.h"
#import "ios/chrome/browser/ui/download/download_manager_view_controller.h"
#import "ios/chrome/browser/web_state_list/fake_web_state_list_delegate.h"
#import "ios/chrome/browser/web_state_list/web_state_list.h"
#import "ios/chrome/browser/web_state_list/web_state_opener.h"
#import "ios/chrome/test/fakes/fake_contained_presenter.h"
#import "ios/chrome/test/fakes/fake_document_interaction_controller.h"
#import "ios/chrome/test/scoped_key_window.h"
......@@ -457,11 +460,61 @@ TEST_F(DownloadManagerCoordinatorTest, DestroyInProgressDownload) {
task = nullptr;
histogram_tester_.ExpectTotalCount("Download.IOSDownloadedFileNetError", 0);
histogram_tester_.ExpectTotalCount("Download.IOSDownloadedFileAction", 0);
histogram_tester_.ExpectTotalCount("Download.IOSDownloadFileInBackground", 0);
histogram_tester_.ExpectUniqueSample(
"Download.IOSDownloadFileResult",
static_cast<base::HistogramBase::Sample>(DownloadFileResult::Other), 1);
}
// Tests quitting the app during in-progress download.
TEST_F(DownloadManagerCoordinatorTest, QuitDuringInProgressDownload) {
auto task = CreateTestTask();
coordinator_.downloadTask = task.get();
web::DownloadTask* task_ptr = task.get();
FakeWebStateListDelegate web_state_list_delegate;
WebStateList web_state_list(&web_state_list_delegate);
auto web_state = std::make_unique<web::TestWebState>();
web_state_list.InsertWebState(
0, std::move(web_state), WebStateList::INSERT_NO_FLAGS, WebStateOpener());
coordinator_.webStateList = &web_state_list;
[coordinator_ start];
EXPECT_EQ(1U, base_view_controller_.childViewControllers.count);
DownloadManagerViewController* viewController =
base_view_controller_.childViewControllers.firstObject;
ASSERT_EQ([DownloadManagerViewController class], [viewController class]);
// Start and the download.
@autoreleasepool {
// This call will retain coordinator, which should outlive thread bundle.
[viewController.delegate
downloadManagerViewControllerDidStartDownload:viewController];
}
// Starting download is async for model.
ASSERT_TRUE(WaitUntilConditionOrTimeout(testing::kWaitForDownloadTimeout, ^{
base::RunLoop().RunUntilIdle();
return task_ptr->GetState() == web::DownloadTask::State::kInProgress;
}));
// Web States are closed without user action only during app termination.
web_state_list.CloseAllWebStates(WebStateList::CLOSE_NO_FLAGS);
// Download task is destroyed before the download is complete.
task = nullptr;
histogram_tester_.ExpectTotalCount("Download.IOSDownloadedFileNetError", 0);
histogram_tester_.ExpectTotalCount("Download.IOSDownloadedFileAction", 0);
histogram_tester_.ExpectUniqueSample(
"Download.IOSDownloadFileInBackground",
static_cast<base::HistogramBase::Sample>(
DownloadFileInBackground::CanceledAfterAppQuit),
1);
histogram_tester_.ExpectUniqueSample(
"Download.IOSDownloadFileResult",
static_cast<base::HistogramBase::Sample>(DownloadFileResult::Other), 1);
coordinator_.webStateList = nullptr;
}
// Tests opening the download in Google Drive app.
TEST_F(DownloadManagerCoordinatorTest, OpenInDrive) {
web::FakeDownloadTask task(GURL(kTestUrl), kTestMimeType);
......@@ -721,6 +774,7 @@ TEST_F(DownloadManagerCoordinatorTest, StartDownload) {
ASSERT_TRUE(GetDownloadsDirectory(&download_dir));
EXPECT_TRUE(download_dir.IsParent(file));
histogram_tester_.ExpectTotalCount("Download.IOSDownloadFileInBackground", 0);
ASSERT_EQ(0,
user_action_tester_.GetActionCount("MobileDownloadRetryDownload"));
}
......
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