Disable permissions embargo for plugins.

Disable permissions embargo for plugins.

Html5ByDefault triggers prompts to enable Flash of the type
CONTENT_SETTINGS_TYPE_PLUGINS. Otherwise, sites can't trigger the
plugins prompt, so it doesn't make sense to include it under embargo
(and users may get confused if we embargo plugins permission prompts).

This CL stops embargo from being applied to plugins prompts. A test is
added to verify the behaviour.

BUG=704397
Review-Url: https://codereview.chromium.org/2772513004
Cr-Commit-Position: refs/heads/master@{#460006}
Committed: https://chromium.googlesource.com/chromium/src/+/4beb9d55d0efc81b8a2a2a91fb88266d4156a35b

[dominickn, 2017-03-23 04:17:27]

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

[dominickn, 2017-03-23 04:17:42]

dominickn@chromium.org changed reviewers:
+ benwells@chromium.org

[dominickn, 2017-03-23 04:17:42]

PTAL, thanks

[commit-bot: I haz the power, 2017-03-23 04:17:43]

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-03-23 04:35:16]

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

[commit-bot: I haz the power, 2017-03-23 04:35:17]

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

[benwells, 2017-03-23 04:39:25]

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

https://codereview.chromium.org/2772513004/diff/1/chrome/browser/permissions/...
chrome/browser/permissions/permission_decision_auto_blocker.cc:283: permission
!= CONTENT_SETTINGS_TYPE_PLUGINS &&
I think it would be cleaner to place this in PermissionContext. That is, add
PermissionContextBase::AutoBlockAfterRepeatedDismissal(), and override that for
FlashPermissionContextBase.

Would that work?

[dominickn, 2017-03-23 04:45:23]

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

https://codereview.chromium.org/2772513004/diff/1/chrome/browser/permissions/...
chrome/browser/permissions/permission_decision_auto_blocker.cc:283: permission
!= CONTENT_SETTINGS_TYPE_PLUGINS &&
On 2017/03/23 04:39:25, benwells wrote:
> I think it would be cleaner to place this in PermissionContext. That is, add
> PermissionContextBase::AutoBlockAfterRepeatedDismissal(), and override that
for
> FlashPermissionContextBase.
> 
> Would that work?

That would mean also placing the logic in PermissionQueueController (which has a
separate call to RecordDismissAndEmbargo), which is why I chose to put it in
here. How strongly do you feel about it?

The other downside is that we would no longer correctly record repeated
dismissals for permissions which opt out of auto blocking (unless we pass a
should_block variable into RecordDismissAndEmbargo since this method also
records the dismissal event into content settings). What are your thoughts on
that?

[dominickn, 2017-03-23 05:10:33]

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

[commit-bot: I haz the power, 2017-03-23 05:10:45]

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-03-23 05:29:12]

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

[commit-bot: I haz the power, 2017-03-23 05:29:13]

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

[dominickn, 2017-03-27 23:05:03]

Ping :)

[benwells, 2017-03-28 02:21:32]

lgtm

[dominickn, 2017-03-28 02:31:23]

The CQ bit was checked by dominickn@chromium.org

[commit-bot: I haz the power, 2017-03-28 02:31:40]

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-03-28 03:47:14]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1490668283112780,
"parent_rev": "4187ef8e65b069e4d3e5645446ad04ebdb1c527e", "commit_rev":
"4beb9d55d0efc81b8a2a2a91fb88266d4156a35b"}

[commit-bot: I haz the power, 2017-03-28 03:47:59]

Description was changed from

==========
Disable permissions embargo for plugins.

Html5ByDefault triggers prompts to enable Flash of the type
CONTENT_SETTINGS_TYPE_PLUGINS. Otherwise, sites can't trigger the
plugins prompt, so it doesn't make sense to include it under embargo
(and users may get confused if we embargo plugins permission prompts).

This CL stops embargo from being applied to plugins prompts. A test is
added to verify the behaviour.

BUG=704397
==========

to

==========
Disable permissions embargo for plugins.

Html5ByDefault triggers prompts to enable Flash of the type
CONTENT_SETTINGS_TYPE_PLUGINS. Otherwise, sites can't trigger the
plugins prompt, so it doesn't make sense to include it under embargo
(and users may get confused if we embargo plugins permission prompts).

This CL stops embargo from being applied to plugins prompts. A test is
added to verify the behaviour.

BUG=704397

Review-Url: https://codereview.chromium.org/2772513004
Cr-Commit-Position: refs/heads/master@{#460006}
Committed:
https://chromium.googlesource.com/chromium/src/+/4beb9d55d0efc81b8a2a2a91fb88...
==========

[commit-bot: I haz the power, 2017-03-28 03:48:00]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/4beb9d55d0efc81b8a2a2a91fb88...