Add way for PermissionContext to be automatically restricted to secure origins.

Add way for PermissionContext to be automatically restricted to secure origins.

This is in order to generalize the fix in
https://codereview.chromium.org/1146403004/

BUG=None

Committed: https://crrev.com/a31c6ff16d3f932a4632eb8aec8af103df02f31f
Cr-Commit-Position: refs/heads/master@{#332192}

[mlamouri (slow - plz ping), 2015-05-23 13:52:08]

mlamouri@chromium.org changed reviewers:
+ ddorwin@chromium.org, miguelg@chromium.org, thakis@chromium.org*

[mlamouri (slow - plz ping), 2015-05-23 13:52:09]

thakis@, please review this CL as OWNER

miguelg@ and ddorwin@, FYI.

[Miguel Garcia, 2015-05-26 09:34:34]

https://codereview.chromium.org/1153873003/diff/1/chrome/browser/content_sett...
File chrome/browser/content_settings/permission_context_base.h (right):

https://codereview.chromium.org/1153873003/diff/1/chrome/browser/content_sett...
chrome/browser/content_settings/permission_context_base.h:48: // the permission
from the omnibox.
perhaps add here that we need to decide if the new permission works for
http/https via RestrictToSecureOrigins

https://codereview.chromium.org/1153873003/diff/1/chrome/browser/content_sett...
chrome/browser/content_settings/permission_context_base.h:127: virtual bool
RestrictToSecureOrigins() const = 0;
can you add a default implementation? I guess default to true.

[ddorwin, 2015-05-26 17:31:31]

LGTM % comments. Thanks!

https://codereview.chromium.org/1153873003/diff/1/chrome/browser/content_sett...
File chrome/browser/content_settings/permission_context_base.h (right):

https://codereview.chromium.org/1153873003/diff/1/chrome/browser/content_sett...
chrome/browser/content_settings/permission_context_base.h:127: virtual bool
RestrictToSecureOrigins() const = 0;
Should this be IsRestrcited...?

https://codereview.chromium.org/1153873003/diff/1/chrome/browser/media/protec...
File chrome/browser/media/protected_media_identifier_permission_context.h
(right):

https://codereview.chromium.org/1153873003/diff/1/chrome/browser/media/protec...
chrome/browser/media/protected_media_identifier_permission_context.h:68: 
nit: extra line.

https://codereview.chromium.org/1153873003/diff/1/chrome/browser/notification...
File chrome/browser/notifications/desktop_notification_service.cc (right):

https://codereview.chromium.org/1153873003/diff/1/chrome/browser/notification...
chrome/browser/notifications/desktop_notification_service.cc:257: return false;
Are notifications really not restricted? Looking at the age of this file, I
guess it applies to more than Service Workers.

https://codereview.chromium.org/1153873003/diff/1/chrome/browser/push_messagi...
File chrome/browser/push_messaging/push_messaging_permission_context.cc (right):

https://codereview.chromium.org/1153873003/diff/1/chrome/browser/push_messagi...
chrome/browser/push_messaging/push_messaging_permission_context.cc:106: return
true;
This is a change in behavior (like MIDI in the other CL), right?

[mlamouri (slow - plz ping), 2015-05-29 14:35:15]

miguelg@ and thakis@, PTAL.

https://codereview.chromium.org/1153873003/diff/1/chrome/browser/content_sett...
File chrome/browser/content_settings/permission_context_base.h (right):

https://codereview.chromium.org/1153873003/diff/1/chrome/browser/content_sett...
chrome/browser/content_settings/permission_context_base.h:48: // the permission
from the omnibox.
On 2015/05/26 at 09:34:33, Miguel Garcia wrote:
> perhaps add here that we need to decide if the new permission works for
http/https via RestrictToSecureOrigins

Done.

https://codereview.chromium.org/1153873003/diff/1/chrome/browser/content_sett...
chrome/browser/content_settings/permission_context_base.h:127: virtual bool
RestrictToSecureOrigins() const = 0;
> can you add a default implementation? I guess default to true.

I did not on purpose so implementations will have to think about it.

On 2015/05/26 at 17:31:31, ddorwin wrote:
> Should this be IsRestrcited...?

Yes.

https://codereview.chromium.org/1153873003/diff/1/chrome/browser/media/protec...
File chrome/browser/media/protected_media_identifier_permission_context.h
(right):

https://codereview.chromium.org/1153873003/diff/1/chrome/browser/media/protec...
chrome/browser/media/protected_media_identifier_permission_context.h:68: 
On 2015/05/26 at 17:31:31, ddorwin wrote:
> nit: extra line.

Done.

https://codereview.chromium.org/1153873003/diff/1/chrome/browser/notification...
File chrome/browser/notifications/desktop_notification_service.cc (right):

https://codereview.chromium.org/1153873003/diff/1/chrome/browser/notification...
chrome/browser/notifications/desktop_notification_service.cc:257: return false;
On 2015/05/26 at 17:31:31, ddorwin wrote:
> Are notifications really not restricted? Looking at the age of this file, I
guess it applies to more than Service Workers.

They are not HTTPS-only.

https://codereview.chromium.org/1153873003/diff/1/chrome/browser/push_messagi...
File chrome/browser/push_messaging/push_messaging_permission_context.cc (right):

https://codereview.chromium.org/1153873003/diff/1/chrome/browser/push_messagi...
chrome/browser/push_messaging/push_messaging_permission_context.cc:106: return
true;
On 2015/05/26 at 17:31:31, ddorwin wrote:
> This is a change in behavior (like MIDI in the other CL), right?

Correct.

[Peter Beverloo, 2015-05-29 14:45:58]

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

[Peter Beverloo, 2015-05-29 14:45:59]

Push and notifications lgtm.

https://codereview.chromium.org/1153873003/diff/20001/chrome/browser/media/pr...
File chrome/browser/media/protected_media_identifier_permission_context.cc
(right):

https://codereview.chromium.org/1153873003/diff/20001/chrome/browser/media/pr...
chrome/browser/media/protected_media_identifier_permission_context.cc:92:
OnPlatformVerificationConsentponse,
Unintentional change?

[mlamouri (slow - plz ping), 2015-06-01 12:34:42]

mlamouri@chromium.org changed reviewers:
+ jochen@chromium.org
- thakis@chromium.org*

[mlamouri (slow - plz ping), 2015-06-01 12:34:42]

mlamouri@chromium.org changed required reviewers:
+ jochen@chromium.org

[mlamouri (slow - plz ping), 2015-06-01 12:34:43]

-thakis
+jochen

jochen@, could you have a look at the chrome/ changes?

[jochen (gone - plz use gerrit), 2015-06-01 13:36:19]

lgtm

[mlamouri (slow - plz ping), 2015-06-01 13:57:41]

The CQ bit was checked by mlamouri@chromium.org

[mlamouri (slow - plz ping), 2015-06-01 13:57:42]

The patchset sent to the CQ was uploaded after l-g-t-m from
ddorwin@chromium.org, peter@chromium.org
Link to the patchset: https://codereview.chromium.org/1153873003/#ps40001
(title: "typo")

[commit-bot: I haz the power, 2015-06-01 13:57:52]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1153873003/40001

[commit-bot: I haz the power, 2015-06-01 15:40:58]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-06-01 15:41:39]

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