Add UMA histogram to determine how often Stage3D could be used.

Add UMA histogram to determine how often Stage3D could be used.


BUG=

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=153887

[jbauman, 2012-08-27 20:59:32]

I think this is correct, but I'm not certain exactly how wmode is supposed to
work.

[brettw, 2012-08-27 22:19:28]

lgtm

https://chromiumcodereview.appspot.com/10875077/diff/1/webkit/plugins/ppapi/p...
File webkit/plugins/ppapi/ppapi_plugin_instance.cc (right):

https://chromiumcodereview.appspot.com/10875077/diff/1/webkit/plugins/ppapi/p...
webkit/plugins/ppapi/ppapi_plugin_instance.cc:549: #if defined(OS_WIN)
Can you split this into a separate function?

https://chromiumcodereview.appspot.com/10875077/diff/1/webkit/plugins/ppapi/p...
webkit/plugins/ppapi/ppapi_plugin_instance.cc:554: for (unsigned int i = 0; i <
arg_names.size(); i++) {
Use size_t since that's the type that vector.size() returns.

https://chromiumcodereview.appspot.com/10875077/diff/1/webkit/plugins/ppapi/p...
webkit/plugins/ppapi/ppapi_plugin_instance.cc:561:
UMA_HISTOGRAM_ENUMERATION("Flash.UsesGPU",
Can you add a comment here about what this histogram contains? The way you're
using it to communicate capabilities is "nontraditional" :)

[vangelis, 2012-08-28 05:12:06]

https://chromiumcodereview.appspot.com/10875077/diff/1/webkit/plugins/ppapi/p...
File webkit/plugins/ppapi/ppapi_plugin_instance.cc (right):

https://chromiumcodereview.appspot.com/10875077/diff/1/webkit/plugins/ppapi/p...
webkit/plugins/ppapi/ppapi_plugin_instance.cc:552: bool needsgpu = false;
nit: according to chrome's variable naming convention this should be needs_gpu

https://chromiumcodereview.appspot.com/10875077/diff/1/webkit/plugins/ppapi/p...
webkit/plugins/ppapi/ppapi_plugin_instance.cc:562: needsgpu * 2 +
prefs.is_webgl_supported, 4);
Is this bool to int conversion pretty standard and supported the same way across
all platforms?

[jbauman, 2012-08-28 16:52:18]

Okay, uploaded a version with more comments, and where it was split into a
separate function.

On 2012/08/28 05:12:06, vangelis wrote:
>
https://chromiumcodereview.appspot.com/10875077/diff/1/webkit/plugins/ppapi/p...
> File webkit/plugins/ppapi/ppapi_plugin_instance.cc (right):
> 
>
https://chromiumcodereview.appspot.com/10875077/diff/1/webkit/plugins/ppapi/p...
> webkit/plugins/ppapi/ppapi_plugin_instance.cc:552: bool needsgpu = false;
> nit: according to chrome's variable naming convention this should be needs_gpu
> 
>
https://chromiumcodereview.appspot.com/10875077/diff/1/webkit/plugins/ppapi/p...
> webkit/plugins/ppapi/ppapi_plugin_instance.cc:562: needsgpu * 2 +
> prefs.is_webgl_supported, 4);
> Is this bool to int conversion pretty standard and supported the same way
across
> all platforms?
The spec defines true to be 1.

[vangelis, 2012-08-28 17:30:23]

https://chromiumcodereview.appspot.com/10875077/diff/6001/webkit/plugins/ppap...
File webkit/plugins/ppapi/ppapi_plugin_instance.cc (right):

https://chromiumcodereview.appspot.com/10875077/diff/6001/webkit/plugins/ppap...
webkit/plugins/ppapi/ppapi_plugin_instance.cc:558: // card, and 2 or 3 if the
content needs the GPU.
Maybe it would be more clear to enumerate the potential results:
0 : No 3D content and GPU is blacklisted
1 : No 3D content and GPU is not blacklisted
2 : 3D content but GPU is blacklisted
3 : 3D content and GPU is not blacklisted

Also, since you're in here, should we do this for other versions of windows?  It
would be useful to know where we stand there too.

[jbauman, 2012-08-28 17:39:39]

On 2012/08/28 17:30:23, vangelis wrote:
>
https://chromiumcodereview.appspot.com/10875077/diff/6001/webkit/plugins/ppap...
> File webkit/plugins/ppapi/ppapi_plugin_instance.cc (right):
> 
>
https://chromiumcodereview.appspot.com/10875077/diff/6001/webkit/plugins/ppap...
> webkit/plugins/ppapi/ppapi_plugin_instance.cc:558: // card, and 2 or 3 if the
> content needs the GPU.
> Maybe it would be more clear to enumerate the potential results:
> 0 : No 3D content and GPU is blacklisted
> 1 : No 3D content and GPU is not blacklisted
> 2 : 3D content but GPU is blacklisted
> 3 : 3D content and GPU is not blacklisted
> 
> Also, since you're in here, should we do this for other versions of windows? 
It
> would be useful to know where we stand there too.

Ok, sure.