Move version metadata from PluginGroup into PluginInstaller.

Move version metadata from PluginGroup into PluginInstaller.

This also moves the plug-in information JSON files into their own subdirectory, so they can be changed without requiring OWNERS reviews for chrome/browser/resources (which is mostly WebUI otherwise).

BUG=124396
TEST=PluginInstallerTest.SecurityStatus:PluginFinderTest.JsonSyntax


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

[Bernhard Bauer, 2012-05-18 15:25:57]

Please review. 

John: chrome/browser/ui/pdf (OWNERS) and plug-in stuff
James: chrome/browser/ui/webui and chrome/browser/resources (OWNERS)
Jochen: rest :)

Thanks!

http://codereview.chromium.org/10263022/diff/47001/chrome/browser/ui/pdf/pdf_...
File chrome/browser/ui/pdf/pdf_unsupported_feature.cc (left):

http://codereview.chromium.org/10263022/diff/47001/chrome/browser/ui/pdf/pdf_...
chrome/browser/ui/pdf/pdf_unsupported_feature.cc:299: if (!reader_vulnerable_) {
I removed this and instead included NEEDS_AUTHORIZATION in the set of statuses
that are considered vulnerable, as we infobar Reader below X.

[James Hawkins, 2012-05-18 16:01:47]

webui/ and resources/ LGTM

[jochen (gone - plz use gerrit), 2012-05-18 18:48:18]

the diff looks like the json files are changed as well?

http://codereview.chromium.org/10263022/diff/47001/chrome/browser/plugin_find...
File chrome/browser/plugin_finder.cc (right):

http://codereview.chromium.org/10263022/diff/47001/chrome/browser/plugin_find...
chrome/browser/plugin_finder.cc:155:
PluginInstaller::SECURITY_STATUS_UP_TO_DATE;
would it be safer to default to not up to date, as a parse error would indicate
that something is fishy?

http://codereview.chromium.org/10263022/diff/47001/chrome/browser/plugin_info...
File chrome/browser/plugin_infobar_delegates.cc (left):

http://codereview.chromium.org/10263022/diff/47001/chrome/browser/plugin_info...
chrome/browser/plugin_infobar_delegates.cc:153: case
PluginInstaller::kStateIdle:
any reason you change this instead of making the security state constants k...
as well? Especially since in the test you re-define the security state contants
to k... again?

http://codereview.chromium.org/10263022/diff/47001/chrome/browser/plugin_inst...
File chrome/browser/plugin_installer.cc (right):

http://codereview.chromium.org/10263022/diff/47001/chrome/browser/plugin_inst...
chrome/browser/plugin_installer.cc:99: // |lower_bound| returns the newest
version that is not newer than |version|.
latest version?

http://codereview.chromium.org/10263022/diff/47001/chrome/browser/renderer_ho...
File chrome/browser/renderer_host/plugin_info_message_filter.cc (right):

http://codereview.chromium.org/10263022/diff/47001/chrome/browser/renderer_ho...
chrome/browser/renderer_host/plugin_info_message_filter.cc:161: #if
defined(ENABLE_PLUGIN_INSTALLATION)
when is this not defined? It seems a bit odd to not warn of known to be
exploitable plugins, just because we don't offer a way to update from within the
browser

[jam, 2012-05-18 23:19:34]

lgtm with 2 nits as long as you test the pdf case to make sure the behavior is
the same (hard for me to verify by looking at the diff)

http://codereview.chromium.org/10263022/diff/47001/chrome/browser/resources/p...
File chrome/browser/resources/plugin_metadata/OWNERS (right):

http://codereview.chromium.org/10263022/diff/47001/chrome/browser/resources/p...
chrome/browser/resources/plugin_metadata/OWNERS:1: set noparent
no "set noparent" in owners files per cr-dev thread

http://codereview.chromium.org/10263022/diff/47001/chrome/browser/ui/pdf/pdf_...
File chrome/browser/ui/pdf/pdf_unsupported_feature.cc (right):

http://codereview.chromium.org/10263022/diff/47001/chrome/browser/ui/pdf/pdf_...
chrome/browser/ui/pdf/pdf_unsupported_feature.cc:298: CHECK(installer);
nit: no need for this. if it's null, the next line will crash

[Bernhard Bauer, 2012-05-21 10:52:27]

On 2012/05/18 18:48:18, jochen wrote:
> the diff looks like the json files are changed as well?

Yeah, they now contain the version information that used to be in PluginGroup.

http://codereview.chromium.org/10263022/diff/47001/chrome/browser/plugin_find...
File chrome/browser/plugin_finder.cc (right):

http://codereview.chromium.org/10263022/diff/47001/chrome/browser/plugin_find...
chrome/browser/plugin_finder.cc:155:
PluginInstaller::SECURITY_STATUS_UP_TO_DATE;
On 2012/05/18 18:48:18, jochen wrote:
> would it be safer to default to not up to date, as a parse error would
indicate
> that something is fishy?

A parse error is one of those "should never happen" situations, as we control
the file, so I'm just initializing the value to zero.

http://codereview.chromium.org/10263022/diff/47001/chrome/browser/plugin_info...
File chrome/browser/plugin_infobar_delegates.cc (left):

http://codereview.chromium.org/10263022/diff/47001/chrome/browser/plugin_info...
chrome/browser/plugin_infobar_delegates.cc:153: case
PluginInstaller::kStateIdle:
On 2012/05/18 18:48:18, jochen wrote:
> any reason you change this instead of making the security state constants k...
> as well? Especially since in the test you re-define the security state
contants
> to k... again?

The Chromium style (http://dev.chromium.org/developers/coding-style#Naming) says
to use MACRO_STYLE for enums (and in the test I need to redeclare them anyway to
get rid of the PluginInstaller:: prefix).

http://codereview.chromium.org/10263022/diff/47001/chrome/browser/plugin_inst...
File chrome/browser/plugin_installer.cc (right):

http://codereview.chromium.org/10263022/diff/47001/chrome/browser/plugin_inst...
chrome/browser/plugin_installer.cc:99: // |lower_bound| returns the newest
version that is not newer than |version|.
On 2012/05/18 18:48:18, jochen wrote:
> latest version?

Done.

http://codereview.chromium.org/10263022/diff/47001/chrome/browser/renderer_ho...
File chrome/browser/renderer_host/plugin_info_message_filter.cc (right):

http://codereview.chromium.org/10263022/diff/47001/chrome/browser/renderer_ho...
chrome/browser/renderer_host/plugin_info_message_filter.cc:161: #if
defined(ENABLE_PLUGIN_INSTALLATION)
On 2012/05/18 18:48:18, jochen wrote:
> when is this not defined? It seems a bit odd to not warn of known to be
> exploitable plugins, just because we don't offer a way to update from within
the
> browser

It's not defined on ChromeOS/Aura (and it should not be defined on Android; at
the moment it is, but that's a bug).

In general, it's not defined on platforms that don't support third-party
plug-ins at all, so there are never going to be known outdated plug-ins.

http://codereview.chromium.org/10263022/diff/47001/chrome/browser/resources/p...
File chrome/browser/resources/plugin_metadata/OWNERS (right):

http://codereview.chromium.org/10263022/diff/47001/chrome/browser/resources/p...
chrome/browser/resources/plugin_metadata/OWNERS:1: set noparent
On 2012/05/18 23:19:34, John Abd-El-Malek wrote:
> no "set noparent" in owners files per cr-dev thread

Done.

http://codereview.chromium.org/10263022/diff/47001/chrome/browser/ui/pdf/pdf_...
File chrome/browser/ui/pdf/pdf_unsupported_feature.cc (right):

http://codereview.chromium.org/10263022/diff/47001/chrome/browser/ui/pdf/pdf_...
chrome/browser/ui/pdf/pdf_unsupported_feature.cc:298: CHECK(installer);
On 2012/05/18 23:19:34, John Abd-El-Malek wrote:
> nit: no need for this. if it's null, the next line will crash

Done.

[jochen (gone - plz use gerrit), 2012-05-21 10:56:15]

lgtm

[commit-bot: I haz the power, 2012-05-21 10:57:22]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/bauerb@chromium.org/10263022/51001

[commit-bot: I haz the power, 2012-05-21 13:11:31]

Try job failure for 10263022-51001 (retry) on win for step "compile" (clobber
build).
It's a second try, previously, step "compile" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win&number...

[commit-bot: I haz the power, 2012-05-21 13:31:24]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/bauerb@chromium.org/10263022/51001

[commit-bot: I haz the power, 2012-05-21 14:49:27]

Try job failure for 10263022-51001 (retry) on linux_rel for step
"browser_tests".
It's a second try, previously, step "browser_tests" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_rel&...

[commit-bot: I haz the power, 2012-05-21 14:55:44]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/bauerb@chromium.org/10263022/51001

[commit-bot: I haz the power, 2012-05-21 18:27:26]

Try job failure for 10263022-51001 (retry) on win_rel for steps "base_unittests,
sync_unit_tests".
It's a second try, previously, steps "base_unittests, sync_unit_tests" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[commit-bot: I haz the power, 2012-05-21 19:55:51]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/bauerb@chromium.org/10263022/51001

[commit-bot: I haz the power, 2012-05-22 01:55:27]

Try job failure for 10263022-51001 (retry) on win_rel for steps "base_unittests,
sync_unit_tests".
It's a second try, previously, steps "base_unittests, browser_tests,
sync_unit_tests" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[commit-bot: I haz the power, 2012-05-22 16:05:08]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/bauerb@chromium.org/10263022/51001

[commit-bot: I haz the power, 2012-05-22 17:01:13]

Try job failure for 10263022-51001 (retry) on win for step "compile" (clobber
build).
It's a second try, previously, step "compile" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win&number...

[commit-bot: I haz the power, 2012-05-22 20:29:50]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/bauerb@chromium.org/10263022/51001

[commit-bot: I haz the power, 2012-05-23 01:03:58]

Try job failure for 10263022-51001 (retry) on win_rel for step
"sync_unit_tests".
It's a second try, previously, step "sync_unit_tests" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[commit-bot: I haz the power, 2012-05-23 10:45:11]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/bauerb@chromium.org/10263022/51001

[commit-bot: I haz the power, 2012-05-23 14:43:23]

Try job failure for 10263022-51001 (retry) on win_rel for step
"sync_unit_tests".
It's a second try, previously, step "sync_unit_tests" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[commit-bot: I haz the power, 2012-05-23 14:48:43]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/bauerb@chromium.org/10263022/51001

[commit-bot: I haz the power, 2012-05-23 17:10:45]

Change committed as 138502