[PR] Fixes a crash caused by navigation to another page right after abort()

The crash was being caused by trying to present the error view controller
while the payment request's view controller was being dismissed. This CL
fixes that by waiting for the payment request's main view controller to get
dismissed before presenting the error view controller.

Bug: 825827
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: I0a481c74d7fe654547c83ea3773d3533f1f8df11
Reviewed-on: https://chromium-review.googlesource.com/986467
Commit-Queue: Moe Ahmadi <mahmadi@chromium.org>
Reviewed-by: edchin <edchin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#548182}

ios/chrome/browser/ui/payments/payment_request_coordinator.h

@@ -17,7 +17,6 @@
 #import "ios/chrome/browser/ui/payments/credit_card_edit_coordinator.h"
 #import "ios/chrome/browser/ui/payments/payment_items_display_coordinator.h"
 #import "ios/chrome/browser/ui/payments/payment_method_selection_coordinator.h"
-#include "ios/chrome/browser/ui/payments/payment_request_error_coordinator.h"
 #import "ios/chrome/browser/ui/payments/payment_request_view_controller.h"
 #import "ios/chrome/browser/ui/payments/shipping_address_selection_coordinator.h"
 #import "ios/chrome/browser/ui/payments/shipping_option_selection_coordinator.h"
@@ -64,10 +63,6 @@ class PaymentShippingOption;
           didSelectShippingOption:
               (const payments::PaymentShippingOption&)shippingOption;
 
-// Notifies the delegate that the presenting view controller is dismissed.
-- (void)paymentRequestCoordinatorDidStop:
-    (PaymentRequestCoordinator*)coordinator;
-
 @end
 
 // Coordinator responsible for creating and presenting the PaymentRequest view
@@ -80,7 +75,6 @@ class PaymentShippingOption;
                         CreditCardEditCoordinatorDelegate,
                         PaymentItemsDisplayCoordinatorDelegate,
                         PaymentMethodSelectionCoordinatorDelegate,
-                        PaymentRequestErrorCoordinatorDelegate,
                         PaymentRequestViewControllerDelegate,
                         ShippingAddressSelectionCoordinatorDelegate,
                         ShippingOptionSelectionCoordinatorDelegate>
@@ -133,8 +127,8 @@ requestFullCreditCard:(const autofill::CreditCard&)card
 // Updates the payment details of the PaymentRequest and updates the UI.
 - (void)updatePaymentDetails:(payments::PaymentDetails)paymentDetails;
 
-// Displays an error message. Invokes |callback| when the message is dismissed.
-- (void)displayErrorWithCallback:(ProceduralBlock)callback;
+// Dismisses the payment request UI. Invokes |completion| when UI is dismissed.
+- (void)stopWithCompletion:(ProceduralBlock)completion;
 
 @end
 

ios/chrome/browser/ui/payments/payment_request_coordinator.mm

@@ -36,13 +36,6 @@ const NSTimeInterval kUpdatePaymentSummaryItemIntervalSeconds = 10.0;
 
 @interface PaymentRequestCoordinator ()
 
-// A weak reference to self used in -stop. -stop is run twice, once by the
-// coordinator that manages the lifetime of this coordinator and once in
-// ChromeCoordinator's -dealloc. It is not possible to create a weak reference
-// to self in the process of deallocation. The second time -stop is called in
-// -dealloc this weak reference is expected to be nil.
-@property(nonatomic, weak) PaymentRequestCoordinator* weakSelf;
-
 // Updates the current total amount and asks the view controller to update the
 // Payment Summary item so that the changes in total amount are reflected.
 - (void)updatePaymentSummaryItem;
@@ -56,7 +49,6 @@ const NSTimeInterval kUpdatePaymentSummaryItemIntervalSeconds = 10.0;
   ContactInfoEditCoordinator* _contactInfoEditCoordinator;
   ContactInfoSelectionCoordinator* _contactInfoSelectionCoordinator;
   PaymentRequestViewController* _viewController;
-  PaymentRequestErrorCoordinator* _errorCoordinator;
   PaymentItemsDisplayCoordinator* _itemsDisplayCoordinator;
   ShippingAddressSelectionCoordinator* _shippingAddressSelectionCoordinator;
   ShippingOptionSelectionCoordinator* _shippingOptionSelectionCoordinator;
@@ -88,11 +80,8 @@ const NSTimeInterval kUpdatePaymentSummaryItemIntervalSeconds = 10.0;
 @synthesize pending = _pending;
 @synthesize cancellable = _cancellable;
 @synthesize delegate = _delegate;
-@synthesize weakSelf = _weakSelf;
 
 - (void)start {
-  _weakSelf = self;
-
   _currentTotal =
       std::make_unique<payments::PaymentItem>(self.paymentRequest->GetTotal(
           self.paymentRequest->selected_payment_method()));
@@ -124,15 +113,15 @@ const NSTimeInterval kUpdatePaymentSummaryItemIntervalSeconds = 10.0;
 }
 
 - (void)stop {
+  [self stopWithCompletion:nil];
+}
+
+- (void)stopWithCompletion:(ProceduralBlock)completion {
   [_updatePaymentSummaryItemTimer invalidate];
 
-  __weak PaymentRequestCoordinator* weakSelf = self.weakSelf;
-  ProceduralBlock callback = ^() {
-    [weakSelf.delegate paymentRequestCoordinatorDidStop:weakSelf];
-  };
   [[_navigationController presentingViewController]
       dismissViewControllerAnimated:YES
-                         completion:callback];
+                         completion:completion];
 
   [_addressEditCoordinator stop];
   _addressEditCoordinator = nil;
@@ -150,8 +139,6 @@ const NSTimeInterval kUpdatePaymentSummaryItemIntervalSeconds = 10.0;
   _shippingOptionSelectionCoordinator = nil;
   [_methodSelectionCoordinator stop];
   _methodSelectionCoordinator = nil;
-  [_errorCoordinator stop];
-  _errorCoordinator = nil;
   _viewController = nil;
   _navigationController = nil;
 }
@@ -229,15 +216,6 @@ requestFullCreditCard:(const autofill::CreditCard&)card
   _addressEditCoordinator = nil;
 }
 
-- (void)displayErrorWithCallback:(ProceduralBlock)callback {
-  _errorCoordinator = [[PaymentRequestErrorCoordinator alloc]
-      initWithBaseViewController:_navigationController];
-  [_errorCoordinator setCallback:callback];
-  [_errorCoordinator setDelegate:self];
-
-  [_errorCoordinator start];
-}
-
 #pragma mark - PaymentRequestViewControllerDelegate
 
 - (void)paymentRequestViewControllerDidCancel:
@@ -343,19 +321,6 @@ requestFullCreditCard:(const autofill::CreditCard&)card
   [_methodSelectionCoordinator start];
 }
 
-#pragma mark - PaymentRequestErrorCoordinatorDelegate
-
-- (void)paymentRequestErrorCoordinatorDidDismiss:
-    (PaymentRequestErrorCoordinator*)coordinator {
-  ProceduralBlock callback = coordinator.callback;
-
-  [_errorCoordinator stop];
-  _errorCoordinator = nil;
-
-  if (callback)
-    callback();
-}
-
 #pragma mark - PaymentItemsDisplayCoordinatorDelegate
 
 - (void)paymentItemsDisplayCoordinatorDidReturn:

ios/chrome/browser/ui/payments/payment_request_coordinator_unittest.mm

@@ -106,18 +106,6 @@ TEST_F(PaymentRequestCoordinatorTest, StartAndStop) {
   [coordinator setPaymentRequest:payment_request()];
   [coordinator setBrowserState:browser_state()];
 
-  // Mock the coordinator delegate.
-  id check_block = ^BOOL(id value) {
-    EXPECT_TRUE(value == coordinator);
-    return YES;
-  };
-  id delegate = [OCMockObject
-      mockForProtocol:@protocol(PaymentRequestCoordinatorDelegate)];
-  [[delegate expect]
-      paymentRequestCoordinatorDidStop:[OCMArg checkWithBlock:check_block]];
-
-  [coordinator setDelegate:delegate];
-
   [coordinator start];
   // Spin the run loop to trigger the animation.
   base::test::ios::SpinRunLoopWithMaxDelay(base::TimeDelta::FromSecondsD(1));
@@ -137,8 +125,6 @@ TEST_F(PaymentRequestCoordinatorTest, StartAndStop) {
     return !base_view_controller.presentedViewController;
   });
   EXPECT_EQ(nil, base_view_controller.presentedViewController);
-
-  EXPECT_OCMOCK_VERIFY(delegate);
 }
 
 // Tests that calling the ShippingAddressSelectionCoordinator delegate method