PaymentHandler: Implement requestPermission().

PaymentHandler: Implement requestPermission().

The method is used to get a permission to install a new payment handler.
The spec[1] is still unclear but we can implement this API via experiences of
other APIs. We will also change the related spec context based on this patch.

This patch includes the following things according to the guide
in chrome/browser/permissions/permission_context_base.h
  - Implement web-facing API in Blink. (but it's still behind a runtime flag)
  - Add a new CONTENT_SETTINGS_TYPE_PAYMENT_HANDLER type.
  - Add PaymentHandlerPermissionContext class that inherits from
    PermissionContextBase and passes the new permission.
  - Provide permission UI including infobar for Android.
  - Test (payment_handler_permission_context_unittest.cc)

Related spec link:
  [1] https://w3c.github.io/payment-handler/#requestpermission-method

BUG=661608, 665949

[zino, 2017-06-20 12:14:24]

Description was changed from

==========
PaymentHandler: Implement requestPermission().

The method is used to get a permission to install a new payment handler.
The spec[1] is still unclear but we can implement this API via experiences of
other APIs. We will also change the related spec context based on this patch.

This patch includes the following things according to the guide
in chrome/browser/permissions/permission_context_base.h
  - Implement web-facing API in Blink. (but it's still behind a runtime flag)
  - Add a new CONTENT_SETTINGS_TYPE_PAYMENT_HANDLER type.
  - Add PaymentHandlerPermissionContext class that inherits from
    PermissionContextBase and passes the new permission.
  - Provide permission UI including infobar for Android.
  - Test (payment_handler_permission_context_unittest.cc)

Related spec link:
  [1] https://w3c.github.io/payment-handler/#requestpermission-method

BUG=661608,665949
==========

to

==========
PaymentHandler: Implement requestPermission().

The method is used to get a permission to install a new payment handler.
The spec[1] is still unclear but we can implement this API via experiences of
other APIs. We will also change the related spec context based on this patch.

This patch includes the following things according to the guide
in chrome/browser/permissions/permission_context_base.h
  - Implement web-facing API in Blink. (but it's still behind a runtime flag)
  - Add a new CONTENT_SETTINGS_TYPE_PAYMENT_HANDLER type.
  - Add PaymentHandlerPermissionContext class that inherits from
    PermissionContextBase and passes the new permission.
  - Provide permission UI including infobar for Android.
  - Test (payment_handler_permission_context_unittest.cc)

Related spec link:
  [1] https://w3c.github.io/payment-handler/#requestpermission-method

BUG=661608,665949
==========

[zino, 2017-06-20 12:14:25]

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

[zino, 2017-06-20 12:15:50]

PTAL

rouslan@ for payments
mlamouri@ for permissions

I'll add some screenshots in crbug. (and then share the link)

[please use gerrit instead, 2017-06-20 13:09:27]

Can you send a pull request to the spec that describes your plan? You can
copy-paste from the other specs that you're using for guidance.

[please use gerrit instead, 2017-06-20 13:28:09]

Please hook up checking for permission when a service worker attempts to
register a payment app.

https://codereview.chromium.org/2946013002/diff/1/chrome/browser/payments/OWNERS
File chrome/browser/payments/OWNERS (right):

https://codereview.chromium.org/2946013002/diff/1/chrome/browser/payments/OWN...
chrome/browser/payments/OWNERS:3: per-file
payment_handler*=jinho.bang@samsung.com
Please add this directory to the WATCHLIST file.

https://codereview.chromium.org/2946013002/diff/1/chrome/browser/payments/pay...
File chrome/browser/payments/payment_handler_permission_context.h (right):

https://codereview.chromium.org/2946013002/diff/1/chrome/browser/payments/pay...
chrome/browser/payments/payment_handler_permission_context.h:11: class GURL;
Unused.

https://codereview.chromium.org/2946013002/diff/1/chrome/browser/payments/pay...
chrome/browser/payments/payment_handler_permission_context.h:12: class
PermissionRequestID;
Unused.

https://codereview.chromium.org/2946013002/diff/1/chrome/browser/payments/pay...
chrome/browser/payments/payment_handler_permission_context.h:14: 
Let's put this in the "payments" namespace.

https://codereview.chromium.org/2946013002/diff/1/chrome/browser/payments/pay...
File chrome/browser/payments/payment_handler_permission_infobar_delegate.cc
(right):

https://codereview.chromium.org/2946013002/diff/1/chrome/browser/payments/pay...
chrome/browser/payments/payment_handler_permission_infobar_delegate.cc:32:
return IDR_ANDROID_INFOBAR_PAYMENT_HANDLER;
This file does not seem to be Android-specific. Does not seem right to use
Android-specific constants.

https://codereview.chromium.org/2946013002/diff/1/chrome/browser/payments/pay...
File chrome/browser/payments/payment_handler_permission_infobar_delegate.h
(right):

https://codereview.chromium.org/2946013002/diff/1/chrome/browser/payments/pay...
chrome/browser/payments/payment_handler_permission_infobar_delegate.h:8:
#include <string>
Not used.

https://codereview.chromium.org/2946013002/diff/1/chrome/browser/payments/pay...
chrome/browser/payments/payment_handler_permission_infobar_delegate.h:12: 
Let's put this in the "payments" namespace.

https://codereview.chromium.org/2946013002/diff/1/chrome/browser/payments/pay...
chrome/browser/payments/payment_handler_permission_infobar_delegate.h:17: const
GURL& requesting_frame,
Forward declare GURL.

https://codereview.chromium.org/2946013002/diff/1/chrome/browser/payments/pay...
chrome/browser/payments/payment_handler_permission_infobar_delegate.h:19:
Profile* profile,
Forward declare Profile.

https://codereview.chromium.org/2946013002/diff/1/chrome/browser/payments/pay...
chrome/browser/payments/payment_handler_permission_infobar_delegate.h:20: const
PermissionSetCallback& callback);
Forward declare PermissionSetCallback.

https://codereview.chromium.org/2946013002/diff/1/chrome/browser/permissions/...
File chrome/browser/permissions/permission_infobar_delegate.cc (right):

https://codereview.chromium.org/2946013002/diff/1/chrome/browser/permissions/...
chrome/browser/permissions/permission_infobar_delegate.cc:54: return
std::unique_ptr<PermissionInfoBarDelegate>(
Since you're already here, please change all these to base::MakeUnique.

https://codereview.chromium.org/2946013002/diff/1/chrome/browser/permissions/...
chrome/browser/permissions/permission_infobar_delegate.cc:72: new
PaymentHandlerPermissionInfoBarDelegate(
return
base::MakeUnique<PaymentHandlerPermissionInfoBarDelegate>(requesting_frame,
user_gesture, profile, callback);

https://codereview.chromium.org/2946013002/diff/1/components/content_settings...
File components/content_settings/core/common/content_settings_types.h (right):

https://codereview.chromium.org/2946013002/diff/1/components/content_settings...
components/content_settings/core/common/content_settings_types.h:70:
CONTENT_SETTINGS_TYPE_PAYMENT_HANDLER,
Add a comment, please.

https://codereview.chromium.org/2946013002/diff/1/components/payments_strings...
File components/payments_strings.grdp (right):

https://codereview.chromium.org/2946013002/diff/1/components/payments_strings...
components/payments_strings.grdp:315: Chromium needs permission to install a new
payment app provided from this site.
"Chromium" in strings should be used inside of <if expr="not _google_chrome">.
Please also add a string for "Chrome".

https://codereview.chromium.org/2946013002/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/payments/PaymentManager.h (right):

https://codereview.chromium.org/2946013002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/payments/PaymentManager.h:38: // For
PaymentManager service
Should this comment be for the next method?

[raymes, 2017-06-21 01:18:47]

raymes@chromium.org changed reviewers:
+ raymes@chromium.org

[raymes, 2017-06-21 01:18:47]

Hey Rouslan, it would be nice to understand a bit more what the plans are for
the payment request permission (both in the UI and in the web platform) prior to
landing code. Would it be possible for you (or someone else) to attend an Enamel
Office hours to share more? +emilyschechter can help arrange this. Thanks!

[please use gerrit instead, 2017-06-21 15:02:36]

On 2017/06/21 01:18:47, raymes wrote:
> Hey Rouslan, it would be nice to understand a bit more what the plans are for
> the payment request permission (both in the UI and in the web platform) prior
to
> landing code. Would it be possible for you (or someone else) to attend an
Enamel
> Office hours to share more? +emilyschechter can help arrange this. Thanks!

Sure thing!

[zino, 2017-06-22 17:26:49]

Thank you for review.

I addressed all your comments except message on ui.

https://codereview.chromium.org/2946013002/diff/1/chrome/browser/payments/OWNERS
File chrome/browser/payments/OWNERS (right):

https://codereview.chromium.org/2946013002/diff/1/chrome/browser/payments/OWN...
chrome/browser/payments/OWNERS:3: per-file
payment_handler*=jinho.bang@samsung.com
On 2017/06/20 13:28:08, rouslan wrote:
> Please add this directory to the WATCHLIST file.

Done.

https://codereview.chromium.org/2946013002/diff/1/chrome/browser/payments/pay...
File chrome/browser/payments/payment_handler_permission_context.h (right):

https://codereview.chromium.org/2946013002/diff/1/chrome/browser/payments/pay...
chrome/browser/payments/payment_handler_permission_context.h:11: class GURL;
On 2017/06/20 13:28:08, rouslan wrote:
> Unused.

Done.

https://codereview.chromium.org/2946013002/diff/1/chrome/browser/payments/pay...
chrome/browser/payments/payment_handler_permission_context.h:12: class
PermissionRequestID;
On 2017/06/20 13:28:08, rouslan wrote:
> Unused.

Done.

https://codereview.chromium.org/2946013002/diff/1/chrome/browser/payments/pay...
chrome/browser/payments/payment_handler_permission_context.h:14: 
On 2017/06/20 13:28:08, rouslan wrote:
> Let's put this in the "payments" namespace.

Done.

https://codereview.chromium.org/2946013002/diff/1/chrome/browser/payments/pay...
File chrome/browser/payments/payment_handler_permission_infobar_delegate.cc
(right):

https://codereview.chromium.org/2946013002/diff/1/chrome/browser/payments/pay...
chrome/browser/payments/payment_handler_permission_infobar_delegate.cc:32:
return IDR_ANDROID_INFOBAR_PAYMENT_HANDLER;
On 2017/06/20 13:28:08, rouslan wrote:
> This file does not seem to be Android-specific. Does not seem right to use
> Android-specific constants.

This should be Android-specific. I moved this file and its header file into
android directory.
Thanks!

https://codereview.chromium.org/2946013002/diff/1/chrome/browser/payments/pay...
File chrome/browser/payments/payment_handler_permission_infobar_delegate.h
(right):

https://codereview.chromium.org/2946013002/diff/1/chrome/browser/payments/pay...
chrome/browser/payments/payment_handler_permission_infobar_delegate.h:8:
#include <string>
On 2017/06/20 13:28:09, rouslan wrote:
> Not used.

Done.

https://codereview.chromium.org/2946013002/diff/1/chrome/browser/payments/pay...
chrome/browser/payments/payment_handler_permission_infobar_delegate.h:12: 
On 2017/06/20 13:28:09, rouslan wrote:
> Let's put this in the "payments" namespace.

Done.

https://codereview.chromium.org/2946013002/diff/1/chrome/browser/payments/pay...
chrome/browser/payments/payment_handler_permission_infobar_delegate.h:17: const
GURL& requesting_frame,
On 2017/06/20 13:28:08, rouslan wrote:
> Forward declare GURL.

Done.

https://codereview.chromium.org/2946013002/diff/1/chrome/browser/payments/pay...
chrome/browser/payments/payment_handler_permission_infobar_delegate.h:19:
Profile* profile,
On 2017/06/20 13:28:09, rouslan wrote:
> Forward declare Profile.

Done.

https://codereview.chromium.org/2946013002/diff/1/chrome/browser/payments/pay...
chrome/browser/payments/payment_handler_permission_infobar_delegate.h:20: const
PermissionSetCallback& callback);
On 2017/06/20 13:28:08, rouslan wrote:
> Forward declare PermissionSetCallback.

It's defined in parent class.

https://codereview.chromium.org/2946013002/diff/1/chrome/browser/permissions/...
File chrome/browser/permissions/permission_infobar_delegate.cc (right):

https://codereview.chromium.org/2946013002/diff/1/chrome/browser/permissions/...
chrome/browser/permissions/permission_infobar_delegate.cc:54: return
std::unique_ptr<PermissionInfoBarDelegate>(
On 2017/06/20 13:28:09, rouslan wrote:
> Since you're already here, please change all these to base::MakeUnique.

Done.

https://codereview.chromium.org/2946013002/diff/1/chrome/browser/permissions/...
chrome/browser/permissions/permission_infobar_delegate.cc:72: new
PaymentHandlerPermissionInfoBarDelegate(
On 2017/06/20 13:28:09, rouslan wrote:
> return
> base::MakeUnique<PaymentHandlerPermissionInfoBarDelegate>(requesting_frame,
> user_gesture, profile, callback);

Done.

https://codereview.chromium.org/2946013002/diff/1/components/content_settings...
File components/content_settings/core/common/content_settings_types.h (right):

https://codereview.chromium.org/2946013002/diff/1/components/content_settings...
components/content_settings/core/common/content_settings_types.h:70:
CONTENT_SETTINGS_TYPE_PAYMENT_HANDLER,
On 2017/06/20 13:28:09, rouslan wrote:
> Add a comment, please.

Done.

https://codereview.chromium.org/2946013002/diff/1/components/payments_strings...
File components/payments_strings.grdp (right):

https://codereview.chromium.org/2946013002/diff/1/components/payments_strings...
components/payments_strings.grdp:315: Chromium needs permission to install a new
payment app provided from this site.
On 2017/06/20 13:28:09, rouslan wrote:
> "Chromium" in strings should be used inside of <if expr="not _google_chrome">.
> Please also add a string for "Chrome".

I'll wait for a new string. (as per crbug's comments)

https://codereview.chromium.org/2946013002/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/payments/PaymentManager.h (right):

https://codereview.chromium.org/2946013002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/payments/PaymentManager.h:38: // For
PaymentManager service
On 2017/06/20 13:28:09, rouslan wrote:
> Should this comment be for the next method?

Done.

[please use gerrit instead, 2017-06-22 19:27:49]

Everything looks good. I will hold my l-g-t-m until we figure out the strings on
the bug.

[zino, 2017-07-06 09:24:12]

Patchset #3 (id:40001) has been deleted