PaymentApp: Add a interface to query all payment app manifests.

PaymentApp: Add a interface to query all payment app manifests.

This CL is adding GetAllManifests() to query all payment app manifests
in Chrome layer. So, just rename existing PaymentAppContext in
content/browser/payments to PaymentAppContextImpl and then add a new
PaymentAppContext interface in content/public to expose some APIs to
Chrome layer.

This CL doesn't include actual impelementation now and it will be
completed in follow-up CL.

BUG=661608
Committed: https://crrev.com/745d987b499655bcbc62fa96207abe085bb503f6
Cr-Commit-Position: refs/heads/master@{#438569}

[zino, 2016-12-14 16:20:14]

Description was changed from

==========
PaymentApp: Add a interface to query all payment app manifests.

This CL is adding GetAllManifests() to query all payment app manifests in Chrome
layer. So, rename existing PaymentAppContext in content/browser/payments to
PaymentAppContextImpl and then add a new PaymentAppContext interface in
content/public to expose some APIs to Chrome layer.

This CL doesn't include actual impelementation now and it will be completed in
follow-up CL.

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

to

==========
PaymentApp: Add a interface to query all payment app manifests.

This CL is adding GetAllManifests() to query all payment app manifests in Chrome
layer. So, rename existing PaymentAppContext in content/browser/payments to
PaymentAppContextImpl and then add a new PaymentAppContext interface in
content/public to expose some APIs to Chrome layer.

This CL doesn't include actual impelementation now and it will be completed in
follow-up CL.

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

[zino, 2016-12-14 16:20:14]

jinho.bang@samsung.com changed reviewers:
+ avi@chromium.org, rouslan@chromium.org, tommyt@opera.com

[zino, 2016-12-14 16:20:25]

PTAL

[zino, 2016-12-14 16:21:10]

Description was changed from

==========
PaymentApp: Add a interface to query all payment app manifests.

This CL is adding GetAllManifests() to query all payment app manifests in Chrome
layer. So, rename existing PaymentAppContext in content/browser/payments to
PaymentAppContextImpl and then add a new PaymentAppContext interface in
content/public to expose some APIs to Chrome layer.

This CL doesn't include actual impelementation now and it will be completed in
follow-up CL.

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

to

==========
PaymentApp: Add a interface to query all payment app manifests.

This CL is adding GetAllManifests() to query all payment app manifests in Chrome
layer. So, just rename existing PaymentAppContext in content/browser/payments to
PaymentAppContextImpl and then add a new PaymentAppContext interface in
content/public to expose some APIs to Chrome layer.

This CL doesn't include actual impelementation now and it will be completed in
follow-up CL.

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

[Avi (use Gerrit), 2016-12-14 16:31:52]

lgtm

[please use gerrit instead, 2016-12-14 16:36:10]

Description was changed from

==========
PaymentApp: Add a interface to query all payment app manifests.

This CL is adding GetAllManifests() to query all payment app manifests in Chrome
layer. So, just rename existing PaymentAppContext in content/browser/payments to
PaymentAppContextImpl and then add a new PaymentAppContext interface in
content/public to expose some APIs to Chrome layer.

This CL doesn't include actual impelementation now and it will be completed in
follow-up CL.

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

to

==========
PaymentApp: Add a interface to query all payment app manifests.

This CL is adding GetAllManifests() to query all payment app manifests
in Chrome layer. So, just rename existing PaymentAppContext in
content/browser/payments to PaymentAppContextImpl and then add a new
PaymentAppContext interface in content/public to expose some APIs to
Chrome layer.

This CL doesn't include actual impelementation now and it will be
completed in follow-up CL.

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

[please use gerrit instead, 2016-12-14 16:50:06]

https://codereview.chromium.org/2575583003/diff/1/content/browser/payments/pa...
File content/browser/payments/payment_app_context_impl.h (right):

https://codereview.chromium.org/2575583003/diff/1/content/browser/payments/pa...
content/browser/payments/payment_app_context_impl.h:24: public
base::RefCountedThreadSafe<PaymentAppContextImpl> {
The pure interface PaymentAppContext should be listed after the
RefCountedThreadSafe, which has an implementation.

"Multiple inheritance is allowed only when all superclasses, with the possible
exception of the first one, are pure interfaces."

https://google.github.io/styleguide/cppguide.html#Multiple_Inheritance

https://codereview.chromium.org/2575583003/diff/1/content/browser/payments/pa...
content/browser/payments/payment_app_context_impl.h:42: 
// PaymentAppContext implementation:

https://codereview.chromium.org/2575583003/diff/1/content/browser/payments/pa...
content/browser/payments/payment_app_context_impl.h:48: virtual
~PaymentAppContextImpl();
override instead of virtual

Can you make it private?

https://codereview.chromium.org/2575583003/diff/1/content/public/browser/paym...
File content/public/browser/payment_app_context.h (right):

https://codereview.chromium.org/2575583003/diff/1/content/public/browser/paym...
content/public/browser/payment_app_context.h:25: };
Need a destructor:

"An interface class can never be directly instantiated because of the pure
virtual method(s) it declares. To make sure all implementations of the interface
can be destroyed correctly, the interface must also declare a virtual destructor
(in an exception to the first rule, this should not be pure). See Stroustrup,
The C++ Programming Language, 3rd edition, section 12.4 for details."

https://google.github.io/styleguide/cppguide.html#Interfaces

[zino, 2016-12-14 17:20:19]

Thank you for review.

https://codereview.chromium.org/2575583003/diff/1/content/browser/payments/pa...
File content/browser/payments/payment_app_context_impl.h (right):

https://codereview.chromium.org/2575583003/diff/1/content/browser/payments/pa...
content/browser/payments/payment_app_context_impl.h:24: public
base::RefCountedThreadSafe<PaymentAppContextImpl> {
On 2016/12/14 16:50:05, rouslan wrote:
> The pure interface PaymentAppContext should be listed after the
> RefCountedThreadSafe, which has an implementation.
> 
> "Multiple inheritance is allowed only when all superclasses, with the possible
> exception of the first one, are pure interfaces."
> 
> https://google.github.io/styleguide/cppguide.html#Multiple_Inheritance

Done.

https://codereview.chromium.org/2575583003/diff/1/content/browser/payments/pa...
content/browser/payments/payment_app_context_impl.h:42: 
On 2016/12/14 16:50:05, rouslan wrote:
> // PaymentAppContext implementation:

Done.

https://codereview.chromium.org/2575583003/diff/1/content/browser/payments/pa...
content/browser/payments/payment_app_context_impl.h:48: virtual
~PaymentAppContextImpl();
On 2016/12/14 16:50:05, rouslan wrote:
> override instead of virtual
> 
> Can you make it private?

Done.

https://codereview.chromium.org/2575583003/diff/1/content/public/browser/paym...
File content/public/browser/payment_app_context.h (right):

https://codereview.chromium.org/2575583003/diff/1/content/public/browser/paym...
content/public/browser/payment_app_context.h:25: };
On 2016/12/14 16:50:06, rouslan wrote:
> Need a destructor:
> 
> "An interface class can never be directly instantiated because of the pure
> virtual method(s) it declares. To make sure all implementations of the
interface
> can be destroyed correctly, the interface must also declare a virtual
destructor
> (in an exception to the first rule, this should not be pure). See Stroustrup,
> The C++ Programming Language, 3rd edition, section 12.4 for details."
> 
> https://google.github.io/styleguide/cppguide.html#Interfaces

Done.

[please use gerrit instead, 2016-12-14 17:39:40]

lgtm % comment

https://codereview.chromium.org/2575583003/diff/20001/content/browser/payment...
File content/browser/payments/payment_app_context_impl.cc (right):

https://codereview.chromium.org/2575583003/diff/20001/content/browser/payment...
content/browser/payments/payment_app_context_impl.cc:25:
DCHECK(services_.empty());
Put the destructor right after GetAllManifests(). Order of method definitions
should match the order of method declarations for easy for maintenance and code
review.

[zino, 2016-12-14 17:48:01]

Thank you for review!

https://codereview.chromium.org/2575583003/diff/20001/content/browser/payment...
File content/browser/payments/payment_app_context_impl.cc (right):

https://codereview.chromium.org/2575583003/diff/20001/content/browser/payment...
content/browser/payments/payment_app_context_impl.cc:25:
DCHECK(services_.empty());
On 2016/12/14 17:39:40, rouslan wrote:
> Put the destructor right after GetAllManifests(). Order of method definitions
> should match the order of method declarations for easy for maintenance and
code
> review.

Done.

[zino, 2016-12-14 17:48:06]

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

[zino, 2016-12-14 17:48:06]

The patchset sent to the CQ was uploaded after l-g-t-m from avi@chromium.org,
rouslan@chromium.org
Link to the patchset: https://codereview.chromium.org/2575583003/#ps40001
(title: "PaymentApp: Add a interface to query all payment app manifests.")

[commit-bot: I haz the power, 2016-12-14 17:48:31]

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, 2016-12-14 19:06:58]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1481737686104340,
"parent_rev": "dc6e0e7012d5fc4eb797ddec3754bfc3196ad441", "commit_rev":
"3c498578f6b0a948c1ba1cbe40947e8697d09565"}

[commit-bot: I haz the power, 2016-12-14 19:07:31]

Description was changed from

==========
PaymentApp: Add a interface to query all payment app manifests.

This CL is adding GetAllManifests() to query all payment app manifests
in Chrome layer. So, just rename existing PaymentAppContext in
content/browser/payments to PaymentAppContextImpl and then add a new
PaymentAppContext interface in content/public to expose some APIs to
Chrome layer.

This CL doesn't include actual impelementation now and it will be
completed in follow-up CL.

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

to

==========
PaymentApp: Add a interface to query all payment app manifests.

This CL is adding GetAllManifests() to query all payment app manifests
in Chrome layer. So, just rename existing PaymentAppContext in
content/browser/payments to PaymentAppContextImpl and then add a new
PaymentAppContext interface in content/public to expose some APIs to
Chrome layer.

This CL doesn't include actual impelementation now and it will be
completed in follow-up CL.

BUG=661608

Review-Url: https://codereview.chromium.org/2575583003
==========

[commit-bot: I haz the power, 2016-12-14 19:07:32]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-12-14 19:11:00]

Description was changed from

==========
PaymentApp: Add a interface to query all payment app manifests.

This CL is adding GetAllManifests() to query all payment app manifests
in Chrome layer. So, just rename existing PaymentAppContext in
content/browser/payments to PaymentAppContextImpl and then add a new
PaymentAppContext interface in content/public to expose some APIs to
Chrome layer.

This CL doesn't include actual impelementation now and it will be
completed in follow-up CL.

BUG=661608

Review-Url: https://codereview.chromium.org/2575583003
==========

to

==========
PaymentApp: Add a interface to query all payment app manifests.

This CL is adding GetAllManifests() to query all payment app manifests
in Chrome layer. So, just rename existing PaymentAppContext in
content/browser/payments to PaymentAppContextImpl and then add a new
PaymentAppContext interface in content/public to expose some APIs to
Chrome layer.

This CL doesn't include actual impelementation now and it will be
completed in follow-up CL.

BUG=661608

Committed: https://crrev.com/745d987b499655bcbc62fa96207abe085bb503f6
Cr-Commit-Position: refs/heads/master@{#438569}
==========

[commit-bot: I haz the power, 2016-12-14 19:11:01]

Patchset 3 (id:??) landed as
https://crrev.com/745d987b499655bcbc62fa96207abe085bb503f6
Cr-Commit-Position: refs/heads/master@{#438569}