[HBD] Only use Flash Download interception for DETECT content setting.

[HBD] Only use Flash Download interception for DETECT content setting.

Previously, when the HBD feature flag was on, Flash downloads would be
intercepted in any case. This CL makes it only happen when the user
has the DETECT content setting on.

In the ALLOW case, Flash should already be installed, so there is no
need to install. In the BLOCK case, the user has already decided to not
run Flash, so there is no need to display a prompt to ask.

BUG=641619
Committed: https://crrev.com/fdf211f02076f6c7251fcece1c36821c96af6c8e
Cr-Commit-Position: refs/heads/master@{#418911}

[tommycli, 2016-09-14 21:37:14]

tommycli@chromium.org changed reviewers:
+ bauerb@chromium.org

[tommycli, 2016-09-14 21:37:15]

bauerb: PTAL at this. Restricts the download intercept to only when user has
DETECT content setting.

[tommycli, 2016-09-14 21:37:19]

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

[commit-bot: I haz the power, 2016-09-14 21:37:59]

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, 2016-09-14 23:06:29]

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

[commit-bot: I haz the power, 2016-09-14 23:06:29]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[Bernhard Bauer, 2016-09-15 08:34:19]

Why do we know that the user has Flash if the setting is ALLOW? They could just
change the default setting before ever encountering Flash, no?

[tommycli, 2016-09-15 17:05:46]

On 2016/09/15 08:34:19, Bernhard Bauer wrote:
> Why do we know that the user has Flash if the setting is ALLOW? They could
just
> change the default setting before ever encountering Flash, no?

Once the Download navigation is intercepted, they see a permission bubble popup
that lets them add an ALLOW or BLOCK exception for this site.

If the user is already on ALLOW, that permission bubble popup would be useless
to them, since they have already expressed a preference (ALLOW), so I think
that's why we should only intercept on DETECT.

[Bernhard Bauer, 2016-09-15 17:18:12]

On 2016/09/15 17:05:46, tommycli wrote:
> On 2016/09/15 08:34:19, Bernhard Bauer wrote:
> > Why do we know that the user has Flash if the setting is ALLOW? They could
> just
> > change the default setting before ever encountering Flash, no?
> 
> Once the Download navigation is intercepted, they see a permission bubble
popup
> that lets them add an ALLOW or BLOCK exception for this site.
> 
> If the user is already on ALLOW, that permission bubble popup would be useless
> to them, since they have already expressed a preference (ALLOW), so I think
> that's why we should only intercept on DETECT.

Oh, so the thinking is that if the setting is ALLOW, we can infer that this is
not the user being directed to install Flash by the page due to it not being
available? OK, that makes sense (I think 😃).

Just to check, this has nothing to do with whether Flash is installed at all
(which will be handled by the component updater on demand), right?

[Bernhard Bauer, 2016-09-15 17:19:03]

On 2016/09/15 17:18:12, Bernhard Bauer wrote:
> On 2016/09/15 17:05:46, tommycli wrote:
> > On 2016/09/15 08:34:19, Bernhard Bauer wrote:
> > > Why do we know that the user has Flash if the setting is ALLOW? They could
> > just
> > > change the default setting before ever encountering Flash, no?
> > 
> > Once the Download navigation is intercepted, they see a permission bubble
> popup
> > that lets them add an ALLOW or BLOCK exception for this site.
> > 
> > If the user is already on ALLOW, that permission bubble popup would be
useless
> > to them, since they have already expressed a preference (ALLOW), so I think
> > that's why we should only intercept on DETECT.
> 
> Oh, so the thinking is that if the setting is ALLOW, we can infer that this is
> not the user being directed to install Flash by the page due to it not being
> available? OK, that makes sense (I think 😃).
> 
> Just to check, this has nothing to do with whether Flash is installed at all
> (which will be handled by the component updater on demand), right?

(And LGTM 😃)

[tommycli, 2016-09-15 17:48:51]

On 2016/09/15 17:18:12, Bernhard Bauer wrote:
> On 2016/09/15 17:05:46, tommycli wrote:
> > On 2016/09/15 08:34:19, Bernhard Bauer wrote:
> > > Why do we know that the user has Flash if the setting is ALLOW? They could
> > just
> > > change the default setting before ever encountering Flash, no?
> > 
> > Once the Download navigation is intercepted, they see a permission bubble
> popup
> > that lets them add an ALLOW or BLOCK exception for this site.
> > 
> > If the user is already on ALLOW, that permission bubble popup would be
useless
> > to them, since they have already expressed a preference (ALLOW), so I think
> > that's why we should only intercept on DETECT.
> 
> Oh, so the thinking is that if the setting is ALLOW, we can infer that this is
> not the user being directed to install Flash by the page due to it not being
> available? OK, that makes sense (I think 😃).
> 
> Just to check, this has nothing to do with whether Flash is installed at all
> (which will be handled by the component updater on demand), right?

Right, it has nothing to do with whether or not Flash is actually installed. In
the Flash component updater case, the website actually thinks that Flash is
already installed! (A fake entry is in navigator.plugins()).

This is purely for the HTML5 by default content setting case, where it's hidden
from navigator.plugins().

Thanks!

[tommycli, 2016-09-15 17:49:17]

The CQ bit was checked by tommycli@chromium.org

[commit-bot: I haz the power, 2016-09-15 17:50:04]

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-09-15 18:34:42]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-09-15 18:36:56]

Description was changed from

==========
[HBD] Only use Flash Download interception for DETECT content setting.

Previously, when the HBD feature flag was on, Flash downloads would be
intercepted in any case. This CL makes it only happen when the user
has the DETECT content setting on.

In the ALLOW case, Flash should already be installed, so there is no
need to install. In the BLOCK case, the user has already decided to not
run Flash, so there is no need to display a prompt to ask.

BUG=641619
==========

to

==========
[HBD] Only use Flash Download interception for DETECT content setting.

Previously, when the HBD feature flag was on, Flash downloads would be
intercepted in any case. This CL makes it only happen when the user
has the DETECT content setting on.

In the ALLOW case, Flash should already be installed, so there is no
need to install. In the BLOCK case, the user has already decided to not
run Flash, so there is no need to display a prompt to ask.

BUG=641619

Committed: https://crrev.com/fdf211f02076f6c7251fcece1c36821c96af6c8e
Cr-Commit-Position: refs/heads/master@{#418911}
==========

[commit-bot: I haz the power, 2016-09-15 18:36:57]

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