Hookup Gpu blacklist with NaCl Pepper3D access.

Hookup Gpu blacklist with NaCl Pepper3D access.

If WebGL is blacklisted, we should not allow NaCl to have Pepper 3D access.  However, we still want trusted Apps like Flash or Netflix player to go through.

BUG=93899
TEST=no NaCl Pepper 3D access if the driver/card is blacklisted for WebGL
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=99383

[Zhenyao Mo, 2011-08-30 00:06:37]

Please review.

[brettw, 2011-08-30 15:57:36]

webkit stuff LGTM, I've added Polina for the nacl proxy stuff.

http://codereview.chromium.org/7790016/diff/1/ppapi/c/private/ppb_gpu_blackli...
File ppapi/c/private/ppb_gpu_blacklist_private.h (right):

http://codereview.chromium.org/7790016/diff/1/ppapi/c/private/ppb_gpu_blackli...
ppapi/c/private/ppb_gpu_blacklist_private.h:14: * NativeClient applications
could access Pepper 3D or not.
could -> can

[polina, 2011-08-31 06:39:52]

http://codereview.chromium.org/7790016/diff/1/ppapi/native_client/src/shared/...
File
ppapi/native_client/src/shared/ppapi_proxy/browser_ppb_graphics_3d_rpc_server.cc
(right):

http://codereview.chromium.org/7790016/diff/1/ppapi/native_client/src/shared/...
ppapi/native_client/src/shared/ppapi_proxy/browser_ppb_graphics_3d_rpc_server.cc:82:
CHECK(module);
NaCl code usually goes for explicit != NULL and == NULL.

http://codereview.chromium.org/7790016/diff/1/ppapi/native_client/src/shared/...
ppapi/native_client/src/shared/ppapi_proxy/browser_ppb_graphics_3d_rpc_server.cc:85:
module->GetBrowserInterface(PPB_GPU_BLACKLIST_INTERFACE));
Can you switch to ppapi_proxy::GetBrowserInterface or even
GetBrowserInterfaceSafe (see browser_globals.h/cc for how we use them for other
interfaces)?

http://codereview.chromium.org/7790016/diff/1/ppapi/native_client/src/shared/...
ppapi/native_client/src/shared/ppapi_proxy/browser_ppb_graphics_3d_rpc_server.cc:87:
return true;  // TODO(zmo): should we return true or false here?
Is it normal for this private interface not to be available?

http://codereview.chromium.org/7790016/diff/1/ppapi/native_client/src/shared/...
ppapi/native_client/src/shared/ppapi_proxy/browser_ppb_graphics_3d_rpc_server.cc:215:
*resource_id = 0;
ppapi_proxy::kInvalidResourceId

[Zhenyao Mo, 2011-09-01 18:54:41]

Revised.  Please review again.

http://codereview.chromium.org/7790016/diff/1/ppapi/c/private/ppb_gpu_blackli...
File ppapi/c/private/ppb_gpu_blacklist_private.h (right):

http://codereview.chromium.org/7790016/diff/1/ppapi/c/private/ppb_gpu_blackli...
ppapi/c/private/ppb_gpu_blacklist_private.h:14: * NativeClient applications
could access Pepper 3D or not.
On 2011/08/30 15:57:36, brettw wrote:
> could -> can

Done.

http://codereview.chromium.org/7790016/diff/1/ppapi/native_client/src/shared/...
File
ppapi/native_client/src/shared/ppapi_proxy/browser_ppb_graphics_3d_rpc_server.cc
(right):

http://codereview.chromium.org/7790016/diff/1/ppapi/native_client/src/shared/...
ppapi/native_client/src/shared/ppapi_proxy/browser_ppb_graphics_3d_rpc_server.cc:82:
CHECK(module);
On 2011/08/31 06:39:53, polina wrote:
> NaCl code usually goes for explicit != NULL and == NULL.

No longer use module to access interface

http://codereview.chromium.org/7790016/diff/1/ppapi/native_client/src/shared/...
ppapi/native_client/src/shared/ppapi_proxy/browser_ppb_graphics_3d_rpc_server.cc:85:
module->GetBrowserInterface(PPB_GPU_BLACKLIST_INTERFACE));
On 2011/08/31 06:39:53, polina wrote:
> Can you switch to ppapi_proxy::GetBrowserInterface or even
> GetBrowserInterfaceSafe (see browser_globals.h/cc for how we use them for
other
> interfaces)?

Done.

http://codereview.chromium.org/7790016/diff/1/ppapi/native_client/src/shared/...
ppapi/native_client/src/shared/ppapi_proxy/browser_ppb_graphics_3d_rpc_server.cc:87:
return true;  // TODO(zmo): should we return true or false here?
On 2011/08/31 06:39:53, polina wrote:
> Is it normal for this private interface not to be available?

I was hoping I could just CHECK here, but examples in existing interfaces always
check the NULL case and return.  I have no idea when that may happen.

http://codereview.chromium.org/7790016/diff/1/ppapi/native_client/src/shared/...
ppapi/native_client/src/shared/ppapi_proxy/browser_ppb_graphics_3d_rpc_server.cc:215:
*resource_id = 0;
On 2011/08/31 06:39:53, polina wrote:
> ppapi_proxy::kInvalidResourceId

Done.

[polina, 2011-09-02 09:56:52]

LGTM

http://codereview.chromium.org/7790016/diff/1/ppapi/native_client/src/shared/...
File
ppapi/native_client/src/shared/ppapi_proxy/browser_ppb_graphics_3d_rpc_server.cc
(right):

http://codereview.chromium.org/7790016/diff/1/ppapi/native_client/src/shared/...
ppapi/native_client/src/shared/ppapi_proxy/browser_ppb_graphics_3d_rpc_server.cc:87:
return true;  // TODO(zmo): should we return true or false here?
On 2011/09/01 18:54:41, Zhenyao Mo wrote:
> On 2011/08/31 06:39:53, polina wrote:
> > Is it normal for this private interface not to be available?
> 
> I was hoping I could just CHECK here, but examples in existing interfaces
always
> check the NULL case and return.  I have no idea when that may happen.

In most other cases use GetBrowserInterfaceSafe which is equal to
GetBrowserInterface + CHECK. I would suggest using that.

[polina, 2011-09-02 09:58:25]

LGTM

(after the comment is addressed)

http://codereview.chromium.org/7790016/diff/9002/ppapi/native_client/src/shar...
File
ppapi/native_client/src/shared/ppapi_proxy/browser_ppb_graphics_3d_rpc_server.cc
(right):

http://codereview.chromium.org/7790016/diff/9002/ppapi/native_client/src/shar...
ppapi/native_client/src/shared/ppapi_proxy/browser_ppb_graphics_3d_rpc_server.cc:83:
if (!interface)
And if you decide against using GetBrowserInterfaceSafe, please apply the style
consistency  request from earlier here and use explicit NULL comparison. Thanks.