[Payment Request] Refactors selector view controllers

[Payment Request] Refactors selector view controllers

Replaces the ShippingAddressSelectionViewController with
PaymentRequestSelectorViewController.

BUG=602666
Review-Url: https://codereview.chromium.org/2827453004
Cr-Commit-Position: refs/heads/master@{#465307}
Committed: https://chromium.googlesource.com/chromium/src/+/0cf09d98d5a7190dc90109fd7322e70594fd21da

[Moe, 2017-04-18 13:58:17]

mahmadi@chromium.org changed reviewers:
+ gambard@chromium.org

[Moe, 2017-04-18 13:58:18]

Hi Gauthier, Please take a look at this CL.

There's no functional changes here. Simply the logic has been moved from from
the VC to the coordinator and the mediator.

https://codereview.chromium.org/2827453004/diff/1/ios/chrome/browser/payments...
File ios/chrome/browser/payments/payment_request_selector_view_controller.mm
(right):

https://codereview.chromium.org/2827453004/diff/1/ios/chrome/browser/payments...
ios/chrome/browser/payments/payment_request_selector_view_controller.mm:132:
#pragma mark UICollectionViewDataSource
Hopefully we won't need this once the styler API has baked in.

[gambard, 2017-04-18 14:34:46]

lgtm

https://codereview.chromium.org/2827453004/diff/1/ios/chrome/browser/payments...
File ios/chrome/browser/payments/payment_request_selector_view_controller.mm
(right):

https://codereview.chromium.org/2827453004/diff/1/ios/chrome/browser/payments...
ios/chrome/browser/payments/payment_request_selector_view_controller.mm:132:
#pragma mark UICollectionViewDataSource
On 2017/04/18 13:58:18, moe wrote:
> Hopefully we won't need this once the styler API has baked in.

Acknowledged.

https://codereview.chromium.org/2827453004/diff/1/ios/chrome/browser/payments...
File ios/chrome/browser/payments/shipping_address_selection_coordinator.mm
(right):

https://codereview.chromium.org/2827453004/diff/1/ios/chrome/browser/payments...
ios/chrome/browser/payments/shipping_address_selection_coordinator.mm:51:
[self.mediator setMessage:self.paymentRequest->shipping_options().empty()
s/self.mediator setMessage:/self.mediator.message =

https://codereview.chromium.org/2827453004/diff/1/ios/chrome/browser/payments...
ios/chrome/browser/payments/shipping_address_selection_coordinator.mm:51:
[self.mediator setMessage:self.paymentRequest->shipping_options().empty()
Are you sure you want to set message to something if the shipping_options are
empty?

https://codereview.chromium.org/2827453004/diff/1/ios/chrome/browser/payments...
File ios/chrome/browser/payments/shipping_address_selection_mediator.h (right):

https://codereview.chromium.org/2827453004/diff/1/ios/chrome/browser/payments...
ios/chrome/browser/payments/shipping_address_selection_mediator.h:18:
@property(nonatomic, copy) NSString* message;
s/message/headerLabel (or headerString, but you should indicate this is related
to the header).

https://codereview.chromium.org/2827453004/diff/1/ios/chrome/browser/payments...
File ios/chrome/browser/payments/shipping_address_selection_mediator.mm (right):

https://codereview.chromium.org/2827453004/diff/1/ios/chrome/browser/payments...
ios/chrome/browser/payments/shipping_address_selection_mediator.mm:34:
@property(nonatomic, assign) PaymentRequest* paymentRequest;
s/assign/weak

https://codereview.chromium.org/2827453004/diff/1/ios/chrome/browser/payments...
ios/chrome/browser/payments/shipping_address_selection_mediator.mm:77:
DCHECK(index < self.items.count);
You did not specify in the protocol that |index| should be < self.items.count.
Please add a comment to PaymentRequestSelectorViewControllerDataSource

[Moe, 2017-04-18 17:19:18]

Thank you Gauthier.

https://codereview.chromium.org/2827453004/diff/1/ios/chrome/browser/payments...
File ios/chrome/browser/payments/shipping_address_selection_coordinator.mm
(right):

https://codereview.chromium.org/2827453004/diff/1/ios/chrome/browser/payments...
ios/chrome/browser/payments/shipping_address_selection_coordinator.mm:51:
[self.mediator setMessage:self.paymentRequest->shipping_options().empty()
On 2017/04/18 14:34:46, gambard wrote:
> Are you sure you want to set message to something if the shipping_options are
> empty?

Yes. that's according to the specs and mocks. It's basically says "there are no
shipping options available, why don't you choose a shipping address to see the
available shipping options..."

https://codereview.chromium.org/2827453004/diff/1/ios/chrome/browser/payments...
ios/chrome/browser/payments/shipping_address_selection_coordinator.mm:51:
[self.mediator setMessage:self.paymentRequest->shipping_options().empty()
On 2017/04/18 14:34:46, gambard wrote:
> s/self.mediator setMessage:/self.mediator.message =

Done. Fixed similar cases as well.

https://codereview.chromium.org/2827453004/diff/1/ios/chrome/browser/payments...
File ios/chrome/browser/payments/shipping_address_selection_mediator.h (right):

https://codereview.chromium.org/2827453004/diff/1/ios/chrome/browser/payments...
ios/chrome/browser/payments/shipping_address_selection_mediator.h:18:
@property(nonatomic, copy) NSString* message;
On 2017/04/18 14:34:46, gambard wrote:
> s/message/headerLabel (or headerString, but you should indicate this is
related
> to the header).

Done.

https://codereview.chromium.org/2827453004/diff/1/ios/chrome/browser/payments...
File ios/chrome/browser/payments/shipping_address_selection_mediator.mm (right):

https://codereview.chromium.org/2827453004/diff/1/ios/chrome/browser/payments...
ios/chrome/browser/payments/shipping_address_selection_mediator.mm:34:
@property(nonatomic, assign) PaymentRequest* paymentRequest;
On 2017/04/18 14:34:46, gambard wrote:
> s/assign/weak

PaymentRequest isn't an Objective-C object.

https://codereview.chromium.org/2827453004/diff/1/ios/chrome/browser/payments...
ios/chrome/browser/payments/shipping_address_selection_mediator.mm:77:
DCHECK(index < self.items.count);
On 2017/04/18 14:34:46, gambard wrote:
> You did not specify in the protocol that |index| should be < self.items.count.
> Please add a comment to PaymentRequestSelectorViewControllerDataSource

Done.

[Moe, 2017-04-18 17:19:24]

The CQ bit was checked by mahmadi@chromium.org

[Moe, 2017-04-18 17:19:25]

The patchset sent to the CQ was uploaded after l-g-t-m from gambard@chromium.org
Link to the patchset: https://codereview.chromium.org/2827453004/#ps20001
(title: "Addressed comments")

[commit-bot: I haz the power, 2017-04-18 17:19:48]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-18 18:54:07]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1492535964806040,
"parent_rev": "b38d7e9643b22f3e2b4e2454011c6e235d5ecef5", "commit_rev":
"0cf09d98d5a7190dc90109fd7322e70594fd21da"}

[commit-bot: I haz the power, 2017-04-18 18:54:17]

Description was changed from

==========
[Payment Request] Refactors selector view controllers

Replaces the ShippingAddressSelectionViewController with
PaymentRequestSelectorViewController.

BUG=602666
==========

to

==========
[Payment Request] Refactors selector view controllers

Replaces the ShippingAddressSelectionViewController with
PaymentRequestSelectorViewController.

BUG=602666

Review-Url: https://codereview.chromium.org/2827453004
Cr-Commit-Position: refs/heads/master@{#465307}
Committed:
https://chromium.googlesource.com/chromium/src/+/0cf09d98d5a7190dc90109fd7322...
==========

[commit-bot: I haz the power, 2017-04-18 18:54:18]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/0cf09d98d5a7190dc90109fd7322...