Whitelist Widevine CDM for plugin UMA on all platforms.

Whitelist Widevine CDM for plugin UMA on all platforms.

Previously this is only whitelisted on Linux/ChromeOS. This CL relax this to
all platforms.

BUG=413812
TEST=Tested on Windows.

Committed: https://crrev.com/f7e0e528982ff057fde0de1cfff2103cc8e82ea0
Cr-Commit-Position: refs/heads/master@{#294663}

[xhwang, 2014-09-12 05:37:33]

xhwang@chromium.org changed reviewers:
+ ddorwin@chromium.org, teravest@chromium.org

[ddorwin, 2014-09-12 05:47:14]

https://codereview.chromium.org/567913002/diff/1/chrome/renderer/pepper/peppe...
File chrome/renderer/pepper/pepper_uma_host.cc (right):

https://codereview.chromium.org/567913002/diff/1/chrome/renderer/pepper/peppe...
chrome/renderer/pepper/pepper_uma_host.cc:36: "widevinecdmadapter",  //
http://crbug.com/368743, http://crbug.com/410630
How does this work on Linux? Don't you need the "lib" part?

[xhwang, 2014-09-12 06:39:57]

On 2014/09/12 05:47:14, ddorwin wrote:
>
https://codereview.chromium.org/567913002/diff/1/chrome/renderer/pepper/peppe...
> File chrome/renderer/pepper/pepper_uma_host.cc (right):
> 
>
https://codereview.chromium.org/567913002/diff/1/chrome/renderer/pepper/peppe...
> chrome/renderer/pepper/pepper_uma_host.cc:36: "widevinecdmadapter",  //
> http://crbug.com/368743, http://crbug.com/410630
> How does this work on Linux? Don't you need the "lib" part?

Sorry, this is not finished yet. We need to search and match some string in the
plugin_base_name_.

[xhwang, 2014-09-12 07:36:05]

Now it's ready for review. PTAL!

[ddorwin, 2014-09-12 16:11:08]

https://codereview.chromium.org/567913002/diff/20001/chrome/renderer/pepper/p...
File chrome/renderer/pepper/pepper_uma_host.cc (right):

https://codereview.chromium.org/567913002/diff/20001/chrome/renderer/pepper/p...
chrome/renderer/pepper/pepper_uma_host.cc:39: "*widevinecdmadapter.*",  // see
http://crbug.com/368743 and
How good is the pattern matching? Can we do something like [lib]wide...?

Is this better than just having some platform-specific code or using a constant
we already have?

https://codereview.chromium.org/567913002/diff/20001/chrome/renderer/pepper/p...
chrome/renderer/pepper/pepper_uma_host.cc:107:
std::set<std::string>::const_iterator iter =
Why is this outside the for loop?

[xhwang, 2014-09-12 16:33:58]

comments only

https://codereview.chromium.org/567913002/diff/20001/chrome/renderer/pepper/p...
File chrome/renderer/pepper/pepper_uma_host.cc (right):

https://codereview.chromium.org/567913002/diff/20001/chrome/renderer/pepper/p...
chrome/renderer/pepper/pepper_uma_host.cc:39: "*widevinecdmadapter.*",  // see
http://crbug.com/368743 and
On 2014/09/12 16:11:08, ddorwin wrote:
> How good is the pattern matching? Can we do something like [lib]wide...?

No, it only supports * and ?. Probably I can find some regex function but felt
that may be overkill...
 
> Is this better than just having some platform-specific code or using a
constant
> we already have?

I debated about that myself :) I just didn't feel like putting all three
versions of widevine cdm adapter file names in this file, which is not scalable
(imagine someone else wants to whitelist another plugins...).

I also looked at GetNativeLibraryName():
https://code.google.com/p/chromium/codesearch#chromium/src/base/native_librar...

There are a few issues:
1, it's .dylib instead of .plugin
2, We still need to support the case where we only want to whitelist a plugin on
certain platforms, e.g. libpdf.so on linux only.

https://codereview.chromium.org/567913002/diff/20001/chrome/renderer/pepper/p...
chrome/renderer/pepper/pepper_uma_host.cc:107:
std::set<std::string>::const_iterator iter =
On 2014/09/12 16:11:08, ddorwin wrote:
> Why is this outside the for loop?

I hated the long line...  will fix :) ~~

[xhwang, 2014-09-12 17:40:57]

Per comments and offline discussion, I dropped the pattern matching approach.

I updated the CL to use kWidevineCdmAdapterFileName instead. The renderer target
in chrome_renderer.gypi is already depending on
../third_party/widevine/cdm/widevine_cdm.gyp:widevine_cdm_version_h, so we kind
of get this for free:
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/chrome_rend...

PTAL again!

[teravest, 2014-09-12 17:46:35]

lgtm

https://codereview.chromium.org/567913002/diff/60001/chrome/renderer/pepper/p...
File chrome/renderer/pepper/pepper_uma_host.cc (right):

https://codereview.chromium.org/567913002/diff/60001/chrome/renderer/pepper/p...
chrome/renderer/pepper/pepper_uma_host.cc:24: #include "widevine_cdm_version.h" 
// In SHARED_INTERMEDIATE_DIR.
I checked the gyp dependencies for this; looks OK.

[ddorwin, 2014-09-12 20:13:43]

Thanks. I like this better. LGTM.

[xhwang, 2014-09-12 20:17:30]

The CQ bit was checked by xhwang@chromium.org

[commit-bot: I haz the power, 2014-09-12 20:21:46]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patchset/567913002/60001

[commit-bot: I haz the power, 2014-09-12 21:29:45]

Committed patchset #4 (id:60001) as 96a448b4198c15b0a5fe8805f4f4c8af1f3fd409

[commit-bot: I haz the power, 2014-09-12 21:35:54]

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