PaymentApp: Implement InvokePaymentApp() in browser side.

PaymentApp: Implement InvokePaymentApp() in content/browser side.

This CL is implementing invokePaymentApp() in browser side. During this work,
added FindRegistrationForIdOnly() method to service worker context wrapper and
some codes to embedded worker test helper for testing event dispatching.

BUG=661608
TEST=payment_app_provider_impl_unittest.cc

Review-Url: https://codereview.chromium.org/2610163002
Cr-Commit-Position: refs/heads/master@{#444415}
Committed: https://chromium.googlesource.com/chromium/src/+/2c684a8bef012e3f85338b72d154b3c68867c021

[zino, 2017-01-04 17:36:55]

Description was changed from

==========
PaymentApp: Add InvokePaymentApp() to PaymentAppProvider.

Add the method to the class but it's not implemented yet.

BUG=
==========

to

==========
PaymentApp: Add InvokePaymentApp() to PaymentAppProvider.

Add the method to the class but it's not implemented yet.

BUG=
==========

[zino, 2017-01-05 20:06:46]

Description was changed from

==========
PaymentApp: Add InvokePaymentApp() to PaymentAppProvider.

Add the method to the class but it's not implemented yet.

BUG=
==========

to

==========
PaymentApp: Implement InvokePaymentApp() in browser side.

BUG=
==========

[zino, 2017-01-05 20:07:27]

Description was changed from

==========
PaymentApp: Implement InvokePaymentApp() in browser side.

BUG=
==========

to

==========
PaymentApp: Implement InvokePaymentApp() in browser side.

BUG=661608
==========

[zino, 2017-01-05 20:07:50]

Description was changed from

==========
PaymentApp: Implement InvokePaymentApp() in browser side.

BUG=661608
==========

to

==========
PaymentApp: Implement InvokePaymentApp() in content/browser side.

BUG=661608
==========

[zino, 2017-01-11 19:45:53]

Patchset #5 (id:80001) has been deleted

[zino, 2017-01-11 19:46:01]

Patchset #5 (id:100001) has been deleted

[zino, 2017-01-11 19:56:57]

Description was changed from

==========
PaymentApp: Implement InvokePaymentApp() in content/browser side.

BUG=661608
==========

to

==========
PaymentApp: Implement InvokePaymentApp() in content/browser side.

This CL is implementing invokePaymentApp() in browser side. During this work,
added FindRegistrationForIdOnly() method to service worker context wrapper and
some codes to embedded worker test helper for testing event dispatching.

BUG=661608
TEST=payment_app_provider_impl_unittest.cc
==========

[zino, 2017-01-11 19:56:57]

jinho.bang@samsung.com changed reviewers:
+ nhiroki@chromium.org, rouslan@chromium.org

[zino, 2017-01-11 19:57:10]

jinho.bang@samsung.com changed reviewers:
+ tommyt@opera.com

[zino, 2017-01-11 20:03:49]

The CQ bit was checked by jinho.bang@samsung.com to run a CQ dry run

[commit-bot: I haz the power, 2017-01-11 20:04:41]

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

[zino, 2017-01-11 20:06:44]

rouslan@, tommyt@ for payments
nhiroki@ for service worker

PTAL (Happy belated new year!)

https://codereview.chromium.org/2610163002/diff/140001/content/browser/paymen...
File content/browser/payments/payment_app_content_unittest_base.cc (right):

https://codereview.chromium.org/2610163002/diff/140001/content/browser/paymen...
content/browser/payments/payment_app_content_unittest_base.cc:45: struct
PaymentAppContentUnitTestBase::PaymentAppForWorkerTestHelper
@rouslan,

Should this struct be located to below last_sw_scope_url()?
(I'm not sure if it is a good way for readability. In header file, it was anyway
declared in private field.)

[commit-bot: I haz the power, 2017-01-11 20:20:35]

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

[commit-bot: I haz the power, 2017-01-11 20:20:35]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[nhiroki, 2017-01-12 03:12:38]

Looks good overall from sw point of view. Added some minor comments.

https://codereview.chromium.org/2610163002/diff/140001/content/browser/paymen...
File content/browser/payments/payment_app_provider_impl.cc (right):

https://codereview.chromium.org/2610163002/diff/140001/content/browser/paymen...
content/browser/payments/payment_app_provider_impl.cc:37: const
scoped_refptr<ServiceWorkerVersion>& active_version,
const& for scoped_refptr is no longer encouraged:
http://www.chromium.org/developers/smart-pointer-guidelines#TOC-What-about-pa...

https://codereview.chromium.org/2610163002/diff/140001/content/browser/paymen...
content/browser/payments/payment_app_provider_impl.cc:53: const
scoped_refptr<ServiceWorkerVersion>& active_version) {
ditto.

https://codereview.chromium.org/2610163002/diff/140001/content/browser/paymen...
content/browser/payments/payment_app_provider_impl.cc:137:
base::Bind(&FindRegistrationOnIO, service_worker_context, registration_id,
std::move(service_worker_context)

 or

calling partition->GetServiceWorkerContext() here?

https://codereview.chromium.org/2610163002/diff/140001/content/browser/servic...
File content/browser/service_worker/service_worker_context_wrapper.cc (right):

https://codereview.chromium.org/2610163002/diff/140001/content/browser/servic...
content/browser/service_worker/service_worker_context_wrapper.cc:719: //
FindRegistrationForId() can run the callback synchronously.
ForIdOnly()

https://codereview.chromium.org/2610163002/diff/140001/content/browser/servic...
File content/browser/service_worker/service_worker_metrics.cc (right):

https://codereview.chromium.org/2610163002/diff/140001/content/browser/servic...
content/browser/service_worker/service_worker_metrics.cc:594: case
EventType::PAYMENT_REQUEST:
To identify which event takes long time, having a UMA of event duration is good.
Can you add a TODO comment like this?

TODO(zino): Add event duration UMA for PaymentRequest API.

https://codereview.chromium.org/2610163002/diff/140001/content/browser/servic...
File content/browser/service_worker/service_worker_metrics.h (right):

https://codereview.chromium.org/2610163002/diff/140001/content/browser/servic...
content/browser/service_worker/service_worker_metrics.h:117: PAYMENT_REQUEST =
22,
This enum is used for UMA, so you also need to update
"ServiceWorkerMetrics.EventType" in tools/metrics/histograms/histograms.xml

[tommyt, 2017-01-12 12:53:24]

lgtm

[zino, 2017-01-12 13:49:41]

The CQ bit was checked by jinho.bang@samsung.com to run a CQ dry run

[commit-bot: I haz the power, 2017-01-12 13:49:53]

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-01-12 14:08:19]

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

[commit-bot: I haz the power, 2017-01-12 14:08:20]

Dry run: Try jobs failed on following builders:
  android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)

[please use gerrit instead, 2017-01-12 19:04:00]

https://codereview.chromium.org/2610163002/diff/140001/content/browser/paymen...
File content/browser/payments/payment_app_content_unittest_base.cc (right):

https://codereview.chromium.org/2610163002/diff/140001/content/browser/paymen...
content/browser/payments/payment_app_content_unittest_base.cc:45: struct
PaymentAppContentUnitTestBase::PaymentAppForWorkerTestHelper
On 2017/01/11 20:06:43, zino wrote:
> @rouslan,
> 
> Should this struct be located to below last_sw_scope_url()?
> (I'm not sure if it is a good way for readability. In header file, it was
anyway
> declared in private field.)

Good question. Let's follow the rule of thumb that everything in .cc should be
in the same order as everything .h. I advised in .h that this struct should be
above storage_partition() declaration. Therefore, let's place the struct in .cc
file immediately above the storage_partition() definition.

https://codereview.chromium.org/2610163002/diff/140001/content/browser/paymen...
content/browser/payments/payment_app_content_unittest_base.cc:45: struct
PaymentAppContentUnitTestBase::PaymentAppForWorkerTestHelper
It feels strange to derive a struct from a class. Let's make
PaymentAppForWorkerTestHelper into a class as well.

https://codereview.chromium.org/2610163002/diff/140001/content/browser/paymen...
content/browser/payments/payment_app_content_unittest_base.cc:48: :
EmbeddedWorkerTestHelper(base::FilePath()) {}
Need to override the parent destructor.

https://codereview.chromium.org/2610163002/diff/140001/content/browser/paymen...
content/browser/payments/payment_app_content_unittest_base.cc:48: :
EmbeddedWorkerTestHelper(base::FilePath()) {}
Need to initialize was_dispatched and last_sw_registration_id. Otherwise they
will have random values that will lead to flakes.

https://codereview.chromium.org/2610163002/diff/140001/content/browser/paymen...
content/browser/payments/payment_app_content_unittest_base.cc:77: GURL
last_sw_scope;
private:
DISALLOW_COPY_AND_ASSIGN(PaymentAppForWorkerTestHelper);

https://codereview.chromium.org/2610163002/diff/140001/content/browser/paymen...
content/browser/payments/payment_app_content_unittest_base.cc:192:
worker_helper_->was_dispatched = false;
Initialization should happen in the constructor of the worker_helper_. If you
need to reset this value multiple times, please use the term "reset" instead of
"init".

https://codereview.chromium.org/2610163002/diff/140001/content/browser/paymen...
File content/browser/payments/payment_app_content_unittest_base.h (right):

https://codereview.chromium.org/2610163002/diff/140001/content/browser/paymen...
content/browser/payments/payment_app_content_unittest_base.h:43: void
init_payment_app_invoked() const;
Only simple getters and settings can use hacker_case. This method looks like
neither. Therefore, it should use CamelCase.

https://codereview.chromium.org/2610163002/diff/140001/content/browser/paymen...
content/browser/payments/payment_app_content_unittest_base.h:52: struct
PaymentAppForWorkerTestHelper;
Forward declarations should precede method declarations in the "private"
section.

https://codereview.chromium.org/2610163002/diff/140001/content/browser/paymen...
File content/browser/payments/payment_app_provider_impl_unittest.cc (right):

https://codereview.chromium.org/2610163002/diff/140001/content/browser/paymen...
content/browser/payments/payment_app_provider_impl_unittest.cc:72:
base::RunLoop().RunUntilIdle();
Which call in here is asynchronous?

[zino, 2017-01-13 16:39:35]

The CQ bit was checked by jinho.bang@samsung.com to run a CQ dry run

[commit-bot: I haz the power, 2017-01-13 16:39:56]

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

[zino, 2017-01-13 16:41:11]

nhiroki@, rouslan@

I addressed all your comments.

Thank you for your review.

PTAL

https://codereview.chromium.org/2610163002/diff/140001/content/browser/paymen...
File content/browser/payments/payment_app_content_unittest_base.cc (right):

https://codereview.chromium.org/2610163002/diff/140001/content/browser/paymen...
content/browser/payments/payment_app_content_unittest_base.cc:45: struct
PaymentAppContentUnitTestBase::PaymentAppForWorkerTestHelper
On 2017/01/12 19:04:00, rouslan wrote:
> It feels strange to derive a struct from a class. Let's make
> PaymentAppForWorkerTestHelper into a class as well.

Done.

https://codereview.chromium.org/2610163002/diff/140001/content/browser/paymen...
content/browser/payments/payment_app_content_unittest_base.cc:48: :
EmbeddedWorkerTestHelper(base::FilePath()) {}
On 2017/01/12 19:04:00, rouslan wrote:
> Need to initialize was_dispatched and last_sw_registration_id. Otherwise they
> will have random values that will lead to flakes.

Done.

https://codereview.chromium.org/2610163002/diff/140001/content/browser/paymen...
content/browser/payments/payment_app_content_unittest_base.cc:77: GURL
last_sw_scope;
On 2017/01/12 19:04:00, rouslan wrote:
> private:
> DISALLOW_COPY_AND_ASSIGN(PaymentAppForWorkerTestHelper);

Done.

https://codereview.chromium.org/2610163002/diff/140001/content/browser/paymen...
content/browser/payments/payment_app_content_unittest_base.cc:192:
worker_helper_->was_dispatched = false;
On 2017/01/12 19:04:00, rouslan wrote:
> Initialization should happen in the constructor of the worker_helper_. If you
> need to reset this value multiple times, please use the term "reset" instead
of
> "init".

Done.

https://codereview.chromium.org/2610163002/diff/140001/content/browser/paymen...
File content/browser/payments/payment_app_content_unittest_base.h (right):

https://codereview.chromium.org/2610163002/diff/140001/content/browser/paymen...
content/browser/payments/payment_app_content_unittest_base.h:43: void
init_payment_app_invoked() const;
On 2017/01/12 19:04:00, rouslan wrote:
> Only simple getters and settings can use hacker_case. This method looks like
> neither. Therefore, it should use CamelCase.

Done.

https://codereview.chromium.org/2610163002/diff/140001/content/browser/paymen...
content/browser/payments/payment_app_content_unittest_base.h:52: struct
PaymentAppForWorkerTestHelper;
On 2017/01/12 19:04:00, rouslan wrote:
> Forward declarations should precede method declarations in the "private"
> section.

Done.

https://codereview.chromium.org/2610163002/diff/140001/content/browser/paymen...
File content/browser/payments/payment_app_provider_impl.cc (right):

https://codereview.chromium.org/2610163002/diff/140001/content/browser/paymen...
content/browser/payments/payment_app_provider_impl.cc:37: const
scoped_refptr<ServiceWorkerVersion>& active_version,
On 2017/01/12 03:12:37, nhiroki (OOO until Jan 17) wrote:
> const& for scoped_refptr is no longer encouraged:
>
http://www.chromium.org/developers/smart-pointer-guidelines#TOC-What-about-pa...

Done.

https://codereview.chromium.org/2610163002/diff/140001/content/browser/paymen...
content/browser/payments/payment_app_provider_impl.cc:53: const
scoped_refptr<ServiceWorkerVersion>& active_version) {
On 2017/01/12 03:12:37, nhiroki (OOO until Jan 17) wrote:
> ditto.

Done.

https://codereview.chromium.org/2610163002/diff/140001/content/browser/paymen...
content/browser/payments/payment_app_provider_impl.cc:137:
base::Bind(&FindRegistrationOnIO, service_worker_context, registration_id,
On 2017/01/12 03:12:37, nhiroki (OOO until Jan 17) wrote:
> std::move(service_worker_context)
> 
>  or
> 
> calling partition->GetServiceWorkerContext() here?

Done.

https://codereview.chromium.org/2610163002/diff/140001/content/browser/paymen...
File content/browser/payments/payment_app_provider_impl_unittest.cc (right):

https://codereview.chromium.org/2610163002/diff/140001/content/browser/paymen...
content/browser/payments/payment_app_provider_impl_unittest.cc:72:
base::RunLoop().RunUntilIdle();
On 2017/01/12 19:04:00, rouslan wrote:
> Which call in here is asynchronous?

It's unnecessary. It's my mistake.

Done.

https://codereview.chromium.org/2610163002/diff/140001/content/browser/servic...
File content/browser/service_worker/service_worker_context_wrapper.cc (right):

https://codereview.chromium.org/2610163002/diff/140001/content/browser/servic...
content/browser/service_worker/service_worker_context_wrapper.cc:719: //
FindRegistrationForId() can run the callback synchronously.
On 2017/01/12 03:12:37, nhiroki (OOO until Jan 17) wrote:
> ForIdOnly()

Done.

https://codereview.chromium.org/2610163002/diff/140001/content/browser/servic...
File content/browser/service_worker/service_worker_metrics.cc (right):

https://codereview.chromium.org/2610163002/diff/140001/content/browser/servic...
content/browser/service_worker/service_worker_metrics.cc:594: case
EventType::PAYMENT_REQUEST:
On 2017/01/12 03:12:37, nhiroki (OOO until Jan 17) wrote:
> To identify which event takes long time, having a UMA of event duration is
good.
> Can you add a TODO comment like this?
> 
> TODO(zino): Add event duration UMA for PaymentRequest API.

I just updated the C++ codes and histograms.xml.

https://codereview.chromium.org/2610163002/diff/140001/content/browser/servic...
File content/browser/service_worker/service_worker_metrics.h (right):

https://codereview.chromium.org/2610163002/diff/140001/content/browser/servic...
content/browser/service_worker/service_worker_metrics.h:117: PAYMENT_REQUEST =
22,
On 2017/01/12 03:12:38, nhiroki (OOO until Jan 17) wrote:
> This enum is used for UMA, so you also need to update
> "ServiceWorkerMetrics.EventType" in tools/metrics/histograms/histograms.xml

Done.

[commit-bot: I haz the power, 2017-01-13 16:56:38]

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

[commit-bot: I haz the power, 2017-01-13 16:56:39]

Dry run: Try jobs failed on following builders:
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[zino, 2017-01-13 17:07:52]

The CQ bit was checked by jinho.bang@samsung.com to run a CQ dry run

[commit-bot: I haz the power, 2017-01-13 17:08:19]

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-01-13 17:38:01]

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

[commit-bot: I haz the power, 2017-01-13 17:38:02]

Dry run: Try jobs failed on following builders:
  android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)

[zino, 2017-01-14 17:47:19]

The CQ bit was checked by jinho.bang@samsung.com to run a CQ dry run

[commit-bot: I haz the power, 2017-01-14 17:47:31]

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-01-14 17:55:19]

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

[commit-bot: I haz the power, 2017-01-14 17:55:20]

Dry run: Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)

[zino, 2017-01-14 18:09:22]

The CQ bit was checked by jinho.bang@samsung.com to run a CQ dry run

[commit-bot: I haz the power, 2017-01-14 18:09:34]

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-01-14 19:35:49]

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

[commit-bot: I haz the power, 2017-01-14 19:35:49]

Dry run: This issue passed the CQ dry run.

[zino, 2017-01-16 15:01:09]

Gentle ping. Reviewers.

https://codereview.chromium.org/2610163002/diff/140001/content/browser/paymen...
File content/browser/payments/payment_app_content_unittest_base.cc (right):

https://codereview.chromium.org/2610163002/diff/140001/content/browser/paymen...
content/browser/payments/payment_app_content_unittest_base.cc:45: struct
PaymentAppContentUnitTestBase::PaymentAppForWorkerTestHelper
On 2017/01/12 19:04:00, rouslan wrote:
> On 2017/01/11 20:06:43, zino wrote:
> > @rouslan,
> > 
> > Should this struct be located to below last_sw_scope_url()?
> > (I'm not sure if it is a good way for readability. In header file, it was
> anyway
> > declared in private field.)
> 
> Good question. Let's follow the rule of thumb that everything in .cc should be
> in the same order as everything .h. I advised in .h that this struct should be
> above storage_partition() declaration. Therefore, let's place the struct in
.cc
> file immediately above the storage_partition() definition.

I tried it but I couldn't because it is used in PaymentAppUnitTestBase()
constructor.

[please use gerrit instead, 2017-01-16 16:46:15]

lgtm % nit

https://codereview.chromium.org/2610163002/diff/240001/content/browser/paymen...
File content/browser/payments/payment_app_provider_impl_unittest.cc (right):

https://codereview.chromium.org/2610163002/diff/240001/content/browser/paymen...
content/browser/payments/payment_app_provider_impl_unittest.cc:137:
ResetPaymentAppInvoked();
nit: This call is not necessary now that your initialize the state on
construction.

[zino, 2017-01-17 12:59:06]

The CQ bit was checked by jinho.bang@samsung.com to run a CQ dry run

[commit-bot: I haz the power, 2017-01-17 12:59:19]

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-01-17 14:49:29]

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

[commit-bot: I haz the power, 2017-01-17 14:49:30]

Dry run: This issue passed the CQ dry run.

[nhiroki, 2017-01-18 03:47:39]

LGTM with nits

https://codereview.chromium.org/2610163002/diff/260001/content/browser/paymen...
File content/browser/payments/payment_app_content_unittest_base.cc (right):

https://codereview.chromium.org/2610163002/diff/260001/content/browser/paymen...
content/browser/payments/payment_app_content_unittest_base.cc:51:
last_sw_registration_id(-1) {}
Can you replace -1 with kInvalidServiceWorkerRegistrationId?
https://chromium.googlesource.com/chromium/src/+/756089ad4639bb917faac4b2f6a8...

https://codereview.chromium.org/2610163002/diff/260001/content/browser/paymen...
content/browser/payments/payment_app_content_unittest_base.cc:60:
mojom::ServiceWorkerEventDispatcherRequest request) override {
This indent is broken.

(We might want to have a presubmit check for code format like this:
https://cs.chromium.org/chromium/src/content/browser/service_worker/PRESUBMIT.py)

https://codereview.chromium.org/2610163002/diff/260001/content/browser/paymen...
content/browser/payments/payment_app_content_unittest_base.cc:83: GURL
last_sw_scope;
These fields need "_" suffix.

https://codereview.chromium.org/2610163002/diff/260001/tools/metrics/histogra...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/2610163002/diff/260001/tools/metrics/histogra...
tools/metrics/histograms/histograms.xml:59752: +    Worker and receiving a
message that it finished handling the event. Incudes
Incudes -> Includes ?

https://codereview.chromium.org/2610163002/diff/260001/tools/metrics/histogra...
tools/metrics/histograms/histograms.xml:103691: +  <int value="21"
label="EXTERNAL_REQUEST"/>
Thank you for adding this :)

[zino, 2017-01-18 15:56:07]

Thank you for review.

https://codereview.chromium.org/2610163002/diff/260001/content/browser/paymen...
File content/browser/payments/payment_app_content_unittest_base.cc (right):

https://codereview.chromium.org/2610163002/diff/260001/content/browser/paymen...
content/browser/payments/payment_app_content_unittest_base.cc:51:
last_sw_registration_id(-1) {}
On 2017/01/18 03:47:39, nhiroki wrote:
> Can you replace -1 with kInvalidServiceWorkerRegistrationId?
>
https://chromium.googlesource.com/chromium/src/+/756089ad4639bb917faac4b2f6a8...

Done.

https://codereview.chromium.org/2610163002/diff/260001/content/browser/paymen...
content/browser/payments/payment_app_content_unittest_base.cc:60:
mojom::ServiceWorkerEventDispatcherRequest request) override {
On 2017/01/18 03:47:39, nhiroki wrote:
> This indent is broken.
> 
> (We might want to have a presubmit check for code format like this:
>
https://cs.chromium.org/chromium/src/content/browser/service_worker/PRESUBMIT.py)

Done.

https://codereview.chromium.org/2610163002/diff/260001/content/browser/paymen...
content/browser/payments/payment_app_content_unittest_base.cc:83: GURL
last_sw_scope;
On 2017/01/18 03:47:39, nhiroki wrote:
> These fields need "_" suffix.

Done.

https://codereview.chromium.org/2610163002/diff/260001/tools/metrics/histogra...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/2610163002/diff/260001/tools/metrics/histogra...
tools/metrics/histograms/histograms.xml:59752: +    Worker and receiving a
message that it finished handling the event. Incudes
On 2017/01/18 03:47:39, nhiroki wrote:
> Incudes -> Includes ?

Done.

https://codereview.chromium.org/2610163002/diff/260001/tools/metrics/histogra...
tools/metrics/histograms/histograms.xml:103691: +  <int value="21"
label="EXTERNAL_REQUEST"/>
On 2017/01/18 03:47:39, nhiroki wrote:
> Thank you for adding this :)

Thank you! :)

[zino, 2017-01-18 15:56:16]

The CQ bit was checked by jinho.bang@samsung.com

[zino, 2017-01-18 15:56:16]

The patchset sent to the CQ was uploaded after l-g-t-m from tommyt@opera.com,
rouslan@chromium.org, nhiroki@chromium.org
Link to the patchset: https://codereview.chromium.org/2610163002/#ps280001
(title: "nhiroki's comments")

[commit-bot: I haz the power, 2017-01-18 15:56:26]

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-01-18 17:54:42]

CQ is committing da patch.

Bot data: {"patchset_id": 280001, "attempt_start_ts": 1484754976344190,
"parent_rev": "d7819bdf2019b5d9b648f73d8d2e6a9f629407f7", "commit_rev":
"2c684a8bef012e3f85338b72d154b3c68867c021"}

[commit-bot: I haz the power, 2017-01-18 17:55:17]

Description was changed from

==========
PaymentApp: Implement InvokePaymentApp() in content/browser side.

This CL is implementing invokePaymentApp() in browser side. During this work,
added FindRegistrationForIdOnly() method to service worker context wrapper and
some codes to embedded worker test helper for testing event dispatching.

BUG=661608
TEST=payment_app_provider_impl_unittest.cc
==========

to

==========
PaymentApp: Implement InvokePaymentApp() in content/browser side.

This CL is implementing invokePaymentApp() in browser side. During this work,
added FindRegistrationForIdOnly() method to service worker context wrapper and
some codes to embedded worker test helper for testing event dispatching.

BUG=661608
TEST=payment_app_provider_impl_unittest.cc

Review-Url: https://codereview.chromium.org/2610163002
Cr-Commit-Position: refs/heads/master@{#444415}
Committed:
https://chromium.googlesource.com/chromium/src/+/2c684a8bef012e3f85338b72d154...
==========

[commit-bot: I haz the power, 2017-01-18 17:55:19]

Committed patchset #13 (id:280001) as
https://chromium.googlesource.com/chromium/src/+/2c684a8bef012e3f85338b72d154...

[Wez, 2017-02-07 22:32:37]

wez@chromium.org changed reviewers:
+ jwd@chromium.org

[Wez, 2017-02-07 22:32:38]

Hallo jwd@chromium.org!
Due to a depot_tools patch which mistakenly removed the OWNERS check for
non-source files (see crbug.com/684270), the following files landed in this CL
and need a retrospective review from you:
	tools/metrics/histograms/histograms.xml
Thanks,
Wez