Selected shipping option in payment summary view + shipping option selection view

ios/chrome/browser/payments/shipping_option_selection_coordinator.mm

+// Copyright 2016 The Chromium Authors. All rights reserved.

[lpromero, 2017/01/11 12:51:15]

2017, here and in every file.

[Moe, 2017/01/12 00:06:18]

Done.

+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#import "ios/chrome/browser/payments/shipping_option_selection_coordinator.h"
+
+#import "base/ios/weak_nsobject.h"
+#include "base/mac/scoped_nsobject.h"
+#import "ios/chrome/browser/payments/shipping_option_selection_view_controller.h"
+
+@interface ShippingOptionSelectionCoordinator ()<
+    ShippingOptionSelectionViewControllerDelegate> {
+  base::WeakNSProtocol<id<ShippingOptionSelectionCoordinatorDelegate>>
+      _delegate;
+  base::scoped_nsobject<ShippingOptionSelectionViewController> _viewController;
+}
+
+// Called when the user selects a shipping option. The cell is checked, the
+// UI is locked so that the user can't interact with it, then the delegate is
+// notified. The delay is here to let the user get a visual feedback of the
+// selection before this view disappears.
+- (void)delayedNotifyDelegateOfSelection;
+
+@end
+
+@implementation ShippingOptionSelectionCoordinator
+
+@synthesize shippingOptions = _shippingOptions;
+@synthesize selectedShippingOption = _selectedShippingOption;
+
+- (id<ShippingOptionSelectionCoordinatorDelegate>)delegate {
+  return _delegate.get();
+}
+
+- (void)setDelegate:(id<ShippingOptionSelectionCoordinatorDelegate>)delegate {
+  _delegate.reset(delegate);
+}
+
+- (void)start {
+  _viewController.reset([[ShippingOptionSelectionViewController alloc] init]);
+  [_viewController setShippingOptions:_shippingOptions];
+  [_viewController setSelectedShippingOption:_selectedShippingOption];
+  [_viewController setDelegate:self];
+  [_viewController loadModel];
+
+  DCHECK(self.baseViewController.navigationController);

[lpromero, 2017/01/11 12:51:15]

No action needed: We never need baseViewController for anything else than its
navigationController, right? We should consider having the baseViewController be
the navigationController itself. I will think about it for the future.

[Moe, 2017/01/12 00:06:18]

Acknowledged.

+  [self.baseViewController.navigationController
+      pushViewController:_viewController
+                animated:YES];
+}
+
+- (void)stop {
+  [self.baseViewController.navigationController popViewControllerAnimated:YES];
+  _viewController.reset();
+}
+
+#pragma mark - ShippingOptionSelectionViewControllerDelegate
+
+- (void)shippingOptionSelectionViewController:
+            (ShippingOptionSelectionViewController*)controller
+                       selectedShippingOption:
+                           (web::PaymentShippingOption*)shippingOption {
+  _selectedShippingOption = shippingOption;
+  [self delayedNotifyDelegateOfSelection];
+}
+
+- (void)shippingOptionSelectionViewControllerDidReturn:
+    (ShippingOptionSelectionViewController*)controller {
+  [_delegate shippingOptionSelectionCoordinatorDidReturn:self];
+}
+
+- (void)delayedNotifyDelegateOfSelection {
+  _viewController.get().view.userInteractionEnabled = NO;
+  base::WeakNSObject<ShippingOptionSelectionCoordinator> weakSelf(self);
+  dispatch_after(
+      dispatch_time(DISPATCH_TIME_NOW, (int64_t)(.2 * NSEC_PER_SEC)),
+      dispatch_get_main_queue(), ^{
+        base::scoped_nsobject<ShippingOptionSelectionCoordinator> strongSelf(
+            [weakSelf retain]);
+        // Early return if the coordinator has been deallocated.
+        if (!strongSelf)
+          return;
+
+        _viewController.get().view.userInteractionEnabled = YES;

[Justin Donnelly, 2017/01/10 17:11:53]

Why re-enable interaction here? Wouldn't it be safer to leave it disabled?

[lpromero, 2017/01/11 12:51:15]

Why not, good idea.

[Justin Donnelly, 2017/01/12 00:23:29]

What about this comment?

[Moe, 2017/01/12 14:32:26]

I copied this logic from bookmark_folder_view_controller.mm.
I think it really doesn't matters to if we leave it disabled or reenable it,
since the view gets popped immediately. This in a way looks like any other
coordinator returning to its delegate. One could argue shouldn't we disable user
interaction before returning from any coordinator?

[Justin Donnelly, 2017/01/12 16:05:10]

I'm having trouble parsing your last statement. You're saying you think we
should disable user interaction?

If so, yes, that's what I'm arguing. :) This line is enabling it. You're right
that it doesn't currently matter but in case anything should change in the
future I'm suggesting it would be safest to not enable interaction here.

+        [_delegate shippingOptionSelectionCoordinator:self
+                               selectedShippingOption:_selectedShippingOption];
+      });
+}
+
+@end