Revert "Log MobileDownloadFileUIInstallGoogleDrive in New Download Manager."

This reverts commit 247dee23.

Reason for revert: Failing on iOS 10 32-bit builder.

Original change's description:
> Log MobileDownloadFileUIInstallGoogleDrive in New Download Manager.
> 
> This CL also changes InstallationNotifier class to be compatible
> with OCMock (UIApplication shared instance is not stored in ivar anymore)
> and adds resetDispatcher method to allow restoring InstallationNotifier
> to its default state.
> 
> Bug: 791806
> Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
> Change-Id: Iaee0d80938639d23862b254732eb49c97ddd4e86
> Reviewed-on: https://chromium-review.googlesource.com/958123
> Commit-Queue: Eugene But <eugenebut@chromium.org>
> Reviewed-by: Peter Lee <pkl@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#543515}

TBR=eugenebut@chromium.org,pkl@chromium.org

Change-Id: I4c90614507c14322c6ab946ac4f8caeedc214cc3
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 791806
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Reviewed-on: https://chromium-review.googlesource.com/966682Reviewed-by: Eugene But <eugenebut@chromium.org>
Commit-Queue: Eugene But <eugenebut@chromium.org>
Cr-Commit-Position: refs/heads/master@{#543721}

ios/chrome/browser/BUILD.gn

@@ -248,7 +248,6 @@ source_set("unit_tests") {
     "//net",
     "//net:test_support",
     "//testing/gtest",
-    "//third_party/ocmock",
     "//url",
   ]
 }

ios/chrome/browser/download/BUILD.gn

@@ -10,8 +10,6 @@ source_set("download") {
     "browser_download_service_factory.mm",
     "download_directory_util.cc",
     "download_directory_util.h",
-    "download_manager_metric_names.cc",
-    "download_manager_metric_names.h",
     "download_manager_tab_helper.h",
     "download_manager_tab_helper.mm",
     "download_manager_tab_helper_delegate.h",

ios/chrome/browser/download/download_manager_metric_names.cc

-// Copyright 2018 The Chromium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#include "ios/chrome/browser/download/download_manager_metric_names.h"
-
-const char kDownloadManagerGoogleDriveInstalled[] =
-    "MobileDownloadFileUIInstallGoogleDrive";

ios/chrome/browser/download/download_manager_metric_names.h

-// Copyright 2018 The Chromium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#ifndef IOS_CHROME_BROWSER_DOWNLOAD_DOWNLOAD_MANAGER_METRIC_NAMES_H_
-#define IOS_CHROME_BROWSER_DOWNLOAD_DOWNLOAD_MANAGER_METRIC_NAMES_H_
-
-// MobileDownloadFileUIInstallGoogleDrive UMA action. Logged when Google Drive
-// app is installed after presenting Store Kit dialog from the Download Manager.
-extern const char kDownloadManagerGoogleDriveInstalled[];
-
-#endif  // IOS_CHROME_BROWSER_DOWNLOAD_DOWNLOAD_MANAGER_METRIC_NAMES_H_

ios/chrome/browser/installation_notifier.mm

@@ -59,6 +59,9 @@ const net::BackoffEntry::Policy kPollingBackoffPolicy = {
 @interface InstallationNotifier (Testing)
 // Sets the dispatcher.
 - (void)setDispatcher:(id<DispatcherProtocol>)dispatcher;
+// Sets the UIApplication used to determine if a scheme can be opened by an
+// application.
+- (void)setSharedApplication:(UIApplication*)sharedApplication;
 @end
 
 @implementation InstallationNotifier {
@@ -86,6 +89,7 @@ const net::BackoffEntry::Policy kPollingBackoffPolicy = {
     _dispatcher = [[DefaultDispatcher alloc] init];
     _installedAppObservers = [[NSMutableDictionary alloc] init];
     _notificationCenter = [NSNotificationCenter defaultCenter];
+    sharedApplication_ = [UIApplication sharedApplication];
     _backoffEntry.reset(new net::BackoffEntry([self backOffPolicy]));
   }
   return self;
@@ -182,7 +186,7 @@ const net::BackoffEntry::Policy kPollingBackoffPolicy = {
     DCHECK([observers count] > 0);
     NSURL* testSchemeURL =
         [NSURL URLWithString:[NSString stringWithFormat:@"%@:", scheme]];
-    if ([[UIApplication sharedApplication] canOpenURL:testSchemeURL]) {
+    if ([sharedApplication_ canOpenURL:testSchemeURL]) {
       [_notificationCenter postNotificationName:scheme object:self];
       for (id weakReferenceToObserver in observers) {
         id observer = [weakReferenceToObserver nonretainedObjectValue];
@@ -212,8 +216,11 @@ const net::BackoffEntry::Policy kPollingBackoffPolicy = {
   _dispatcher = dispatcher;
 }
 
-- (void)resetDispatcher {
-  _dispatcher = [[DefaultDispatcher alloc] init];
+- (void)setSharedApplication:(id)sharedApplication {
+  // Verify that the test application object responds to all the selectors that
+  // will be called on it.
+  CHECK([sharedApplication respondsToSelector:@selector(canOpenURL:)]);
+  sharedApplication_ = (UIApplication*)sharedApplication;
 }
 
 @end

ios/chrome/browser/installation_notifier_unittest.mm

@@ -13,7 +13,6 @@
 #include "ios/web/public/test/test_web_thread.h"
 #include "net/base/backoff_entry.h"
 #include "testing/platform_test.h"
-#import "third_party/ocmock/OCMock/OCMock.h"
 
 #if !defined(__has_feature) || !__has_feature(objc_arc)
 #error "This file requires ARC support."
@@ -79,9 +78,27 @@
 
 @end
 
+@interface MockUIApplication : NSObject
+// Mocks UIApplication's canOpenURL.
+@end
+
+@implementation MockUIApplication {
+  BOOL canOpen_;
+}
+
+- (void)setCanOpenURL:(BOOL)canOpen {
+  canOpen_ = canOpen;
+}
+
+- (BOOL)canOpenURL:(NSURL*)url {
+  return canOpen_;
+}
+
+@end
+
 @interface InstallationNotifier (Testing)
 - (void)setDispatcher:(id<DispatcherProtocol>)dispatcher;
-- (void)resetDispatcher;
+- (void)setSharedApplication:(id)sharedApplication;
 - (void)dispatchInstallationNotifierBlock;
 - (void)registerForInstallationNotifications:(id)observer
                                 withSelector:(SEL)notificationSelector
@@ -103,16 +120,12 @@ class InstallationNotifierTest : public PlatformTest {
     dispatcher_ = dispatcher;
     notificationReceiver1_ = ([[MockNotificationReceiver alloc] init]);
     notificationReceiver2_ = ([[MockNotificationReceiver alloc] init]);
-    application_ = OCMClassMock([UIApplication class]);
-    OCMStub([application_ sharedApplication]).andReturn(application_);
+    sharedApplication_ = [[MockUIApplication alloc] init];
+    [installationNotifier_ setSharedApplication:sharedApplication_];
     [installationNotifier_ setDispatcher:dispatcher_];
     histogramTester_.reset(new base::HistogramTester());
   }
 
-  ~InstallationNotifierTest() override {
-    [installationNotifier_ resetDispatcher];
-  }
-
   void VerifyHistogramValidity(int expectedYes, int expectedNo) {
     histogramTester_->ExpectTotalCount("NativeAppLauncher.InstallationDetected",
                                        expectedYes + expectedNo);
@@ -141,12 +154,12 @@ class InstallationNotifierTest : public PlatformTest {
   __weak FakeDispatcher* dispatcher_;
   MockNotificationReceiver* notificationReceiver1_;
   MockNotificationReceiver* notificationReceiver2_;
-  id application_;
+  MockUIApplication* sharedApplication_;
   std::unique_ptr<base::HistogramTester> histogramTester_;
 };
 
 TEST_F(InstallationNotifierTest, RegisterWithAppAlreadyInstalled) {
-  OCMStub([application_ canOpenURL:[OCMArg any]]).andReturn(YES);
+  [sharedApplication_ setCanOpenURL:YES];
   [installationNotifier_
       registerForInstallationNotifications:notificationReceiver1_
                               withSelector:@selector(receivedNotification)
@@ -161,11 +174,11 @@ TEST_F(InstallationNotifierTest, RegisterWithAppAlreadyInstalled) {
 }
 
 TEST_F(InstallationNotifierTest, RegisterWithAppInstalledAfterSomeTime) {
-  [dispatcher_
-      executeAfter:10
-             block:^{
-               OCMStub([application_ canOpenURL:[OCMArg any]]).andReturn(YES);
-             }];
+  [sharedApplication_ setCanOpenURL:NO];
+  [dispatcher_ executeAfter:10
+                      block:^{
+                        [sharedApplication_ setCanOpenURL:YES];
+                      }];
   [installationNotifier_
       registerForInstallationNotifications:notificationReceiver1_
                               withSelector:@selector(receivedNotification)
@@ -175,11 +188,11 @@ TEST_F(InstallationNotifierTest, RegisterWithAppInstalledAfterSomeTime) {
 }
 
 TEST_F(InstallationNotifierTest, RegisterForTwoInstallations) {
-  [dispatcher_
-      executeAfter:10
-             block:^{
-               OCMStub([application_ canOpenURL:[OCMArg any]]).andReturn(YES);
-             }];
+  [sharedApplication_ setCanOpenURL:NO];
+  [dispatcher_ executeAfter:10
+                      block:^{
+                        [sharedApplication_ setCanOpenURL:YES];
+                      }];
   [installationNotifier_
       registerForInstallationNotifications:notificationReceiver1_
                               withSelector:@selector(receivedNotification)
@@ -202,7 +215,7 @@ TEST_F(InstallationNotifierTest, RegisterForTwoInstallations) {
 }
 
 TEST_F(InstallationNotifierTest, RegisterAndThenUnregister) {
-  OCMStub([application_ canOpenURL:[OCMArg any]]).andReturn(NO);
+  [sharedApplication_ setCanOpenURL:NO];
   [dispatcher_ executeAfter:10
                       block:^{
                         [installationNotifier_
@@ -217,7 +230,7 @@ TEST_F(InstallationNotifierTest, RegisterAndThenUnregister) {
 }
 
 TEST_F(InstallationNotifierTest, TestExponentialBackoff) {
-  OCMStub([application_ canOpenURL:[OCMArg any]]).andReturn(NO);
+  [sharedApplication_ setCanOpenURL:NO];
   // Making sure that delay is multiplied by |multiplyFactor| every time.
   [dispatcher_ executeAfter:0
                       block:^{

ios/chrome/browser/ui/download/BUILD.gn

@@ -79,7 +79,6 @@ source_set("unit_tests") {
     "//ios/chrome/browser/ui:ui_util",
     "//ios/chrome/browser/web:test_support",
     "//ios/chrome/test:test_support",
-    "//ios/chrome/test/app:test_support",
     "//ios/chrome/test/fakes",
     "//ios/testing:ios_test_support",
     "//ios/web/public/test",

ios/chrome/browser/ui/download/download_manager_coordinator.mm

@@ -7,14 +7,10 @@
 #include <memory>
 
 #import "base/logging.h"
-#include "base/metrics/user_metrics.h"
-#include "base/metrics/user_metrics_action.h"
 #include "base/strings/sys_string_conversions.h"
 #include "components/strings/grit/components_strings.h"
-#include "ios/chrome/browser/download/download_manager_metric_names.h"
 #import "ios/chrome/browser/download/download_manager_tab_helper.h"
 #import "ios/chrome/browser/download/google_drive_app_util.h"
-#import "ios/chrome/browser/installation_notifier.h"
 #import "ios/chrome/browser/store_kit/store_kit_coordinator.h"
 #import "ios/chrome/browser/ui/download/download_manager_mediator.h"
 #import "ios/chrome/browser/ui/download/download_manager_view_controller.h"
@@ -49,10 +45,6 @@
 @synthesize animatesPresentation = _animatesPresentation;
 @synthesize downloadTask = _downloadTask;
 
-- (void)dealloc {
-  [[InstallationNotifier sharedInstance] unregisterForNotifications:self];
-}
-
 - (void)start {
   DCHECK(self.presenter);
 
@@ -179,11 +171,6 @@
   }
   [_storeKitCoordinator start];
   [controller setInstallDriveButtonVisible:NO animated:YES];
-
-  [[InstallationNotifier sharedInstance]
-      registerForInstallationNotifications:self
-                              withSelector:@selector(didInstallGoogleDriveApp)
-                                 forScheme:kGoogleDriveAppURLScheme];
 }
 
 - (void)downloadManagerViewControllerDidStartDownload:
@@ -249,10 +236,4 @@
                                       completion:nil];
 }
 
-// Called when Google Drive app is installed after starting StoreKitCoordinator.
-- (void)didInstallGoogleDriveApp {
-  base::RecordAction(
-      base::UserMetricsAction(kDownloadManagerGoogleDriveInstalled));
-}
-
 @end

ios/chrome/browser/ui/download/download_manager_coordinator_unittest.mm

@@ -11,11 +11,8 @@
 #include "base/mac/foundation_util.h"
 #include "base/run_loop.h"
 #include "base/strings/sys_string_conversions.h"
-#include "base/test/user_action_tester.h"
 #include "ios/chrome/browser/download/download_directory_util.h"
-#include "ios/chrome/browser/download/download_manager_metric_names.h"
 #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/test/fakes/fake_contained_presenter.h"
 #import "ios/chrome/test/fakes/fake_document_interaction_controller.h"
@@ -81,22 +78,15 @@ class DownloadManagerCoordinatorTest : public PlatformTest {
   }
   ~DownloadManagerCoordinatorTest() override {
     [document_interaction_controller_class_ stopMocking];
-    [application_ stopMocking];
   }
 
-  web::TestWebThreadBundle thread_bundle_;
   FakeContainedPresenter* presenter_;
   UIViewController* base_view_controller_;
   ScopedKeyWindow scoped_key_window_;
   web::TestWebState web_state_;
   id document_interaction_controller_class_;
   StubTabHelper tab_helper_;
-  // Application can be lazily created by tests, but it has to be OCMock.
-  // Destructor will call -stopMocking on this object to make sure that
-  // UIApplication is not mocked after these test finish running.
-  id application_;
   DownloadManagerCoordinator* coordinator_;
-  base::UserActionTester user_action_tester_;
 };
 
 // Tests starting the coordinator. Verifies that view controller is presented
@@ -284,11 +274,8 @@ TEST_F(DownloadManagerCoordinatorTest, Close) {
   DownloadManagerViewController* viewController =
       base_view_controller_.childViewControllers.firstObject;
   ASSERT_EQ([DownloadManagerViewController class], [viewController class]);
-  @autoreleasepool {
-    // This call will retain coordinator, which should outlive thread bundle.
-    [viewController.delegate
-        downloadManagerViewControllerDidClose:viewController];
-  }
+  [viewController.delegate
+      downloadManagerViewControllerDidClose:viewController];
 
   // Verify that child view controller is removed, download task is set to null
   // and download task is cancelled.
@@ -318,11 +305,9 @@ TEST_F(DownloadManagerCoordinatorTest, InstallDrive) {
   // button changes it's alpha.
   ASSERT_EQ(1.0f, viewController.installDriveButton.superview.alpha);
 
-  @autoreleasepool {
-    // This call will retain coordinator, which should outlive thread bundle.
-    [viewController.delegate
-        installDriveForDownloadManagerViewController:viewController];
-  }
+  [viewController.delegate
+      installDriveForDownloadManagerViewController:viewController];
+
   // Verify that Store Kit dialog was presented.
   EXPECT_TRUE(WaitUntilConditionOrTimeout(kWaitForUIElementTimeout, ^{
     return [base_view_controller_.presentedViewController class] ==
@@ -334,27 +319,14 @@ TEST_F(DownloadManagerCoordinatorTest, InstallDrive) {
     return viewController.installDriveButton.superview.alpha == 0.0f;
   }));
 
-  // Simulate Google Drive app installation and verify that expected user action
-  // has been recorded.
-  ASSERT_EQ(0, user_action_tester_.GetActionCount(
-                   kDownloadManagerGoogleDriveInstalled));
-  // SKStoreProductViewController uses UIApplication, so it's not possible to
-  // install the mock before the test run.
-  application_ = OCMClassMock([UIApplication class]);
-  OCMStub([application_ sharedApplication]).andReturn(application_);
-  OCMStub([application_ canOpenURL:GetGoogleDriveAppUrl()]).andReturn(YES);
-  EXPECT_TRUE(WaitUntilConditionOrTimeout(testing::kWaitForActionTimeout, ^{
-    base::RunLoop().RunUntilIdle();
-    return user_action_tester_.GetActionCount(
-               kDownloadManagerGoogleDriveInstalled) == 1;
-  }));
-
   // Stop to avoid holding a dangling pointer to destroyed task.
   [coordinator_ stop];
 }
 
 // Tests presenting Open In... menu.
 TEST_F(DownloadManagerCoordinatorTest, OpenIn) {
+  web::TestWebThreadBundle thread_bundle;
+
   web::FakeDownloadTask task(GURL(kTestUrl), kTestMimeType);
   task.SetSuggestedFilename(base::SysNSStringToUTF16(kTestSuggestedFileName));
   coordinator_.downloadTask = &task;
@@ -385,11 +357,8 @@ TEST_F(DownloadManagerCoordinatorTest, OpenIn) {
   UIView* view = [[UIView alloc] init];
   [view addLayoutGuide:guide];
   ASSERT_FALSE(document_interaction_controller.presentedOpenInMenu);
-  @autoreleasepool {
-    // This call will retain coordinator, which should outlive thread bundle.
-    [viewController.delegate downloadManagerViewController:viewController
-                          presentOpenInMenuWithLayoutGuide:guide];
-  }
+  [viewController.delegate downloadManagerViewController:viewController
+                        presentOpenInMenuWithLayoutGuide:guide];
   ASSERT_TRUE(document_interaction_controller.presentedOpenInMenu);
   ASSERT_TRUE(CGRectEqualToRect(
       CGRectZero, document_interaction_controller.presentedOpenInMenu.rect));
@@ -409,11 +378,9 @@ TEST_F(DownloadManagerCoordinatorTest, CloseInProgressDownload) {
   DownloadManagerViewController* viewController =
       base_view_controller_.childViewControllers.firstObject;
   ASSERT_EQ([DownloadManagerViewController class], [viewController class]);
-  @autoreleasepool {
-    // This call will retain coordinator, which should outlive thread bundle.
-    [viewController.delegate
-        downloadManagerViewControllerDidClose:viewController];
-  }
+  [viewController.delegate
+      downloadManagerViewControllerDidClose:viewController];
+
   // Verify that UIAlert is presented.
   ASSERT_TRUE([base_view_controller_.presentedViewController
       isKindOfClass:[UIAlertController class]]);
@@ -461,6 +428,8 @@ TEST_F(DownloadManagerCoordinatorTest, DecidePolicyForDownload) {
 // Tests starting the download. Verifies that download task is started and its
 // file writer is configured to write into download directory.
 TEST_F(DownloadManagerCoordinatorTest, StartDownload) {
+  web::TestWebThreadBundle thread_bundle;
+
   web::FakeDownloadTask task(GURL(kTestUrl), kTestMimeType);
   task.SetSuggestedFilename(base::SysNSStringToUTF16(kTestSuggestedFileName));
   web::DownloadTask* task_ptr = &task;
@@ -470,11 +439,8 @@ TEST_F(DownloadManagerCoordinatorTest, StartDownload) {
   DownloadManagerViewController* viewController =
       base_view_controller_.childViewControllers.firstObject;
   ASSERT_EQ([DownloadManagerViewController class], [viewController class]);
-  @autoreleasepool {
-    // This call will retain coordinator, which should outlive thread bundle.
-    [viewController.delegate
-        downloadManagerViewControllerDidStartDownload:viewController];
-  }
+  [viewController.delegate
+      downloadManagerViewControllerDidStartDownload:viewController];
 
   // Starting download is async for model.
   ASSERT_TRUE(WaitUntilConditionOrTimeout(testing::kWaitForDownloadTimeout, ^{

ios/chrome/browser/ui/main/browser_view_wrangler_unittest.mm

@@ -30,33 +30,30 @@ class BrowserViewWranglerTest : public PlatformTest {
 };
 
 TEST_F(BrowserViewWranglerTest, TestInitNilObserver) {
-  // |thread_bundle_| must outlive all objects created by BVC, because those
-  // objects may rely on threading API in dealloc.
-  @autoreleasepool {
-    BrowserViewWrangler* wrangler = [[BrowserViewWrangler alloc]
-              initWithBrowserState:chrome_browser_state_.get()
-
-                  tabModelObserver:nil
-        applicationCommandEndpoint:(id<ApplicationCommands>)nil];
-    // Test that BVC and tab model are created on demand.
-    BrowserViewController* bvc = [wrangler mainBVC];
-    EXPECT_NE(bvc, nil);
-
-    TabModel* tabModel = [wrangler mainTabModel];
-    EXPECT_NE(tabModel, nil);
-
-    // Test that once created the BVC and tab model aren't re-created.
-    EXPECT_EQ(bvc, [wrangler mainBVC]);
-    EXPECT_EQ(tabModel, [wrangler mainTabModel]);
-
-    // Test that the OTR objects are (a) OTR and (b) not the same as the non-OTR
-    // objects.
-    EXPECT_NE(bvc, [wrangler otrBVC]);
-    EXPECT_NE(tabModel, [wrangler otrTabModel]);
-    EXPECT_TRUE([wrangler otrTabModel].browserState->IsOffTheRecord());
-
-    [wrangler shutdown];
-  }
+  BrowserViewWrangler* wrangler = [[BrowserViewWrangler alloc]
+            initWithBrowserState:chrome_browser_state_.get()
+
+                tabModelObserver:nil
+      applicationCommandEndpoint:(id<ApplicationCommands>)nil];
+
+  // Test that BVC and tab model are created on demand.
+  BrowserViewController* bvc = [wrangler mainBVC];
+  EXPECT_NE(bvc, nil);
+
+  TabModel* tabModel = [wrangler mainTabModel];
+  EXPECT_NE(tabModel, nil);
+
+  // Test that once created the BVC and tab model aren't re-created.
+  EXPECT_EQ(bvc, [wrangler mainBVC]);
+  EXPECT_EQ(tabModel, [wrangler mainTabModel]);
+
+  // Test that the OTR objects are (a) OTR and (b) not the same as the non-OTR
+  // objects.
+  EXPECT_NE(bvc, [wrangler otrBVC]);
+  EXPECT_NE(tabModel, [wrangler otrTabModel]);
+  EXPECT_TRUE([wrangler otrTabModel].browserState->IsOffTheRecord());
+
+  [wrangler shutdown];
 }
 
 }  // namespace