1st CL in a series to remove PluginGroup. Updates PluginList.

1st CL in a series to remove PluginGroup. Updates PluginList.

This is the first CL in a series that aims at removing PluginGroup class and moving harded coded group definition data to .json config files. This CL updates PluginList class with new methods not relying on PluginGroup class, these methods are to be called by upper layer methods instead of PluginGroup dependent methods. Changes in upper layer will be introduced in upcoming CLs. The last CL in the series should remove PluginGroup class and all its dependencies.


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

[ibraaaa, 2012-08-20 17:09:50]

This CL updates PluginList class only. For all methods with signature taking a
PluginGroup, another method is added to take only WebPluginInfo. For all methods
whose internal behavior relied on PluginGroup, these were changed to use a list
of WebPluginInfo without damaging the logic for PluginGroup. Thus at any point
of time the net result is that PluginList class should be holding two lists one
of PluginGroups and the other of WebPluginInfo and both are in the same state.

In a later CL, after eliminating all dependencies on PluginGroup related
methods, these methods will be deleted to end up of PluginList holding a list of
WebPluginInfo only.

[Bernhard Bauer, 2012-08-20 17:22:42]

Looks very good generally, just some nits below, and with the CL description:

* Could you add a short first line to the CL description? The first line is
displayed in a lots of places, so having a short description makes it a bit more
readable.
* Please also add BUG=124396 to the description.

https://chromiumcodereview.appspot.com/10860044/diff/1/webkit/plugins/npapi/p...
File webkit/plugins/npapi/plugin_list.cc (right):

https://chromiumcodereview.appspot.com/10860044/diff/1/webkit/plugins/npapi/p...
webkit/plugins/npapi/plugin_list.cc:368: ScopedVector<PluginGroup>
new_plugin_groups; // TODO(ibraaaa): DELETE
Nit: two spaces before comment. Same below.

https://chromiumcodereview.appspot.com/10860044/diff/1/webkit/plugins/npapi/p...
File webkit/plugins/npapi/plugin_list.h (right):

https://chromiumcodereview.appspot.com/10860044/diff/1/webkit/plugins/npapi/p...
webkit/plugins/npapi/plugin_list.h:151: // TODO(ibraaaa): DELETE
Could you add a reference to http://crbug.com/124396 in these comments?

https://chromiumcodereview.appspot.com/10860044/diff/1/webkit/plugins/npapi/p...
webkit/plugins/npapi/plugin_list.h:286: // Removes a plug-in from plugins_list_
by its path.
Nit: Put |plugins_list_| between pipe symbols?

[ibraaaa, 2012-08-20 18:05:28]

Done, PTAL.

https://chromiumcodereview.appspot.com/10860044/diff/1/webkit/plugins/npapi/p...
File webkit/plugins/npapi/plugin_list.cc (right):

https://chromiumcodereview.appspot.com/10860044/diff/1/webkit/plugins/npapi/p...
webkit/plugins/npapi/plugin_list.cc:368: ScopedVector<PluginGroup>
new_plugin_groups; // TODO(ibraaaa): DELETE
On 2012/08/20 17:22:42, Bernhard Bauer wrote:
> Nit: two spaces before comment. Same below.

Done.

https://chromiumcodereview.appspot.com/10860044/diff/1/webkit/plugins/npapi/p...
File webkit/plugins/npapi/plugin_list.h (right):

https://chromiumcodereview.appspot.com/10860044/diff/1/webkit/plugins/npapi/p...
webkit/plugins/npapi/plugin_list.h:151: // TODO(ibraaaa): DELETE
On 2012/08/20 17:22:42, Bernhard Bauer wrote:
> Could you add a reference to http://crbug.com/124396 in these comments?

Done.

https://chromiumcodereview.appspot.com/10860044/diff/1/webkit/plugins/npapi/p...
webkit/plugins/npapi/plugin_list.h:286: // Removes a plug-in from plugins_list_
by its path.
On 2012/08/20 17:22:42, Bernhard Bauer wrote:
> Nit: Put |plugins_list_| between pipe symbols?

Done.

[Bernhard Bauer, 2012-08-20 18:15:12]

LGTM!

[commit-bot: I haz the power, 2012-08-20 18:15:42]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/ibraaaa@google.com/10860044/10

[commit-bot: I haz the power, 2012-08-20 18:15:49]

Presubmit check for 10860044-10 failed and returned exit status 1.


Running presubmit commit checks ...

** Presubmit Warnings **
New code should not use wstrings.  If you are calling a cross-platform API that
accepts a wstring, fix the API.
    webkit/plugins/npapi/plugin_list_win.cc:468
    webkit/plugins/npapi/plugin_list_win.cc:489

Presubmit checks took 1.3s to calculate.

[commit-bot: I haz the power, 2012-08-20 18:24:24]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/ibraaaa@google.com/10860044/8002

[commit-bot: I haz the power, 2012-08-20 18:48:12]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/ibraaaa@google.com/10860044/3004

[commit-bot: I haz the power, 2012-08-20 19:08:38]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/ibraaaa@google.com/10860044/2005

[commit-bot: I haz the power, 2012-08-20 22:14:09]

Change committed as 152399