Commit 7becea0d authored by Eugene But's avatar Eugene But Committed by Commit Bot

[ios] Clean up comments in download_manager_coordinator_unittest.mm

This is a follow up to:
https://chromium-review.googlesource.com/c/chromium/src/+/2300834/1/ios/chrome/browser/ui/download/download_manager_coordinator_unittest.mm#708

CL updates comments which explain the reason for wrapping certain code
into @autorelease block.

Bug: None
Change-Id: Ie31e5701f81d6f3fdd8d5347e81441732c1afa75
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2303033
Auto-Submit: Eugene But <eugenebut@chromium.org>
Commit-Queue: Sylvain Defresne <sdefresne@chromium.org>
Reviewed-by: default avatarSylvain Defresne <sdefresne@chromium.org>
Cr-Commit-Position: refs/heads/master@{#789416}
parent 4983e185
...@@ -94,8 +94,9 @@ class DownloadManagerCoordinatorTest : public PlatformTest { ...@@ -94,8 +94,9 @@ class DownloadManagerCoordinatorTest : public PlatformTest {
~DownloadManagerCoordinatorTest() override { ~DownloadManagerCoordinatorTest() override {
// Stop to avoid holding a dangling pointer to destroyed task. // Stop to avoid holding a dangling pointer to destroyed task.
@autoreleasepool { @autoreleasepool {
// task_environment_ has to outlive the coordinator. Dismissing // Calling -stop will retain and autorelease coordinator_.
// coordinator retains are autoreleases it. // task_environment_ has to outlive the coordinator, so wrapping -stop
// call in @autorelease will ensure that coordinator_ is deallocated.
[coordinator_ stop]; [coordinator_ stop];
} }
...@@ -153,8 +154,9 @@ TEST_F(DownloadManagerCoordinatorTest, Stop) { ...@@ -153,8 +154,9 @@ TEST_F(DownloadManagerCoordinatorTest, Stop) {
coordinator_.downloadTask = &task; coordinator_.downloadTask = &task;
[coordinator_ start]; [coordinator_ start];
@autoreleasepool { @autoreleasepool {
// task_environment_ has to outlive the coordinator. Dismissing coordinator // Calling -stop will retain and autorelease coordinator_. task_environment_
// retains are autoreleases it. // has to outlive the coordinator, so wrapping -stop call in @autorelease
// will ensure that coordinator_ is deallocated.
[coordinator_ stop]; [coordinator_ stop];
} }
...@@ -182,7 +184,10 @@ TEST_F(DownloadManagerCoordinatorTest, DestructionDuringDownload) { ...@@ -182,7 +184,10 @@ TEST_F(DownloadManagerCoordinatorTest, DestructionDuringDownload) {
base::ThreadTaskRunnerHandle::Get(), path)); base::ThreadTaskRunnerHandle::Get(), path));
@autoreleasepool { @autoreleasepool {
// These calls will retain coordinator, which should outlive thread bundle. // Calling -downloadManagerViewControllerDidStartDownload will retain and
// autorelease coordinator_. task_environment_ has to outlive the
// coordinator, so wrapping -downloadManagerViewControllerDidStartDownload
// call in @autorelease will ensure that coordinator_ is deallocated.
[viewController.delegate [viewController.delegate
downloadManagerViewControllerDidStartDownload:viewController]; downloadManagerViewControllerDidStartDownload:viewController];
...@@ -298,8 +303,10 @@ TEST_F(DownloadManagerCoordinatorTest, DelegateHideDownload) { ...@@ -298,8 +303,10 @@ TEST_F(DownloadManagerCoordinatorTest, DelegateHideDownload) {
didCreateDownload:task.get() didCreateDownload:task.get()
webStateIsVisible:YES]; webStateIsVisible:YES];
@autoreleasepool { @autoreleasepool {
// task_environment_ has to outlive the coordinator. Dismissing coordinator // Calling -downloadManagerTabHelper:didHideDownload: will retain and
// retains are autoreleases it. // autorelease coordinator_. task_environment_ has to outlive the
// coordinator, so wrapping -downloadManagerTabHelper:didHideDownload:
// call in @autorelease will ensure that coordinator_ is deallocated.
[coordinator_ downloadManagerTabHelper:&tab_helper_ [coordinator_ downloadManagerTabHelper:&tab_helper_
didHideDownload:task.get()]; didHideDownload:task.get()];
} }
...@@ -346,7 +353,10 @@ TEST_F(DownloadManagerCoordinatorTest, Close) { ...@@ -346,7 +353,10 @@ TEST_F(DownloadManagerCoordinatorTest, Close) {
ASSERT_EQ([DownloadManagerViewController class], [viewController class]); ASSERT_EQ([DownloadManagerViewController class], [viewController class]);
ASSERT_EQ(0, user_action_tester_.GetActionCount("IOSDownloadClose")); ASSERT_EQ(0, user_action_tester_.GetActionCount("IOSDownloadClose"));
@autoreleasepool { @autoreleasepool {
// This call will retain coordinator, which should outlive thread bundle. // Calling -downloadManagerViewControllerDidClose: will retain and
// autorelease coordinator_. task_environment_ has to outlive the
// coordinator, so wrapping -downloadManagerViewControllerDidClose:
// call in @autorelease will ensure that coordinator_ is deallocated.
[viewController.delegate [viewController.delegate
downloadManagerViewControllerDidClose:viewController]; downloadManagerViewControllerDidClose:viewController];
} }
...@@ -386,7 +396,10 @@ TEST_F(DownloadManagerCoordinatorTest, InstallDrive) { ...@@ -386,7 +396,10 @@ TEST_F(DownloadManagerCoordinatorTest, InstallDrive) {
ASSERT_EQ( ASSERT_EQ(
0, user_action_tester_.GetActionCount("IOSDownloadInstallGoogleDrive")); 0, user_action_tester_.GetActionCount("IOSDownloadInstallGoogleDrive"));
@autoreleasepool { @autoreleasepool {
// This call will retain coordinator, which should outlive thread bundle. // Calling -installDriveForDownloadManagerViewController: will retain and
// autorelease coordinator_. task_environment_ has to outlive the
// coordinator, so wrapping -installDriveForDownloadManagerViewController:
// call in @autorelease will ensure that coordinator_ is deallocated.
[viewController.delegate [viewController.delegate
installDriveForDownloadManagerViewController:viewController]; installDriveForDownloadManagerViewController:viewController];
} }
...@@ -452,7 +465,11 @@ TEST_F(DownloadManagerCoordinatorTest, OpenIn) { ...@@ -452,7 +465,11 @@ TEST_F(DownloadManagerCoordinatorTest, OpenIn) {
// Present Open In... menu. // Present Open In... menu.
@autoreleasepool { @autoreleasepool {
// These calls will retain coordinator, which should outlive thread bundle. // Calling -installDriveForDownloadManagerViewController: and
// presentOpenInForDownloadManagerViewController will retain and
// autorelease coordinator_. task_environment_ has to outlive the
// coordinator, so wrapping calls in @autorelease will ensure that
// coordinator_ is deallocated.
[view_controller.delegate [view_controller.delegate
downloadManagerViewControllerDidStartDownload:view_controller]; downloadManagerViewControllerDidStartDownload:view_controller];
...@@ -499,7 +516,10 @@ TEST_F(DownloadManagerCoordinatorTest, DestroyInProgressDownload) { ...@@ -499,7 +516,10 @@ TEST_F(DownloadManagerCoordinatorTest, DestroyInProgressDownload) {
// Start and the download. // Start and the download.
@autoreleasepool { @autoreleasepool {
// This call will retain coordinator, which should outlive thread bundle. // Calling -downloadManagerViewControllerDidStartDownload: will retain and
// autorelease coordinator_. task_environment_ has to outlive the
// coordinator, so wrapping -downloadManagerViewControllerDidStartDownload:
// call in @autorelease will ensure that coordinator_ is deallocated.
[viewController.delegate [viewController.delegate
downloadManagerViewControllerDidStartDownload:viewController]; downloadManagerViewControllerDidStartDownload:viewController];
} }
...@@ -540,7 +560,10 @@ TEST_F(DownloadManagerCoordinatorTest, QuitDuringInProgressDownload) { ...@@ -540,7 +560,10 @@ TEST_F(DownloadManagerCoordinatorTest, QuitDuringInProgressDownload) {
// Start and the download. // Start and the download.
@autoreleasepool { @autoreleasepool {
// This call will retain coordinator, which should outlive thread bundle. // Calling -downloadManagerViewControllerDidStartDownload: will retain and
// autorelease coordinator_. task_environment_ has to outlive the
// coordinator, so wrapping -downloadManagerViewControllerDidStartDownload:
// call in @autorelease will ensure that coordinator_ is deallocated.
[viewController.delegate [viewController.delegate
downloadManagerViewControllerDidStartDownload:viewController]; downloadManagerViewControllerDidStartDownload:viewController];
} }
...@@ -591,7 +614,10 @@ TEST_F(DownloadManagerCoordinatorTest, CloseInProgressDownload) { ...@@ -591,7 +614,10 @@ TEST_F(DownloadManagerCoordinatorTest, CloseInProgressDownload) {
&web_state_, OverlayModality::kWebContentArea); &web_state_, OverlayModality::kWebContentArea);
ASSERT_EQ(0U, queue->size()); ASSERT_EQ(0U, queue->size());
@autoreleasepool { @autoreleasepool {
// This call will retain coordinator, which should outlive thread bundle. // Calling -downloadManagerViewControllerDidClose: will retain and
// autorelease coordinator_. task_environment_ has to outlive the
// coordinator, so wrapping -downloadManagerViewControllerDidClose:
// call in @autorelease will ensure that coordinator_ is deallocated.
[viewController.delegate [viewController.delegate
downloadManagerViewControllerDidClose:viewController]; downloadManagerViewControllerDidClose:viewController];
} }
...@@ -614,8 +640,9 @@ TEST_F(DownloadManagerCoordinatorTest, CloseInProgressDownload) { ...@@ -614,8 +640,9 @@ TEST_F(DownloadManagerCoordinatorTest, CloseInProgressDownload) {
// Stop to avoid holding a dangling pointer to destroyed task. // Stop to avoid holding a dangling pointer to destroyed task.
queue->CancelAllRequests(); queue->CancelAllRequests();
@autoreleasepool { @autoreleasepool {
// task_environment_ has to outlive the coordinator. Dismissing coordinator // Calling -stop will retain and autorelease coordinator_. task_environment_
// retains are autoreleases it. // has to outlive the coordinator, so wrapping -stop call in @autorelease
// will ensure that coordinator_ is deallocated.
[coordinator_ stop]; [coordinator_ stop];
} }
...@@ -661,8 +688,9 @@ TEST_F(DownloadManagerCoordinatorTest, DecidePolicyForDownload) { ...@@ -661,8 +688,9 @@ TEST_F(DownloadManagerCoordinatorTest, DecidePolicyForDownload) {
queue->CancelAllRequests(); queue->CancelAllRequests();
@autoreleasepool { @autoreleasepool {
// task_environment_ has to outlive the coordinator. Dismissing coordinator // Calling -stop will retain and autorelease coordinator_. task_environment_
// retains are autoreleases it. // has to outlive the coordinator, so wrapping -stop call in @autorelease
// will ensure that coordinator_ is deallocated.
[coordinator_ stop]; [coordinator_ stop];
} }
...@@ -704,8 +732,9 @@ TEST_F(DownloadManagerCoordinatorTest, ...@@ -704,8 +732,9 @@ TEST_F(DownloadManagerCoordinatorTest,
queue->CancelAllRequests(); queue->CancelAllRequests();
@autoreleasepool { @autoreleasepool {
// task_environment_ has to outlive the coordinator. Dismissing coordinator // Calling -stop will retain and autorelease coordinator_. task_environment_
// retains are autoreleases it. // has to outlive the coordinator, so wrapping -stop call in @autorelease
// will ensure that coordinator_ is deallocated.
[coordinator_ stop]; [coordinator_ stop];
} }
...@@ -725,7 +754,10 @@ TEST_F(DownloadManagerCoordinatorTest, StartDownload) { ...@@ -725,7 +754,10 @@ TEST_F(DownloadManagerCoordinatorTest, StartDownload) {
base_view_controller_.childViewControllers.firstObject; base_view_controller_.childViewControllers.firstObject;
ASSERT_EQ([DownloadManagerViewController class], [viewController class]); ASSERT_EQ([DownloadManagerViewController class], [viewController class]);
@autoreleasepool { @autoreleasepool {
// This call will retain coordinator, which should outlive thread bundle. // Calling -downloadManagerViewControllerDidStartDownload: will retain and
// autorelease coordinator_. task_environment_ has to outlive the
// coordinator, so wrapping -downloadManagerViewControllerDidStartDownload:
// call in @autorelease will ensure that coordinator_ is deallocated.
[viewController.delegate [viewController.delegate
downloadManagerViewControllerDidStartDownload:viewController]; downloadManagerViewControllerDidStartDownload:viewController];
} }
...@@ -764,7 +796,10 @@ TEST_F(DownloadManagerCoordinatorTest, RetryingDownload) { ...@@ -764,7 +796,10 @@ TEST_F(DownloadManagerCoordinatorTest, RetryingDownload) {
ASSERT_EQ([DownloadManagerViewController class], [viewController class]); ASSERT_EQ([DownloadManagerViewController class], [viewController class]);
ASSERT_EQ(0, user_action_tester_.GetActionCount("IOSDownloadStartDownload")); ASSERT_EQ(0, user_action_tester_.GetActionCount("IOSDownloadStartDownload"));
@autoreleasepool { @autoreleasepool {
// This call will retain coordinator, which should outlive thread bundle. // Calling -downloadManagerViewControllerDidStartDownload: will retain and
// autorelease coordinator_. task_environment_ has to outlive the
// coordinator, so wrapping -downloadManagerViewControllerDidStartDownload:
// call in @autorelease will ensure that coordinator_ is deallocated.
[viewController.delegate [viewController.delegate
downloadManagerViewControllerDidStartDownload:viewController]; downloadManagerViewControllerDidStartDownload:viewController];
} }
...@@ -773,7 +808,10 @@ TEST_F(DownloadManagerCoordinatorTest, RetryingDownload) { ...@@ -773,7 +808,10 @@ TEST_F(DownloadManagerCoordinatorTest, RetryingDownload) {
ASSERT_EQ(1, user_action_tester_.GetActionCount("IOSDownloadStartDownload")); ASSERT_EQ(1, user_action_tester_.GetActionCount("IOSDownloadStartDownload"));
@autoreleasepool { @autoreleasepool {
// This call will retain coordinator, which should outlive thread bundle. // Calling -downloadManagerViewControllerDidStartDownload: will retain and
// autorelease coordinator_. task_environment_ has to outlive the
// coordinator, so wrapping -downloadManagerViewControllerDidStartDownload:
// call in @autorelease will ensure that coordinator_ is deallocated.
[viewController.delegate [viewController.delegate
downloadManagerViewControllerDidStartDownload:viewController]; downloadManagerViewControllerDidStartDownload:viewController];
} }
...@@ -811,7 +849,10 @@ TEST_F(DownloadManagerCoordinatorTest, FailingInBackground) { ...@@ -811,7 +849,10 @@ TEST_F(DownloadManagerCoordinatorTest, FailingInBackground) {
base_view_controller_.childViewControllers.firstObject; base_view_controller_.childViewControllers.firstObject;
ASSERT_EQ([DownloadManagerViewController class], [viewController class]); ASSERT_EQ([DownloadManagerViewController class], [viewController class]);
@autoreleasepool { @autoreleasepool {
// This call will retain coordinator, which should outlive thread bundle. // Calling -downloadManagerViewControllerDidStartDownload: will retain and
// autorelease coordinator_. task_environment_ has to outlive the
// coordinator, so wrapping -downloadManagerViewControllerDidStartDownload:
// call in @autorelease will ensure that coordinator_ is deallocated.
[viewController.delegate [viewController.delegate
downloadManagerViewControllerDidStartDownload:viewController]; downloadManagerViewControllerDidStartDownload:viewController];
} }
...@@ -851,7 +892,10 @@ TEST_F(DownloadManagerCoordinatorTest, SucceedingInBackground) { ...@@ -851,7 +892,10 @@ TEST_F(DownloadManagerCoordinatorTest, SucceedingInBackground) {
// Start the download. // Start the download.
@autoreleasepool { @autoreleasepool {
// This call will retain coordinator, which should outlive thread bundle. // Calling -downloadManagerViewControllerDidStartDownload: will retain and
// autorelease coordinator_. task_environment_ has to outlive the
// coordinator, so wrapping -downloadManagerViewControllerDidStartDownload:
// call in @autorelease will ensure that coordinator_ is deallocated.
[viewController.delegate [viewController.delegate
downloadManagerViewControllerDidStartDownload:viewController]; downloadManagerViewControllerDidStartDownload:viewController];
} }
...@@ -884,8 +928,9 @@ TEST_F(DownloadManagerCoordinatorTest, ViewController) { ...@@ -884,8 +928,9 @@ TEST_F(DownloadManagerCoordinatorTest, ViewController) {
EXPECT_NSEQ(viewController, coordinator_.viewController); EXPECT_NSEQ(viewController, coordinator_.viewController);
@autoreleasepool { @autoreleasepool {
// task_environment_ has to outlive the coordinator. Dismissing coordinator // Calling -stop will retain and autorelease coordinator_. task_environment_
// retains are autoreleases it. // has to outlive the coordinator, so wrapping -stop call in @autorelease
// will ensure that coordinator_ is deallocated.
[coordinator_ stop]; [coordinator_ stop];
} }
EXPECT_FALSE(coordinator_.viewController); EXPECT_FALSE(coordinator_.viewController);
......
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