Only register the newer one of {bundled, component} Pepper Flash.

Only register the newer one of {bundled, component} Pepper Flash.


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

[Bernhard Bauer, 2012-10-09 13:52:01]

Please review.

[cpu_(ooo_6.6-7.5), 2012-10-09 20:00:36]

http://codereview.chromium.org/11090018/diff/1/chrome/browser/component_updat...
File chrome/browser/component_updater/pepper_flash_component_installer.cc
(right):

http://codereview.chromium.org/11090018/diff/1/chrome/browser/component_updat...
chrome/browser/component_updater/pepper_flash_component_installer.cc:169:
(plugin.pepper_permissions & ppapi::PERMISSION_FLASH);
can you add a comment here about this check. I am unfamiliar with this
permission thing.

http://codereview.chromium.org/11090018/diff/1/chrome/browser/component_updat...
chrome/browser/component_updater/pepper_flash_component_installer.cc:184:
continue;
is this doing IO? do we want to call this instead from line 333 ? 

I assume that the caller of line 258 is not the issue. Is it?

[Bernhard Bauer, 2012-10-09 20:52:39]

http://codereview.chromium.org/11090018/diff/1/chrome/browser/component_updat...
File chrome/browser/component_updater/pepper_flash_component_installer.cc
(right):

http://codereview.chromium.org/11090018/diff/1/chrome/browser/component_updat...
chrome/browser/component_updater/pepper_flash_component_installer.cc:169:
(plugin.pepper_permissions & ppapi::PERMISSION_FLASH);
On 2012/10/09 20:00:36, cpu wrote:
> can you add a comment here about this check. I am unfamiliar with this
> permission thing.

Will do. I could also use a different criterion to look for Pepper Flash, but
this seems like the most stable one.

http://codereview.chromium.org/11090018/diff/1/chrome/browser/component_updat...
chrome/browser/component_updater/pepper_flash_component_installer.cc:184:
continue;
On 2012/10/09 20:00:36, cpu wrote:
> is this doing IO? do we want to call this instead from line 333 ?

No, it just asks PluginList for its internal plugins (for which we don't need to
hit the disk) and then looks at the returned structs.

> I assume that the caller of line 258 is not the issue. Is it?

Not in this particular manifestation of the bug, but seeing as we have two
completely independent channels here to deploy Flapper, I just wanted to make
sure we do The Right Thing in every case, which is only loading the newer
plugin.

[cpu_(ooo_6.6-7.5), 2012-10-09 22:32:40]

lgtm

[commit-bot: I haz the power, 2012-10-10 07:54:40]

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

[commit-bot: I haz the power, 2012-10-10 07:54:49]

Presubmit check for 11090018-8001 failed and returned exit status 1.


Running presubmit commit checks ...

** Presubmit ERRORS **
Missing LGTM from an OWNER for files in these directories:
    content/browser
    content/public/browser

[Bernhard Bauer, 2012-10-10 08:03:53]

Hey John,

can I get an OWNERS review for content/?

[jam, 2012-10-10 19:23:48]

lgtm

[commit-bot: I haz the power, 2012-10-10 20:03:27]

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

[commit-bot: I haz the power, 2012-10-10 22:10:29]

Change committed as 161210