Fix SKStoreProductViewController dismissal in StoreKitCoordinator.

Do not call -dismissViewControllerAnimated:completion: on
|self.baseViewController|, since the receiver of the method can be
dismissed if there is no presented view controller. On iOS 12
SKStoreProductViewControllerDelegate is responsible for dismissing
SKStoreProductViewController. On iOS 13.0 OS dismisses
SKStoreProductViewController after calling -productViewControllerDidFinish:
On iOS 13.2 OS dismisses SKStoreProductViewController before calling
-productViewControllerDidFinish: Calling
-dismissViewControllerAnimated:completion: on |self.baseViewController| on
iOS 13.2 will dismiss base view controller and break the application UI.
According to SKStoreProductViewController documentation the delegate is
responsible for calling deprecated dismissModalViewControllerAnimated: so
the documentation is clearly outdated and this code should be resilient to
different SKStoreProductViewController behavior without relying on iOS
version check (see crbug.com/1027058).

Bug: 1027058
Change-Id: Ib3918930b9950e79b088f14b3d0424e42d7b5c58
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1928590
Commit-Queue: Eugene But <eugenebut@chromium.org>
Reviewed-by: Mohammad Refaat <mrefaat@chromium.org>
Reviewed-by: edchin <edchin@chromium.org>
Auto-Submit: Eugene But <eugenebut@chromium.org>
Cr-Commit-Position: refs/heads/master@{#720711}

ios/chrome/browser/store_kit/store_kit_coordinator.mm

@@ -47,7 +47,23 @@
 }
 
 - (void)stop {
-  [self.baseViewController dismissViewControllerAnimated:YES completion:nil];
+  // Do not call -dismissViewControllerAnimated:completion: on
+  // |self.baseViewController|, since the receiver of the method can be
+  // dismissed if there is no presented view controller. On iOS 12
+  // SKStoreProductViewControllerDelegate is responsible for dismissing
+  // SKStoreProductViewController. On iOS 13.0 OS dismisses
+  // SKStoreProductViewController after calling -productViewControllerDidFinish:
+  // On iOS 13.2 OS dismisses SKStoreProductViewController before calling
+  // -productViewControllerDidFinish: Calling
+  // -dismissViewControllerAnimated:completion: on |self.baseViewController| on
+  // iOS 13.2 will dismiss base view controller and break the application UI.
+  // According to SKStoreProductViewController documentation the delegate is
+  // responsible for calling deprecated dismissModalViewControllerAnimated: so
+  // the documentation is clearly outdated and this code should be resilient to
+  // different SKStoreProductViewController behavior without relying on iOS
+  // version check (see crbug.com/1027058).
+  [self.viewController dismissViewControllerAnimated:YES completion:nil];
+
   self.viewController = nil;
 }
 

ios/chrome/browser/store_kit/store_kit_coordinator_unittest.mm

@@ -21,10 +21,15 @@
 class StoreKitCoordinatorTest : public PlatformTest {
  protected:
   StoreKitCoordinatorTest()
-      : base_view_controller_([[UIViewController alloc] init]),
+      : root_view_controller_([[UIViewController alloc] init]),
+        base_view_controller_([[UIViewController alloc] init]),
+
         coordinator_([[StoreKitCoordinator alloc]
             initWithBaseViewController:base_view_controller_]) {
-    [scoped_key_window_.Get() setRootViewController:base_view_controller_];
+    [scoped_key_window_.Get() setRootViewController:root_view_controller_];
+    [root_view_controller_ presentViewController:base_view_controller_
+                                        animated:NO
+                                      completion:nil];
   }
 
   ~StoreKitCoordinatorTest() override {
@@ -38,6 +43,7 @@ class StoreKitCoordinatorTest : public PlatformTest {
     }
   }
 
+  UIViewController* root_view_controller_;
   UIViewController* base_view_controller_;
   StoreKitCoordinator* coordinator_;
   ScopedKeyWindow scoped_key_window_;
@@ -162,6 +168,10 @@ TEST_F(StoreKitCoordinatorTest, MAYBE_NoOverlappingPresentedViewControllers) {
   [base_view_controller_ presentViewController:dummy_view_controller
                                       animated:NO
                                     completion:nil];
+  EXPECT_TRUE(base::test::ios::WaitUntilConditionOrTimeout(
+      base::test::ios::kWaitForUIElementTimeout, ^bool {
+        return base_view_controller_.presentedViewController;
+      }));
   EXPECT_NSEQ(dummy_view_controller,
               base_view_controller_.presentedViewController);
 
@@ -175,6 +185,7 @@ TEST_F(StoreKitCoordinatorTest, MAYBE_NoOverlappingPresentedViewControllers) {
   EXPECT_NSEQ(dummy_view_controller,
               base_view_controller_.presentedViewController);
   [coordinator_ stop];
+  [dummy_view_controller dismissViewControllerAnimated:NO completion:nil];
   EXPECT_TRUE(base::test::ios::WaitUntilConditionOrTimeout(
       base::test::ios::kWaitForActionTimeout, ^bool {
         return !base_view_controller_.presentedViewController;
@@ -193,3 +204,31 @@ TEST_F(StoreKitCoordinatorTest, MAYBE_NoOverlappingPresentedViewControllers) {
   EXPECT_EQ([SKStoreProductViewController class],
             [base_view_controller_.presentedViewController class]);
 }
+
+// iOS 13 dismisses SKStoreProductViewController when user taps "Done". This
+// test makes sure that StoreKitCoordinator gracefully handles the situation.
+TEST_F(StoreKitCoordinatorTest, StopAfterDismissingPresentedViewController) {
+  NSDictionary* product_params = @{
+    SKStoreProductParameterITunesItemIdentifier : @"TestITunesItemIdentifier",
+  };
+  [coordinator_ openAppStoreWithParameters:product_params];
+  EXPECT_TRUE(base::test::ios::WaitUntilConditionOrTimeout(
+      base::test::ios::kWaitForUIElementTimeout, ^bool {
+        return base_view_controller_.presentedViewController;
+      }));
+
+  // iOS 13 dismisses SKStoreProductViewController when user taps "Done".
+  [base_view_controller_.presentedViewController
+      dismissViewControllerAnimated:NO
+                         completion:nil];
+  EXPECT_TRUE(base::test::ios::WaitUntilConditionOrTimeout(
+      base::test::ios::kWaitForUIElementTimeout, ^{
+        return !base_view_controller_.presentedViewController;
+      }));
+
+  // Make sure that base view controller is not dismissed (crbug.com.1027058).
+  [coordinator_ stop];
+  EXPECT_FALSE(base::test::ios::WaitUntilConditionOrTimeout(1.0, ^{
+    return !root_view_controller_.presentedViewController;
+  }));
+}