Change the "Disable outdated plug-ins" lab to block them instead.

Change the "Disable outdated plug-ins" lab to block them instead.

This means that outdated plug-ins still show up in navigator.plugins, which gets rid of the misleading "Install Quicktime" messages.

Screens: http://www.dropmocks.com/mNkqe

BUG=47731
TEST=none

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=69435

[Bernhard Bauer, 2010-12-15 19:57:30]

please review. thanks!

[cevans, 2010-12-15 20:07:45]

Code change looks reasonable to me. I had one thing to confirm, one question
and one suggestion:

- To confirm, an out-of-date situation seems to take precedence when the
user is also in "block" mode? This seems best; I'd want this precedence so I
don't accidentally run an out-of-date plugin whilst in "block" or
"click-to-play" mode.

- Suggestion: can we use a different resource string for "block mode" vs.
"out-of-date"? I think we want two different strings: "This plug-in is
blocked" vs. "This plug-in is blocked because it is out of date". We also
need to know the reason for the block for some future tweaks.

- Question: now that we've resolved how the PMs want the UI to behave, can
we turn this on by default? This is an M10 feature, so the sooner we turn it
on on trunk, the sooner we get feedback.


Cheers
Chris

On Wed, Dec 15, 2010 at 11:57 AM, <bauerb@chromium.org> wrote:

> Reviewers: darin,
>
> Message:
> please review. thanks!
>
> Description:
> Change the "Disable outdated plug-ins" lab to block them instead.
>
> This means that outdated plug-ins still show up in navigator.plugins, which
> gets
> rid of the misleading "Install Quicktime" messages.
>
> Screens: http://www.dropmocks.com/mNkqe
>
> BUG=47731
> TEST=none
>
> Please review this at http://codereview.chromium.org/5917001/
>
> SVN Base: svn://svn.chromium.org/chrome/trunk/src
>
> Affected files:
>  M chrome/app/generated_resources.grd
>  M chrome/browser/plugin_service.cc
>  M chrome/browser/plugin_updater.cc
>  M chrome/browser/tab_contents/tab_contents.cc
>  M chrome/renderer/render_view.h
>  M chrome/renderer/render_view.cc
>
>
>

[Bernhard Bauer, 2010-12-15 20:27:17]

On Wed, Dec 15, 2010 at 20:07, Chris Evans <cevans@google.com> wrote:
> Code change looks reasonable to me. I had one thing to confirm, one question
> and one suggestion:
> - To confirm, an out-of-date situation seems to take precedence when the
> user is also in "block" mode? This seems best; I'd want this precedence so I
> don't accidentally run an out-of-date plugin whilst in "block" or
> "click-to-play" mode.

Yes, the out-of-date check is the first thing.

> - Suggestion: can we use a different resource string for "block mode" vs.
> "out-of-date"? I think we want two different strings: "This plug-in is
> blocked" vs. "This plug-in is blocked because it is out of date". We also
> need to know the reason for the block for some future tweaks.

Sure.

> - Question: now that we've resolved how the PMs want the UI to behave, can
> we turn this on by default? This is an M10 feature, so the sooner we turn it
> on on trunk, the sooner we get feedback.

:-D

My idea was to land it, have the UI leads try it out in a
continuous/Canary build, and then turn it on by default.

Bernhard.

>
> Cheers
> Chris
>
> On Wed, Dec 15, 2010 at 11:57 AM, <bauerb@chromium.org> wrote:
>>
>> Reviewers: darin,
>>
>> Message:
>> please review. thanks!
>>
>> Description:
>> Change the "Disable outdated plug-ins" lab to block them instead.
>>
>> This means that outdated plug-ins still show up in navigator.plugins,
>> which gets
>> rid of the misleading "Install Quicktime" messages.
>>
>> Screens: http://www.dropmocks.com/mNkqe
>>
>> BUG=47731
>> TEST=none
>>
>> Please review this at http://codereview.chromium.org/5917001/
>>
>> SVN Base: svn://svn.chromium.org/chrome/trunk/src
>>
>> Affected files:
>>  M chrome/app/generated_resources.grd
>>  M chrome/browser/plugin_service.cc
>>  M chrome/browser/plugin_updater.cc
>>  M chrome/browser/tab_contents/tab_contents.cc
>>  M chrome/renderer/render_view.h
>>  M chrome/renderer/render_view.cc
>>
>>
>
>

[darin (slow to review), 2010-12-16 17:39:25]

http://codereview.chromium.org/5917001/diff/6001/chrome/renderer/render_view.cc
File chrome/renderer/render_view.cc (right):

http://codereview.chromium.org/5917001/diff/6001/chrome/renderer/render_view....
chrome/renderer/render_view.cc:2757: if
(cmd->HasSwitch(switches::kDisableOutdatedPlugins) &&
i think you should rename the command line switch to kBlockOutdatedPlugins

http://codereview.chromium.org/5917001/diff/6001/chrome/renderer/render_view....
chrome/renderer/render_view.cc:2759: Send(new
ViewHostMsg_DisabledOutdatedPlugin(routing_id_,
nit: change the IPC to BlockedOutdatedPlugin

[Bernhard Bauer, 2010-12-16 18:23:04]

http://codereview.chromium.org/5917001/diff/6001/chrome/renderer/render_view.cc
File chrome/renderer/render_view.cc (right):

http://codereview.chromium.org/5917001/diff/6001/chrome/renderer/render_view....
chrome/renderer/render_view.cc:2757: if
(cmd->HasSwitch(switches::kDisableOutdatedPlugins) &&
On 2010/12/16 17:39:25, darin wrote:
> i think you should rename the command line switch to kBlockOutdatedPlugins

Done.

http://codereview.chromium.org/5917001/diff/6001/chrome/renderer/render_view....
chrome/renderer/render_view.cc:2759: Send(new
ViewHostMsg_DisabledOutdatedPlugin(routing_id_,
On 2010/12/16 17:39:25, darin wrote:
> nit: change the IPC to BlockedOutdatedPlugin

Done.

[darin (slow to review), 2010-12-16 18:38:48]

LGTM