[ios] Stops checking for a presentedVC on AR and Store Kit presentations.

Due to https://crbug.com/965688 we need to dismiss the InfobarBanner
before presenting a new ViewController.

Most Coordinators present without checking if baseVC is already
presenting another VC (Even some where web is triggering the presentation,
like PassKitCoordinator).

Before this CL the VC's trying to be presented will NO-OP if a VC is already
presented.
This CL stops checking if baseVC is presenting so it lets BVC decide if it
should present or not (BVC will dismiss any InfobarBanner before presenting
any other VC).

This means that if an InfobarBanner is being presented
this will no longer NO-OP. It will dismiss the InfobarBanner first and then
present StoreKit or ARQuickLook.

In order to make sure that we don't re-present a ViewController again, the
properties are now weak so a failed presentation (meaning they are not part
of the view hierarchy) will set the ViewController property to nil.

StoreKitCoordinatorTest now uses a real UIViewController since
FakeUIViewController presentedViewController doesn't behave as a
UIViewController, meaning that if we present a VC on top of an already
presented VC the new VC gets presented, which shouldn't happen.


Bug: 911864
Change-Id: I15f8d5b4a7f866295fece58423fde8c59ec97e7a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1623497Reviewed-by: Mohammad Refaat <mrefaat@chromium.org>
Reviewed-by: Mark Cogan <marq@chromium.org>
Reviewed-by: Peter Lee <pkl@chromium.org>
Reviewed-by: Eugene But <eugenebut@chromium.org>
Commit-Queue: Sergio Collazos <sczs@chromium.org>
Commit-Queue: Moe Ahmadi <mahmadi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#663068}

ios/chrome/browser/store_kit/store_kit_coordinator.mm

@@ -14,9 +14,10 @@
 #error "This file requires ARC support."
 #endif
 
-@interface StoreKitCoordinator ()<SKStoreProductViewControllerDelegate> {
-  SKStoreProductViewController* _viewController;
-}
+@interface StoreKitCoordinator () <SKStoreProductViewControllerDelegate>
+// StoreKitViewController to present. Set as a weak reference so it only exists
+// while its being presented by baseViewController.
+@property(nonatomic, weak) SKStoreProductViewController* viewController;
 @end
 
 @implementation StoreKitCoordinator
@@ -27,26 +28,27 @@
 - (void)start {
   DCHECK(self.iTunesProductParameters
              [SKStoreProductParameterITunesItemIdentifier]);
-  // StoreKit shouldn't be launched, if there is one already presented or if
-  // there is another view presented by the base view controller.
-  if (_viewController || self.baseViewController.presentedViewController)
+  // StoreKit shouldn't be launched, if there is one already presented.
+  if (self.viewController)
     return;
-  _viewController = [[SKStoreProductViewController alloc] init];
-  _viewController.delegate = self;
-  [_viewController
+  SKStoreProductViewController* viewController =
+      [[SKStoreProductViewController alloc] init];
+  viewController.delegate = self;
+  [viewController
       loadProductWithParameters:self.iTunesProductParameters
                 completionBlock:^(BOOL result, NSError* _Nullable error) {
                   UMA_HISTOGRAM_BOOLEAN("IOS.StoreKitLoadedSuccessfully",
                                         result);
                 }];
-  [self.baseViewController presentViewController:_viewController
+  [self.baseViewController presentViewController:viewController
                                         animated:YES
                                       completion:nil];
+  self.viewController = viewController;
 }
 
 - (void)stop {
   [self.baseViewController dismissViewControllerAnimated:YES completion:nil];
-  _viewController = nil;
+  self.viewController = nil;
 }
 
 #pragma mark - StoreKitLauncher

ios/chrome/browser/store_kit/store_kit_coordinator_unittest.mm

@@ -6,6 +6,7 @@
 
 #import <StoreKit/StoreKit.h>
 
+#import "base/test/ios/wait_util.h"
 #import "ios/chrome/test/fakes/fake_ui_view_controller.h"
 #import "ios/chrome/test/scoped_key_window.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -20,13 +21,24 @@
 class StoreKitCoordinatorTest : public PlatformTest {
  protected:
   StoreKitCoordinatorTest()
-      : base_view_controller_([[FakeUIViewController alloc] init]),
+      : base_view_controller_([[UIViewController alloc] init]),
         coordinator_([[StoreKitCoordinator alloc]
             initWithBaseViewController:base_view_controller_]) {
     [scoped_key_window_.Get() setRootViewController:base_view_controller_];
   }
 
-  FakeUIViewController* base_view_controller_;
+  ~StoreKitCoordinatorTest() override {
+    // Make sure StoreKit has been dismissed.
+    if (base_view_controller_.presentedViewController) {
+      [coordinator_ stop];
+      EXPECT_TRUE(base::test::ios::WaitUntilConditionOrTimeout(
+          base::test::ios::kWaitForActionTimeout, ^bool {
+            return !base_view_controller_.presentedViewController;
+          }));
+    }
+  }
+
+  UIViewController* base_view_controller_;
   StoreKitCoordinator* coordinator_;
   ScopedKeyWindow scoped_key_window_;
 };
@@ -39,11 +51,20 @@ TEST_F(StoreKitCoordinatorTest, OpenStoreWithParamsPresentViewController) {
     SKStoreProductParameterAffiliateToken : @"TestToken"
   };
   [coordinator_ openAppStoreWithParameters:product_params];
+  EXPECT_TRUE(base::test::ios::WaitUntilConditionOrTimeout(
+      base::test::ios::kWaitForActionTimeout, ^bool {
+        return base_view_controller_.presentedViewController;
+      }));
+
   EXPECT_NSEQ(product_params, coordinator_.iTunesProductParameters);
 
   EXPECT_NSEQ([SKStoreProductViewController class],
               [base_view_controller_.presentedViewController class]);
   [coordinator_ stop];
+  EXPECT_TRUE(base::test::ios::WaitUntilConditionOrTimeout(
+      base::test::ios::kWaitForActionTimeout, ^bool {
+        return !base_view_controller_.presentedViewController;
+      }));
 
   EXPECT_FALSE(base_view_controller_.presentedViewController);
 }
@@ -56,12 +77,21 @@ TEST_F(StoreKitCoordinatorTest, OpenStorePresentViewController) {
     SKStoreProductParameterITunesItemIdentifier : kTestITunesItemIdentifier,
   };
   [coordinator_ openAppStore:kTestITunesItemIdentifier];
+  EXPECT_TRUE(base::test::ios::WaitUntilConditionOrTimeout(
+      base::test::ios::kWaitForActionTimeout, ^bool {
+        return base_view_controller_.presentedViewController;
+      }));
+
   EXPECT_NSEQ(product_params, coordinator_.iTunesProductParameters);
 
   EXPECT_NSEQ([SKStoreProductViewController class],
               [base_view_controller_.presentedViewController class]);
 
   [coordinator_ stop];
+  EXPECT_TRUE(base::test::ios::WaitUntilConditionOrTimeout(
+      base::test::ios::kWaitForActionTimeout, ^bool {
+        return !base_view_controller_.presentedViewController;
+      }));
 
   EXPECT_FALSE(base_view_controller_.presentedViewController);
 }
@@ -74,6 +104,10 @@ TEST_F(StoreKitCoordinatorTest, NoOverlappingStoreKitsPresented) {
     SKStoreProductParameterITunesItemIdentifier : kTestITunesItemIdentifier,
   };
   [coordinator_ start];
+  EXPECT_TRUE(base::test::ios::WaitUntilConditionOrTimeout(
+      base::test::ios::kWaitForActionTimeout, ^bool {
+        return base_view_controller_.presentedViewController;
+      }));
 
   EXPECT_NSEQ([SKStoreProductViewController class],
               [base_view_controller_.presentedViewController class]);
@@ -87,9 +121,19 @@ TEST_F(StoreKitCoordinatorTest, NoOverlappingStoreKitsPresented) {
               base_view_controller_.presentedViewController);
 
   [coordinator_ stop];
+  EXPECT_TRUE(base::test::ios::WaitUntilConditionOrTimeout(
+      base::test::ios::kWaitForActionTimeout, ^bool {
+        return !base_view_controller_.presentedViewController;
+      }));
+
   EXPECT_FALSE(base_view_controller_.presentedViewController);
 
   [coordinator_ start];
+  EXPECT_TRUE(base::test::ios::WaitUntilConditionOrTimeout(
+      base::test::ios::kWaitForActionTimeout, ^bool {
+        return base_view_controller_.presentedViewController;
+      }));
+
   // After reseting the view controller, a new storekit view should be
   // presented.
   EXPECT_NSEQ([SKStoreProductViewController class],
@@ -114,13 +158,28 @@ TEST_F(StoreKitCoordinatorTest, NoOverlappingPresentedViewControllers) {
               base_view_controller_.presentedViewController);
 
   [coordinator_ start];
+  EXPECT_TRUE(base::test::ios::WaitUntilConditionOrTimeout(
+      base::test::ios::kWaitForActionTimeout, ^bool {
+        return base_view_controller_.presentedViewController;
+      }));
+
   // Verify that that presented view controlled is not changed.
   EXPECT_NSEQ(dummy_view_controller,
               base_view_controller_.presentedViewController);
   [coordinator_ stop];
+  EXPECT_TRUE(base::test::ios::WaitUntilConditionOrTimeout(
+      base::test::ios::kWaitForActionTimeout, ^bool {
+        return !base_view_controller_.presentedViewController;
+      }));
+
   EXPECT_FALSE(base_view_controller_.presentedViewController);
 
   [coordinator_ start];
+  EXPECT_TRUE(base::test::ios::WaitUntilConditionOrTimeout(
+      base::test::ios::kWaitForActionTimeout, ^bool {
+        return base_view_controller_.presentedViewController;
+      }));
+
   // After reseting the view controller, a new storekit view should be
   // presented.
   EXPECT_NSEQ([SKStoreProductViewController class],

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

@@ -66,8 +66,9 @@ PresentQLPreviewController GetHistogramEnum(
 // The file URL pointing to the downloaded USDZ format file.
 @property(nonatomic, copy) NSURL* fileURL;
 
-// Displays USDZ format files.
-@property(nonatomic, strong) QLPreviewController* viewController;
+// Displays USDZ format files. Set as a weak reference so it only exists while
+// its being presented by baseViewController.
+@property(nonatomic, weak) QLPreviewController* viewController;
 
 @end
 
@@ -183,18 +184,18 @@ PresentQLPreviewController GetHistogramEnum(
       kIOSPresentQLPreviewControllerHistogram,
       GetHistogramEnum(self.baseViewController, self.fileURL));
 
-  // QLPreviewController should not be presented if there is already a view
-  // controller presented by the base view controller or the file URL is nil.
-  if (self.baseViewController.presentedViewController || !self.fileURL) {
+  // QLPreviewController should not be presented if the file URL is nil.
+  if (!self.fileURL) {
     return;
   }
 
-  self.viewController = [[QLPreviewController alloc] init];
-  self.viewController.dataSource = self;
-  self.viewController.delegate = self;
-  [self.baseViewController presentViewController:self.viewController
+  QLPreviewController* viewController = [[QLPreviewController alloc] init];
+  viewController.dataSource = self;
+  viewController.delegate = self;
+  [self.baseViewController presentViewController:viewController
                                         animated:YES
                                       completion:nil];
+  self.viewController = viewController;
 }
 
 #pragma mark - QLPreviewControllerDataSource