[Payment Request] Adds a JourneyLogger instance to the PaymentRequest

[Payment Request] Adds a JourneyLogger instance to the PaymentRequest

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

[Moe, 2017-07-06 04:03:17]

The CQ bit was checked by mahmadi@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-07-06 04:03:26]

Dry run: 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-07-06 04:34:31]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-07-06 04:34:32]

Dry run: Try jobs failed on following builders:
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)

[Moe, 2017-07-06 13:43:36]

The CQ bit was checked by mahmadi@chromium.org to run a CQ dry run

[Moe, 2017-07-06 13:43:46]

Patchset #1 (id:1) has been deleted

[commit-bot: I haz the power, 2017-07-06 13:43:51]

Dry run: CQ is trying da patch.

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

[Moe, 2017-07-06 14:04:05]

Patchset #1 (id:20001) has been deleted

[Moe, 2017-07-06 14:04:23]

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

[Moe, 2017-07-06 14:04:24]

Hi Rouslan, Please take a look.

[please use gerrit instead, 2017-07-06 14:52:25]

https://codereview.chromium.org/2970233002/diff/40001/ios/chrome/browser/paym...
File ios/chrome/browser/payments/payment_request.h (right):

https://codereview.chromium.org/2970233002/diff/40001/ios/chrome/browser/paym...
ios/chrome/browser/payments/payment_request.h:70: ios::ChromeBrowserState*
browser_state_,
Remove the trailing _ from |browser_state_| on this line.

https://codereview.chromium.org/2970233002/diff/40001/ios/chrome/browser/paym...
File ios/chrome/browser/payments/payment_request_unittest.mm (right):

https://codereview.chromium.org/2970233002/diff/40001/ios/chrome/browser/paym...
ios/chrome/browser/payments/payment_request_unittest.mm:9: #include
"base/memory/ptr_util.h"
Not used, I believe.

https://codereview.chromium.org/2970233002/diff/40001/ios/chrome/browser/paym...
ios/chrome/browser/payments/payment_request_unittest.mm:51: PaymentRequestTest()
{
PaymentRequestTest()
    : chrome_browser_state_(TestChromeBrowserState::Builder().Build()) {}

https://codereview.chromium.org/2970233002/diff/40001/ios/chrome/browser/ui/p...
File ios/chrome/browser/ui/payments/address_edit_coordinator_unittest.mm
(right):

https://codereview.chromium.org/2970233002/diff/40001/ios/chrome/browser/ui/p...
ios/chrome/browser/ui/payments/address_edit_coordinator_unittest.mm:113: :
pref_service_(autofill::test::PrefServiceForTesting()) {
chrome_browser_state_(TestChromeBrowserState::Builder().Build())

https://codereview.chromium.org/2970233002/diff/40001/ios/chrome/browser/ui/p...
ios/chrome/browser/ui/payments/address_edit_coordinator_unittest.mm:145:
std::unique_ptr<TestChromeBrowserState> chrome_browser_state_;
payment_request_ uses chrome_browser_state_, so let's put chrome_browser_state_
before payment_request_. C++ destruction order bottom-up, so
chrome_browser_state_ will be deleted before payment_request_, which violates
your assumptions, I believe.

https://codereview.chromium.org/2970233002/diff/40001/ios/chrome/browser/ui/p...
File
ios/chrome/browser/ui/payments/billing_address_selection_coordinator_unittest.mm
(right):

https://codereview.chromium.org/2970233002/diff/40001/ios/chrome/browser/ui/p...
ios/chrome/browser/ui/payments/billing_address_selection_coordinator_unittest.mm:93:
std::unique_ptr<TestChromeBrowserState> chrome_browser_state_;
Ditto

https://codereview.chromium.org/2970233002/diff/40001/ios/chrome/browser/ui/p...
File ios/chrome/browser/ui/payments/contact_info_edit_coordinator_unittest.mm
(right):

https://codereview.chromium.org/2970233002/diff/40001/ios/chrome/browser/ui/p...
ios/chrome/browser/ui/payments/contact_info_edit_coordinator_unittest.mm:134:
std::unique_ptr<TestChromeBrowserState> chrome_browser_state_;
Ditto

https://codereview.chromium.org/2970233002/diff/40001/ios/chrome/browser/ui/p...
File ios/chrome/browser/ui/payments/contact_info_edit_mediator_unittest.mm
(right):

https://codereview.chromium.org/2970233002/diff/40001/ios/chrome/browser/ui/p...
ios/chrome/browser/ui/payments/contact_info_edit_mediator_unittest.mm:47:
std::unique_ptr<TestChromeBrowserState> chrome_browser_state_;
Ditto

https://codereview.chromium.org/2970233002/diff/40001/ios/chrome/browser/ui/p...
File
ios/chrome/browser/ui/payments/contact_info_selection_coordinator_unittest.mm
(right):

https://codereview.chromium.org/2970233002/diff/40001/ios/chrome/browser/ui/p...
ios/chrome/browser/ui/payments/contact_info_selection_coordinator_unittest.mm:53:
std::unique_ptr<TestChromeBrowserState> chrome_browser_state_;
Ditto

https://codereview.chromium.org/2970233002/diff/40001/ios/chrome/browser/ui/p...
File ios/chrome/browser/ui/payments/contact_info_selection_mediator_unittest.mm
(right):

https://codereview.chromium.org/2970233002/diff/40001/ios/chrome/browser/ui/p...
ios/chrome/browser/ui/payments/contact_info_selection_mediator_unittest.mm:69:
std::unique_ptr<TestChromeBrowserState> chrome_browser_state_;
Ditto

https://codereview.chromium.org/2970233002/diff/40001/ios/chrome/browser/ui/p...
File ios/chrome/browser/ui/payments/credit_card_edit_coordinator_unittest.mm
(right):

https://codereview.chromium.org/2970233002/diff/40001/ios/chrome/browser/ui/p...
ios/chrome/browser/ui/payments/credit_card_edit_coordinator_unittest.mm:132:
std::unique_ptr<TestChromeBrowserState> chrome_browser_state_;
Ditto

https://codereview.chromium.org/2970233002/diff/40001/ios/chrome/browser/ui/p...
File
ios/chrome/browser/ui/payments/payment_items_display_coordinator_unittest.mm
(right):

https://codereview.chromium.org/2970233002/diff/40001/ios/chrome/browser/ui/p...
ios/chrome/browser/ui/payments/payment_items_display_coordinator_unittest.mm:46:
std::unique_ptr<TestChromeBrowserState> chrome_browser_state_;
Ditto

https://codereview.chromium.org/2970233002/diff/40001/ios/chrome/browser/ui/p...
File
ios/chrome/browser/ui/payments/payment_items_display_view_controller_unittest.mm
(right):

https://codereview.chromium.org/2970233002/diff/40001/ios/chrome/browser/ui/p...
ios/chrome/browser/ui/payments/payment_items_display_view_controller_unittest.mm:74:
std::unique_ptr<TestChromeBrowserState> chrome_browser_state_;
Ditto

https://codereview.chromium.org/2970233002/diff/40001/ios/chrome/browser/ui/p...
File
ios/chrome/browser/ui/payments/payment_method_selection_coordinator_unittest.mm
(right):

https://codereview.chromium.org/2970233002/diff/40001/ios/chrome/browser/ui/p...
ios/chrome/browser/ui/payments/payment_method_selection_coordinator_unittest.mm:64:
std::unique_ptr<TestChromeBrowserState> chrome_browser_state_;
Ditto

https://codereview.chromium.org/2970233002/diff/40001/ios/chrome/browser/ui/p...
File ios/chrome/browser/ui/payments/payment_request_view_controller_unittest.mm
(right):

https://codereview.chromium.org/2970233002/diff/40001/ios/chrome/browser/ui/p...
ios/chrome/browser/ui/payments/payment_request_view_controller_unittest.mm:180:
std::unique_ptr<TestChromeBrowserState> chrome_browser_state_;
And so on..

[Moe, 2017-07-07 02:24:55]

Thanks for the feedback. PTAL?

https://codereview.chromium.org/2970233002/diff/40001/ios/chrome/browser/paym...
File ios/chrome/browser/payments/payment_request.h (right):

https://codereview.chromium.org/2970233002/diff/40001/ios/chrome/browser/paym...
ios/chrome/browser/payments/payment_request.h:70: ios::ChromeBrowserState*
browser_state_,
On 2017/07/06 at 14:52:24, rouslan wrote:
> Remove the trailing _ from |browser_state_| on this line.

Done.

https://codereview.chromium.org/2970233002/diff/40001/ios/chrome/browser/paym...
File ios/chrome/browser/payments/payment_request_unittest.mm (right):

https://codereview.chromium.org/2970233002/diff/40001/ios/chrome/browser/paym...
ios/chrome/browser/payments/payment_request_unittest.mm:9: #include
"base/memory/ptr_util.h"
On 2017/07/06 at 14:52:24, rouslan wrote:
> Not used, I believe.

Done.

https://codereview.chromium.org/2970233002/diff/40001/ios/chrome/browser/paym...
ios/chrome/browser/payments/payment_request_unittest.mm:51: PaymentRequestTest()
{
On 2017/07/06 at 14:52:24, rouslan wrote:
> PaymentRequestTest()
>     : chrome_browser_state_(TestChromeBrowserState::Builder().Build()) {}

Done.

https://codereview.chromium.org/2970233002/diff/40001/ios/chrome/browser/ui/p...
File ios/chrome/browser/ui/payments/address_edit_coordinator_unittest.mm
(right):

https://codereview.chromium.org/2970233002/diff/40001/ios/chrome/browser/ui/p...
ios/chrome/browser/ui/payments/address_edit_coordinator_unittest.mm:113: :
pref_service_(autofill::test::PrefServiceForTesting()) {
On 2017/07/06 at 14:52:24, rouslan wrote:
> chrome_browser_state_(TestChromeBrowserState::Builder().Build())

Done.

https://codereview.chromium.org/2970233002/diff/40001/ios/chrome/browser/ui/p...
ios/chrome/browser/ui/payments/address_edit_coordinator_unittest.mm:145:
std::unique_ptr<TestChromeBrowserState> chrome_browser_state_;
On 2017/07/06 at 14:52:24, rouslan wrote:
> payment_request_ uses chrome_browser_state_, so let's put
chrome_browser_state_ before payment_request_. C++ destruction order bottom-up,
so chrome_browser_state_ will be deleted before payment_request_, which violates
your assumptions, I believe.

You have a point. Done.

https://codereview.chromium.org/2970233002/diff/40001/ios/chrome/browser/ui/p...
File
ios/chrome/browser/ui/payments/billing_address_selection_coordinator_unittest.mm
(right):

https://codereview.chromium.org/2970233002/diff/40001/ios/chrome/browser/ui/p...
ios/chrome/browser/ui/payments/billing_address_selection_coordinator_unittest.mm:93:
std::unique_ptr<TestChromeBrowserState> chrome_browser_state_;
On 2017/07/06 at 14:52:24, rouslan wrote:
> Ditto

Done.

https://codereview.chromium.org/2970233002/diff/40001/ios/chrome/browser/ui/p...
File ios/chrome/browser/ui/payments/contact_info_edit_coordinator_unittest.mm
(right):

https://codereview.chromium.org/2970233002/diff/40001/ios/chrome/browser/ui/p...
ios/chrome/browser/ui/payments/contact_info_edit_coordinator_unittest.mm:134:
std::unique_ptr<TestChromeBrowserState> chrome_browser_state_;
On 2017/07/06 at 14:52:25, rouslan wrote:
> Ditto

Done.

https://codereview.chromium.org/2970233002/diff/40001/ios/chrome/browser/ui/p...
File ios/chrome/browser/ui/payments/contact_info_edit_mediator_unittest.mm
(right):

https://codereview.chromium.org/2970233002/diff/40001/ios/chrome/browser/ui/p...
ios/chrome/browser/ui/payments/contact_info_edit_mediator_unittest.mm:47:
std::unique_ptr<TestChromeBrowserState> chrome_browser_state_;
On 2017/07/06 at 14:52:25, rouslan wrote:
> Ditto

Done.

https://codereview.chromium.org/2970233002/diff/40001/ios/chrome/browser/ui/p...
File
ios/chrome/browser/ui/payments/contact_info_selection_coordinator_unittest.mm
(right):

https://codereview.chromium.org/2970233002/diff/40001/ios/chrome/browser/ui/p...
ios/chrome/browser/ui/payments/contact_info_selection_coordinator_unittest.mm:53:
std::unique_ptr<TestChromeBrowserState> chrome_browser_state_;
On 2017/07/06 at 14:52:25, rouslan wrote:
> Ditto

Done.

https://codereview.chromium.org/2970233002/diff/40001/ios/chrome/browser/ui/p...
File ios/chrome/browser/ui/payments/contact_info_selection_mediator_unittest.mm
(right):

https://codereview.chromium.org/2970233002/diff/40001/ios/chrome/browser/ui/p...
ios/chrome/browser/ui/payments/contact_info_selection_mediator_unittest.mm:69:
std::unique_ptr<TestChromeBrowserState> chrome_browser_state_;
On 2017/07/06 at 14:52:25, rouslan wrote:
> Ditto

Done.

https://codereview.chromium.org/2970233002/diff/40001/ios/chrome/browser/ui/p...
File ios/chrome/browser/ui/payments/credit_card_edit_coordinator_unittest.mm
(right):

https://codereview.chromium.org/2970233002/diff/40001/ios/chrome/browser/ui/p...
ios/chrome/browser/ui/payments/credit_card_edit_coordinator_unittest.mm:132:
std::unique_ptr<TestChromeBrowserState> chrome_browser_state_;
On 2017/07/06 at 14:52:25, rouslan wrote:
> Ditto

Done.

https://codereview.chromium.org/2970233002/diff/40001/ios/chrome/browser/ui/p...
File
ios/chrome/browser/ui/payments/payment_items_display_coordinator_unittest.mm
(right):

https://codereview.chromium.org/2970233002/diff/40001/ios/chrome/browser/ui/p...
ios/chrome/browser/ui/payments/payment_items_display_coordinator_unittest.mm:46:
std::unique_ptr<TestChromeBrowserState> chrome_browser_state_;
On 2017/07/06 at 14:52:25, rouslan wrote:
> Ditto

Done.

https://codereview.chromium.org/2970233002/diff/40001/ios/chrome/browser/ui/p...
File
ios/chrome/browser/ui/payments/payment_items_display_view_controller_unittest.mm
(right):

https://codereview.chromium.org/2970233002/diff/40001/ios/chrome/browser/ui/p...
ios/chrome/browser/ui/payments/payment_items_display_view_controller_unittest.mm:74:
std::unique_ptr<TestChromeBrowserState> chrome_browser_state_;
On 2017/07/06 at 14:52:25, rouslan wrote:
> Ditto

Done.

https://codereview.chromium.org/2970233002/diff/40001/ios/chrome/browser/ui/p...
File
ios/chrome/browser/ui/payments/payment_method_selection_coordinator_unittest.mm
(right):

https://codereview.chromium.org/2970233002/diff/40001/ios/chrome/browser/ui/p...
ios/chrome/browser/ui/payments/payment_method_selection_coordinator_unittest.mm:64:
std::unique_ptr<TestChromeBrowserState> chrome_browser_state_;
On 2017/07/06 at 14:52:25, rouslan wrote:
> Ditto

Done.

https://codereview.chromium.org/2970233002/diff/40001/ios/chrome/browser/ui/p...
File ios/chrome/browser/ui/payments/payment_request_view_controller_unittest.mm
(right):

https://codereview.chromium.org/2970233002/diff/40001/ios/chrome/browser/ui/p...
ios/chrome/browser/ui/payments/payment_request_view_controller_unittest.mm:180:
std::unique_ptr<TestChromeBrowserState> chrome_browser_state_;
On 2017/07/06 at 14:52:25, rouslan wrote:
> And so on..

Done.

[please use gerrit instead, 2017-07-07 13:44:01]

lgtm

[Moe, 2017-07-07 14:47:12]

The CQ bit was checked by mahmadi@chromium.org

[commit-bot: I haz the power, 2017-07-07 14:47:22]

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-07-07 16:50:07]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1499438832021800,
"parent_rev": "e827db9bb82986fb9e00d28695c9bc60b7ebde0a", "commit_rev":
"eb98fc5f18248d7c2f109d3d12d8e4210e22579a"}

[commit-bot: I haz the power, 2017-07-07 16:50:21]

Description was changed from

==========
[Payment Request] Adds a JourneyLogger instance to the PaymentRequest

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

to

==========
[Payment Request] Adds a JourneyLogger instance to the PaymentRequest

BUG=602666

Review-Url: https://codereview.chromium.org/2970233002
Cr-Commit-Position: refs/heads/master@{#484958}
Committed:
https://chromium.googlesource.com/chromium/src/+/eb98fc5f18248d7c2f109d3d12d8...
==========

[commit-bot: I haz the power, 2017-07-07 16:50:23]

Committed patchset #2 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/eb98fc5f18248d7c2f109d3d12d8...