[subresource_filter] Force the popup blocker on sites with activation

[subresource_filter] Force the popup blocker on sites with activation

This CL essentially strips the user_gesture bit from the popup blocker
heuristic on sites with subresource_filter activation.

BUG=689992
Review-Url: https://codereview.chromium.org/2822773002
Cr-Commit-Position: refs/heads/master@{#466991}
Committed: https://chromium.googlesource.com/chromium/src/+/cebcfd84a5f1d8a7f42f177cc27f9248446cbbcd

[Charlie Harrison, 2017-04-14 19:42:59]

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

[commit-bot: I haz the power, 2017-04-14 19:43:26]

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

[Charlie Harrison, 2017-04-14 19:45:05]

Description was changed from

==========
[subresource_filter] Activated frames cannot create new windows

BUG=689992
==========

to

==========
[subresource_filter] Activated frames cannot create new windows

BUG=689992
==========

[commit-bot: I haz the power, 2017-04-14 20:54:47]

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

[commit-bot: I haz the power, 2017-04-14 20:54:47]

Dry run: This issue passed the CQ dry run.

[Charlie Harrison, 2017-04-17 18:21:23]

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

[commit-bot: I haz the power, 2017-04-17 18:21:54]

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

[Charlie Harrison, 2017-04-17 18:56:26]

Description was changed from

==========
[subresource_filter] Activated frames cannot create new windows

BUG=689992
==========

to

==========
[subresource_filter] Force the popup blocker on Activated sites

BUG=689992
==========

[commit-bot: I haz the power, 2017-04-17 19:38:50]

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

[commit-bot: I haz the power, 2017-04-17 19:38:51]

Dry run: This issue passed the CQ dry run.

[Charlie Harrison, 2017-04-22 18:37:57]

csharrison@chromium.org changed reviewers:
+ engedy@chromium.org

[Charlie Harrison, 2017-04-22 18:37:57]

engedy: PTAL? Still needs the dependent patch to land but the logic ends up
being pretty simple.

[engedy, 2017-04-24 10:03:28]

Huge kudos for doing the refactoring so cleanly in a separate CL. LGTM % minor
comments + could we say `sites with activation` in the CL description?

https://codereview.chromium.org/2822773002/diff/20001/chrome/browser/chrome_c...
File chrome/browser/chrome_content_browser_client.cc (right):

https://codereview.chromium.org/2822773002/diff/20001/chrome/browser/chrome_c...
chrome/browser/chrome_content_browser_client.cc:2555: bool popup_block_candidate
=
nit: const bool

https://codereview.chromium.org/2822773002/diff/20001/chrome/browser/subresou...
File chrome/browser/subresource_filter/subresource_filter_browsertest.cc
(right):

https://codereview.chromium.org/2822773002/diff/20001/chrome/browser/subresou...
chrome/browser/subresource_filter/subresource_filter_browsertest.cc:891:
SetRulesetWithRules({testing::CreateSuffixRule("included_script.js")}));
Would passing std::vector<proto::UrlRule>(), that is, an empty ruleset work
here?

https://codereview.chromium.org/2822773002/diff/20001/chrome/browser/subresou...
chrome/browser/subresource_filter/subresource_filter_browsertest.cc:898:
EXPECT_FALSE(opened_window);
For good measure, let's augment this test so that it also checks that a window
can be opened for sites without activation. (To ensure the opening fails not
because of bugs in this test.)

Easiest could be to serve the testing page both from a.com and b.com, there are
already other tests in this file doing this.

[Charlie Harrison, 2017-04-24 15:41:34]

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

[Charlie Harrison, 2017-04-24 15:41:55]

Thanks Balazs!

https://codereview.chromium.org/2822773002/diff/20001/chrome/browser/chrome_c...
File chrome/browser/chrome_content_browser_client.cc (right):

https://codereview.chromium.org/2822773002/diff/20001/chrome/browser/chrome_c...
chrome/browser/chrome_content_browser_client.cc:2555: bool popup_block_candidate
=
On 2017/04/24 10:03:27, engedy wrote:
> nit: const bool

Done.

https://codereview.chromium.org/2822773002/diff/20001/chrome/browser/subresou...
File chrome/browser/subresource_filter/subresource_filter_browsertest.cc
(right):

https://codereview.chromium.org/2822773002/diff/20001/chrome/browser/subresou...
chrome/browser/subresource_filter/subresource_filter_browsertest.cc:891:
SetRulesetWithRules({testing::CreateSuffixRule("included_script.js")}));
On 2017/04/24 10:03:27, engedy wrote:
> Would passing std::vector<proto::UrlRule>(), that is, an empty ruleset work
> here?

Yep, done.

https://codereview.chromium.org/2822773002/diff/20001/chrome/browser/subresou...
chrome/browser/subresource_filter/subresource_filter_browsertest.cc:898:
EXPECT_FALSE(opened_window);
On 2017/04/24 10:03:27, engedy wrote:
> For good measure, let's augment this test so that it also checks that a window
> can be opened for sites without activation. (To ensure the opening fails not
> because of bugs in this test.)
> 
> Easiest could be to serve the testing page both from a.com and b.com, there
are
> already other tests in this file doing this.

Done.

[commit-bot: I haz the power, 2017-04-24 15:42:22]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Charlie Harrison, 2017-04-24 15:42:28]

csharrison@chromium.org changed reviewers:
+ jkarlin@chromium.org

[Charlie Harrison, 2017-04-24 15:42:28]

+jkarlin would you PTAL as well?

[Charlie Harrison, 2017-04-24 15:43:34]

Description was changed from

==========
[subresource_filter] Force the popup blocker on Activated sites

BUG=689992
==========

to

==========
[subresource_filter] Force the popup blocker on sites with activation

This CL essentially strips the user_gesture bit from the popup blocker
heuristic on sites with subresource_filter activation.

BUG=689992
==========

[commit-bot: I haz the power, 2017-04-24 16:57:02]

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

[commit-bot: I haz the power, 2017-04-24 16:57:03]

Dry run: This issue passed the CQ dry run.

[jkarlin, 2017-04-24 18:49:38]

lgtm!

[Charlie Harrison, 2017-04-24 18:51:30]

Thanks! I will likely wait ~24h after the dependent CL lands
before sending this one to CQ just in case that one ends up
being reverted.

[Charlie Harrison, 2017-04-25 14:51:30]

The CQ bit was checked by csharrison@chromium.org

[Charlie Harrison, 2017-04-25 14:51:30]

The patchset sent to the CQ was uploaded after l-g-t-m from engedy@chromium.org
Link to the patchset: https://codereview.chromium.org/2822773002/#ps40001
(title: "engedy review")

[commit-bot: I haz the power, 2017-04-25 14:51:56]

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-04-25 14:56:39]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1493131890474890,
"parent_rev": "53f787b5b82c2e4d3967bc09a803faae7ea42350", "commit_rev":
"cebcfd84a5f1d8a7f42f177cc27f9248446cbbcd"}

[commit-bot: I haz the power, 2017-04-25 14:56:49]

Description was changed from

==========
[subresource_filter] Force the popup blocker on sites with activation

This CL essentially strips the user_gesture bit from the popup blocker
heuristic on sites with subresource_filter activation.

BUG=689992
==========

to

==========
[subresource_filter] Force the popup blocker on sites with activation

This CL essentially strips the user_gesture bit from the popup blocker
heuristic on sites with subresource_filter activation.

BUG=689992

Review-Url: https://codereview.chromium.org/2822773002
Cr-Commit-Position: refs/heads/master@{#466991}
Committed:
https://chromium.googlesource.com/chromium/src/+/cebcfd84a5f1d8a7f42f177cc27f...
==========

[commit-bot: I haz the power, 2017-04-25 14:56:50]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/cebcfd84a5f1d8a7f42f177cc27f...