Added infrastructure to run gpu demos under Pepper3D. Created a Demo class th...

Added infrastructure to run gpu demos under Pepper3D. Created a Demo class that can be run as both standalone apps (exe) or pepper plugins (dll). Created entry points and framework classes for both platforms - exe and pepper. A demo application has three layers:
1. Entry point
   - standalone (main_exe.cc): Contains the main function for console applications. Instantiates and runs a window.
   - pepper (main_pepper.cc): Contains NPAPI entry points. Instantiates pepper plugin object.
2. Framework that hosts demo
   - standalone: Window
   - pepper: Plugin
3. Demo - One demo class for all platforms. This does all the rendering and event handling.

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

[alokp, 2010-01-22 18:11:43]

Note that I have only included hello-triangle demo in the build. I will add
other demos in the next CL.

[apatrick, 2010-01-22 19:20:00]

http://codereview.chromium.org/554053/diff/1/15
File gpu/demos/framework/demo.h (right):

http://codereview.chromium.org/554053/diff/1/15#newcode26
gpu/demos/framework/demo.h:26: virtual const TCHAR* Title() const = 0;
This is a windows specific type. I think just wchar_t. Chrome generally uses
Unicode.

http://codereview.chromium.org/554053/diff/1/15#newcode30
gpu/demos/framework/demo.h:30: // This function is called by the framework to
initialize the OpenGL state
Nit: newline above?

http://codereview.chromium.org/554053/diff/1/15#newcode42
gpu/demos/framework/demo.h:42: inline int width() const { return width_; }
inline is redundant.

http://codereview.chromium.org/554053/diff/1/15#newcode59
gpu/demos/framework/demo.h:59: base::Time last_draw_time_;
NaCl + base problem again. I'll talk to sehr about this.

http://codereview.chromium.org/554053/diff/1/8
File gpu/demos/framework/demo_factory.h (right):

http://codereview.chromium.org/554053/diff/1/8#newcode1
gpu/demos/framework/demo_factory.h:1: // Copyright (c) 2006-2009 The Chromium
Authors. All rights reserved.
2010

http://codereview.chromium.org/554053/diff/1/8#newcode13
gpu/demos/framework/demo_factory.h:13: extern Demo* CreateDemo();
This looks like a circular dependency of the framework library on the demo
itself. Is there a way to avoid it?

http://codereview.chromium.org/554053/diff/1/11
File gpu/demos/framework/main_pepper.cc (right):

http://codereview.chromium.org/554053/diff/1/11#newcode8
gpu/demos/framework/main_pepper.cc:8: #include "webkit/glue/plugins/nphostapi.h"
This'll probably be trouble for NaCl demos too but I don't think it needs to be
addressed in this CL.

http://codereview.chromium.org/554053/diff/1/11#newcode18
gpu/demos/framework/main_pepper.cc:18: if (browser->version >= 14) {
Should it return an error if the version is insufficient. There should be a
version constant defined as 14 somewhere.

http://codereview.chromium.org/554053/diff/1/11#newcode92
gpu/demos/framework/main_pepper.cc:92: browser->retainobject((NPObject*)plugin);
See comment in plugin.h about deriving from NPObject.

http://codereview.chromium.org/554053/diff/1/11#newcode97
gpu/demos/framework/main_pepper.cc:97: fprintf(stderr, "Unhandled variable to
NPP_GetValue\n");
Why not the regular logging macros? Aside from them being in base of course.

http://codereview.chromium.org/554053/diff/1/11#newcode114
gpu/demos/framework/main_pepper.cc:114: plugin_funcs->version = 11;
Use a version constant.

http://codereview.chromium.org/554053/diff/1/10
File gpu/demos/framework/plugin.cc (right):

http://codereview.chromium.org/554053/diff/1/10#newcode12
gpu/demos/framework/plugin.cc:12: static NPExtensions* extensions = NULL;
static is redundant. I think the convention for globals is a g_ prefix.

http://codereview.chromium.org/554053/diff/1/10#newcode16
gpu/demos/framework/plugin.cc:16: static NPObject* PluginAllocate(NPP npp,
NPClass* the_class) {
None of the statics are necessary in an anonymous namespace.

http://codereview.chromium.org/554053/diff/1/10#newcode18
gpu/demos/framework/plugin.cc:18: return reinterpret_cast<NPObject*>(plugin);
reinterpret_cast not needed if plugin derives from NPObject.

http://codereview.chromium.org/554053/diff/1/10#newcode22
gpu/demos/framework/plugin.cc:22: Plugin* plugin =
reinterpret_cast<Plugin*>(header);
can become static_cast

http://codereview.chromium.org/554053/diff/1/10#newcode43
gpu/demos/framework/plugin.cc:43: INT32_TO_NPVARIANT(1, *result);
Maybe VOID_TO_NPVARIANT would be a better default.

http://codereview.chromium.org/554053/diff/1/10#newcode107
gpu/demos/framework/plugin.cc:107: reinterpret_cast<void*>(&extensions));
I think the cast is redundant.

http://codereview.chromium.org/554053/diff/1/12
File gpu/demos/framework/plugin.h (right):

http://codereview.chromium.org/554053/diff/1/12#newcode36
gpu/demos/framework/plugin.h:36: // This class object needs to be safely casted
to NPObject*. To accomplish
You can just make Plugin derive from NPObject. Then you can have virtual
functions and you don't need the reinterpret casts or header().

http://codereview.chromium.org/554053/diff/1/12#newcode50
gpu/demos/framework/plugin.h:50: base::AtExitManager at_exit_manager_;
There should only be one of these per thread. That won't be the case here if
there are multiple plugin instances.

http://codereview.chromium.org/554053/diff/1/12#newcode55
gpu/demos/framework/plugin.h:55: extern NPNetscapeFuncs* browser;
g_ prefix. Is there any way to localize this variable to a single .cc file?

http://codereview.chromium.org/554053/diff/1/17
File gpu/demos/framework/window.cc (right):

http://codereview.chromium.org/554053/diff/1/17#newcode22
gpu/demos/framework/window.cc:22: static const int32 kTransferBufferSize = 512 *
1024;
These statics are redundant.

http://codereview.chromium.org/554053/diff/1/17#newcode53
gpu/demos/framework/window.cc:53: wnd_class.lpfnWndProc = (WNDPROC)WindowProc;
This cast should be redundant if the signature of WindowProc is right.

http://codereview.chromium.org/554053/diff/1/17#newcode55
gpu/demos/framework/window.cc:55: wnd_class.hbrBackground =
(HBRUSH)GetStockObject(BLACK_BRUSH);
static_cast or reinterpret_cast

http://codereview.chromium.org/554053/diff/1/17#newcode149
gpu/demos/framework/window.cc:149: while (!done) {
I am still surprised that you are able to implement your own message loop.
GPUProcessor uses base::MessageLoop::PostTask if it ever needs to process more
than a certain number of GL calls after a flush.

http://codereview.chromium.org/554053/diff/1/3
File gpu/demos/gles2_book/example.h (right):

http://codereview.chromium.org/554053/diff/1/3#newcode31
gpu/demos/gles2_book/example.h:31: template <typename UserData>
This is much better.

http://codereview.chromium.org/554053/diff/1/4
File gpu/demos/gles2_book/hello_triangle.cc (right):

http://codereview.chromium.org/554053/diff/1/4#newcode1
gpu/demos/gles2_book/hello_triangle.cc:1: // Copyright (c) 2009 The Chromium
Authors. All rights reserved.
2010

http://codereview.chromium.org/554053/diff/1/4#newcode24
gpu/demos/gles2_book/hello_triangle.cc:24: const TCHAR* Title() const {
wchar_t

[alokp, 2010-01-22 20:46:06]

http://codereview.chromium.org/554053/diff/1/15
File gpu/demos/framework/demo.h (right):

http://codereview.chromium.org/554053/diff/1/15#newcode26
gpu/demos/framework/demo.h:26: virtual const TCHAR* Title() const = 0;
changed to wchar_t

http://codereview.chromium.org/554053/diff/1/15#newcode30
gpu/demos/framework/demo.h:30: // This function is called by the framework to
initialize the OpenGL state
On 2010/01/22 19:20:00, apatrick wrote:
> Nit: newline above?

Done.

http://codereview.chromium.org/554053/diff/1/15#newcode42
gpu/demos/framework/demo.h:42: inline int width() const { return width_; }
On 2010/01/22 19:20:00, apatrick wrote:
> inline is redundant.

Done.

http://codereview.chromium.org/554053/diff/1/8
File gpu/demos/framework/demo_factory.h (right):

http://codereview.chromium.org/554053/diff/1/8#newcode1
gpu/demos/framework/demo_factory.h:1: // Copyright (c) 2006-2009 The Chromium
Authors. All rights reserved.
On 2010/01/22 19:20:00, apatrick wrote:
> 2010

Done.

http://codereview.chromium.org/554053/diff/1/8#newcode13
gpu/demos/framework/demo_factory.h:13: extern Demo* CreateDemo();
there is no circular dependency. demo class does not depend on framework. It is
entry point -> framework -> demo.

http://codereview.chromium.org/554053/diff/1/11
File gpu/demos/framework/main_pepper.cc (right):

http://codereview.chromium.org/554053/diff/1/11#newcode18
gpu/demos/framework/main_pepper.cc:18: if (browser->version >= 14) {
Yes. what type of error should be appropriate? I copied most of the stuff here
from pepper_test_plugin.

http://codereview.chromium.org/554053/diff/1/11#newcode97
gpu/demos/framework/main_pepper.cc:97: fprintf(stderr, "Unhandled variable to
NPP_GetValue\n");
changed to log.

http://codereview.chromium.org/554053/diff/1/11#newcode114
gpu/demos/framework/main_pepper.cc:114: plugin_funcs->version = 11;
On 2010/01/22 19:20:00, apatrick wrote:
> Use a version constant.

Done.

http://codereview.chromium.org/554053/diff/1/10
File gpu/demos/framework/plugin.cc (right):

http://codereview.chromium.org/554053/diff/1/10#newcode12
gpu/demos/framework/plugin.cc:12: static NPExtensions* extensions = NULL;
On 2010/01/22 19:20:00, apatrick wrote:
> static is redundant. I think the convention for globals is a g_ prefix.

Done.

http://codereview.chromium.org/554053/diff/1/10#newcode16
gpu/demos/framework/plugin.cc:16: static NPObject* PluginAllocate(NPP npp,
NPClass* the_class) {
On 2010/01/22 19:20:00, apatrick wrote:
> None of the statics are necessary in an anonymous namespace.

Done.

http://codereview.chromium.org/554053/diff/1/10#newcode43
gpu/demos/framework/plugin.cc:43: INT32_TO_NPVARIANT(1, *result);
On 2010/01/22 19:20:00, apatrick wrote:
> Maybe VOID_TO_NPVARIANT would be a better default.

Done.

http://codereview.chromium.org/554053/diff/1/10#newcode107
gpu/demos/framework/plugin.cc:107: reinterpret_cast<void*>(&extensions));
On 2010/01/22 19:20:00, apatrick wrote:
> I think the cast is redundant.

Done.

http://codereview.chromium.org/554053/diff/1/12
File gpu/demos/framework/plugin.h (right):

http://codereview.chromium.org/554053/diff/1/12#newcode36
gpu/demos/framework/plugin.h:36: // This class object needs to be safely casted
to NPObject*. To accomplish
On 2010/01/22 19:20:00, apatrick wrote:
> You can just make Plugin derive from NPObject. Then you can have virtual
> functions and you don't need the reinterpret casts or header().

Done.

http://codereview.chromium.org/554053/diff/1/12#newcode50
gpu/demos/framework/plugin.h:50: base::AtExitManager at_exit_manager_;
I could do a new/delete in NP_Initialize/NP_Shutdown. That should work right?

http://codereview.chromium.org/554053/diff/1/12#newcode55
gpu/demos/framework/plugin.h:55: extern NPNetscapeFuncs* browser;
added g_ prefix. It is initialized in main_pepper.cc and used in two places
main_pepper.cc and plugin.cc. I could move this to main_pepper.cc and send it to
Plugin as a constructor argument. Would that be better?

http://codereview.chromium.org/554053/diff/1/17
File gpu/demos/framework/window.cc (right):

http://codereview.chromium.org/554053/diff/1/17#newcode22
gpu/demos/framework/window.cc:22: static const int32 kTransferBufferSize = 512 *
1024;
On 2010/01/22 19:20:00, apatrick wrote:
> These statics are redundant.

Done.

http://codereview.chromium.org/554053/diff/1/17#newcode53
gpu/demos/framework/window.cc:53: wnd_class.lpfnWndProc = (WNDPROC)WindowProc;
On 2010/01/22 19:20:00, apatrick wrote:
> This cast should be redundant if the signature of WindowProc is right.

Done.

http://codereview.chromium.org/554053/diff/1/17#newcode149
gpu/demos/framework/window.cc:149: while (!done) {
what would be recommended then? I was thinking of using SDL to make it
cross-platform. I am assuming SDL would have something similar.

http://codereview.chromium.org/554053/diff/1/4
File gpu/demos/gles2_book/hello_triangle.cc (right):

http://codereview.chromium.org/554053/diff/1/4#newcode1
gpu/demos/gles2_book/hello_triangle.cc:1: // Copyright (c) 2009 The Chromium
Authors. All rights reserved.
On 2010/01/22 19:20:00, apatrick wrote:
> 2010

Done.

http://codereview.chromium.org/554053/diff/1/4#newcode24
gpu/demos/gles2_book/hello_triangle.cc:24: const TCHAR* Title() const {
On 2010/01/22 19:20:00, apatrick wrote:
> wchar_t

Done.

[apatrick, 2010-01-22 21:49:42]

http://codereview.chromium.org/554053/diff/1/8
File gpu/demos/framework/demo_factory.h (right):

http://codereview.chromium.org/554053/diff/1/8#newcode13
gpu/demos/framework/demo_factory.h:13: extern Demo* CreateDemo();
Cool.

http://codereview.chromium.org/554053/diff/1/11
File gpu/demos/framework/main_pepper.cc (right):

http://codereview.chromium.org/554053/diff/1/11#newcode5
gpu/demos/framework/main_pepper.cc:5: #include <stdio.h>
I think not needed if you're using LOG.

http://codereview.chromium.org/554053/diff/1/11#newcode18
gpu/demos/framework/main_pepper.cc:18: if (browser->version >= 14) {
I think NPERR_INCOMPATIBLE_VERSION_ERROR.

http://codereview.chromium.org/554053/diff/1/11#newcode92
gpu/demos/framework/main_pepper.cc:92: browser->retainobject((NPObject*)plugin);
You don't need (NPObject*) now.

http://codereview.chromium.org/554053/diff/1/11#newcode114
gpu/demos/framework/main_pepper.cc:114: plugin_funcs->version = 11;
I meant NPVERS_HAS_NPRUNTIME_SCRIPTING.

http://codereview.chromium.org/554053/diff/1/10
File gpu/demos/framework/plugin.cc (right):

http://codereview.chromium.org/554053/diff/1/10#newcode18
gpu/demos/framework/plugin.cc:18: return reinterpret_cast<NPObject*>(plugin);
You shouldn't need any cast if Plugin derives from NPObject.

http://codereview.chromium.org/554053/diff/1/12
File gpu/demos/framework/plugin.h (right):

http://codereview.chromium.org/554053/diff/1/12#newcode50
gpu/demos/framework/plugin.h:50: base::AtExitManager at_exit_manager_;
Yeah that would do it.

http://codereview.chromium.org/554053/diff/1/12#newcode55
gpu/demos/framework/plugin.h:55: extern NPNetscapeFuncs* browser;
You could make it internal to main_pepper.cc and pass a pointer to it to the
Plugin constructor maybe?

http://codereview.chromium.org/554053/diff/1/17
File gpu/demos/framework/window.cc (right):

http://codereview.chromium.org/554053/diff/1/17#newcode55
gpu/demos/framework/window.cc:55: wnd_class.hbrBackground =
(HBRUSH)GetStockObject(BLACK_BRUSH);
You missed this one.

http://codereview.chromium.org/554053/diff/1/17#newcode149
gpu/demos/framework/window.cc:149: while (!done) {
I'm not sure. I'd have to experiment. But I think MessageLoop should have a
method for pumping the message loop. brettw might be a good person to ask.

[cpu_(ooo_6.6-7.5), 2010-01-22 22:23:05]

good job, some drive-by comments.

http://codereview.chromium.org/554053/diff/42/6004
File gpu/demos/app_framework/application.cc (left):

http://codereview.chromium.org/554053/diff/42/6004#oldcode4
gpu/demos/app_framework/application.cc:4: 
I think this file should be named application_win.cc it is customary in chrome
and gyp can take care of excluding it from other OS builds so you would not need
the ifdefined(OS_WIN)

http://codereview.chromium.org/554053/diff/42/6004#oldcode35
gpu/demos/app_framework/application.cc:35: case WM_PAINT: {
note that you are not handling the background paint so, windows is erasing the
background with black at every paint. Since I assume you re-paint the entire
viewport every time this is flickery.

http://codereview.chromium.org/554053/diff/42/6004#oldcode68
gpu/demos/app_framework/application.cc:68: TranslateMessage(&msg);
you want to have a consistent style, in this case I recommend 
::TranslateMessage  ::DispatchMessage , ::RegisterClass etc.

http://codereview.chromium.org/554053/diff/42/6004#oldcode72
gpu/demos/app_framework/application.cc:72: if (!done)
SendMessage(window_handle_, WM_PAINT, 0, 0);
this is bad form. To force a paint use UpdateWindow() or RedrawWindow().
WM_PAINT is a pseudo-message btw.

http://codereview.chromium.org/554053/diff/42/6004#oldcode133
gpu/demos/app_framework/application.cc:133: wnd_class.lpfnWndProc =
(WNDPROC)WindowProc;
the chrome style uses static_cast or reinterpret_cast, never the old style C
cast.

[alokp, 2010-01-22 22:30:43]

http://codereview.chromium.org/554053/diff/42/6004
File gpu/demos/app_framework/application.cc (left):

http://codereview.chromium.org/554053/diff/42/6004#oldcode4
gpu/demos/app_framework/application.cc:4: 
this file has been moved to framework/window.cc. I will address your comments in
the new file.

As far as renaming is concerned, I am planning to use SDL to make it
cross-platform. So the contents in this file is temporary.

[alokp, 2010-01-23 03:54:04]

http://codereview.chromium.org/554053/diff/1/11
File gpu/demos/framework/main_pepper.cc (right):

http://codereview.chromium.org/554053/diff/1/11#newcode5
gpu/demos/framework/main_pepper.cc:5: #include <stdio.h>
already removed. the right side is the new snapshot.

http://codereview.chromium.org/554053/diff/1/11#newcode114
gpu/demos/framework/main_pepper.cc:114: plugin_funcs->version = 11;
I used NPVERS_HAS_PLUGIN_THREAD_ASYNC_CALL everywhere.

http://codereview.chromium.org/554053/diff/1/10
File gpu/demos/framework/plugin.cc (right):

http://codereview.chromium.org/554053/diff/1/10#newcode18
gpu/demos/framework/plugin.cc:18: return reinterpret_cast<NPObject*>(plugin);
On 2010/01/22 21:49:42, apatrick wrote:
> You shouldn't need any cast if Plugin derives from NPObject.

Done.

http://codereview.chromium.org/554053/diff/1/12
File gpu/demos/framework/plugin.h (right):

http://codereview.chromium.org/554053/diff/1/12#newcode50
gpu/demos/framework/plugin.h:50: base::AtExitManager at_exit_manager_;
On 2010/01/22 21:49:42, apatrick wrote:
> Yeah that would do it.

Done.

http://codereview.chromium.org/554053/diff/1/12#newcode55
gpu/demos/framework/plugin.h:55: extern NPNetscapeFuncs* browser;
That will not work because Plugin is constructed in plugin.cc by PluginAllocate
function. I will have to move that function to main_pepper.cc too, which may not
be desirable.

http://codereview.chromium.org/554053/diff/1/17
File gpu/demos/framework/window.cc (right):

http://codereview.chromium.org/554053/diff/1/17#newcode55
gpu/demos/framework/window.cc:55: wnd_class.hbrBackground =
(HBRUSH)GetStockObject(BLACK_BRUSH);
On 2010/01/22 21:49:42, apatrick wrote:
> You missed this one.

Done.

http://codereview.chromium.org/554053/diff/42/6004
File gpu/demos/app_framework/application.cc (left):

http://codereview.chromium.org/554053/diff/42/6004#oldcode35
gpu/demos/app_framework/application.cc:35: case WM_PAINT: {
On 2010/01/22 22:23:05, cpu wrote:
> note that you are not handling the background paint so, windows is erasing the
> background with black at every paint. Since I assume you re-paint the entire
> viewport every time this is flickery.

Done.

http://codereview.chromium.org/554053/diff/42/6004#oldcode68
gpu/demos/app_framework/application.cc:68: TranslateMessage(&msg);
On 2010/01/22 22:23:05, cpu wrote:
> you want to have a consistent style, in this case I recommend 
> ::TranslateMessage  ::DispatchMessage , ::RegisterClass etc.

Done.

http://codereview.chromium.org/554053/diff/42/6004#oldcode72
gpu/demos/app_framework/application.cc:72: if (!done)
SendMessage(window_handle_, WM_PAINT, 0, 0);
On 2010/01/22 22:23:05, cpu wrote:
> this is bad form. To force a paint use UpdateWindow() or RedrawWindow().
> WM_PAINT is a pseudo-message btw.
> 

Done.

http://codereview.chromium.org/554053/diff/42/6004#oldcode133
gpu/demos/app_framework/application.cc:133: wnd_class.lpfnWndProc =
(WNDPROC)WindowProc;
On 2010/01/22 22:23:05, cpu wrote:
> the chrome style uses static_cast or reinterpret_cast, never the old style C
> cast.
> 

Done.

[alokp, 2010-01-24 21:57:06]

I believe I have addressed all comments. Please take a final look.

[cpu_(ooo_6.6-7.5), 2010-01-25 20:38:12]

OK on my comments.

On 2010/01/24 21:57:06, alokp wrote:
> I believe I have addressed all comments. Please take a final look.

[apatrick, 2010-01-25 22:41:52]

http://codereview.chromium.org/554053/diff/6058/6071
File gpu/demos/framework/main_exe.cc (right):

http://codereview.chromium.org/554053/diff/6058/6071#newcode16
gpu/demos/framework/main_exe.cc:16: // the program termintaes.
terminates

http://codereview.chromium.org/554053/diff/6058/6068
File gpu/demos/framework/main_pepper.cc (right):

http://codereview.chromium.org/554053/diff/6058/6068#newcode31
gpu/demos/framework/main_pepper.cc:31: instance->pdata =
reinterpret_cast<void*>(plugin);
you don't need the reinterpret_cast

http://codereview.chromium.org/554053/diff/6058/6068#newcode38
gpu/demos/framework/main_pepper.cc:38: Plugin* plugin =
reinterpret_cast<Plugin*>(instance->pdata);
Why did you change this from static_cast? I don't see the need for a
reinterpret_cast.

http://codereview.chromium.org/554053/diff/6058/6068#newcode45
gpu/demos/framework/main_pepper.cc:45: Plugin* plugin =
reinterpret_cast<Plugin*>(instance->pdata);
ditto

http://codereview.chromium.org/554053/diff/6058/6068#newcode96
gpu/demos/framework/main_pepper.cc:96: Plugin* plugin =
reinterpret_cast<Plugin*>(instance->pdata);
ditto

[alokp, 2010-01-25 23:39:27]

http://codereview.chromium.org/554053/diff/6058/6071
File gpu/demos/framework/main_exe.cc (right):

http://codereview.chromium.org/554053/diff/6058/6071#newcode16
gpu/demos/framework/main_exe.cc:16: // the program termintaes.
On 2010/01/25 22:41:52, apatrick wrote:
> terminates

Done.

http://codereview.chromium.org/554053/diff/6058/6068
File gpu/demos/framework/main_pepper.cc (right):

http://codereview.chromium.org/554053/diff/6058/6068#newcode31
gpu/demos/framework/main_pepper.cc:31: instance->pdata =
reinterpret_cast<void*>(plugin);
I just wanted to be explicit about what is going on here. Is that not
recommended in chromium code?

http://codereview.chromium.org/554053/diff/6058/6068#newcode38
gpu/demos/framework/main_pepper.cc:38: Plugin* plugin =
reinterpret_cast<Plugin*>(instance->pdata);
because we are casting from a void* to Plugin* that have no relation. AFAIK
reinterpret_cast is used to cast between unrelated types, no?

[apatrick, 2010-01-26 00:16:01]

http://codereview.chromium.org/554053/diff/6058/6068
File gpu/demos/framework/main_pepper.cc (right):

http://codereview.chromium.org/554053/diff/6058/6068#newcode31
gpu/demos/framework/main_pepper.cc:31: instance->pdata =
reinterpret_cast<void*>(plugin);
I don't know if Chromium has strong guidelines here. If you prefer the explicit
cast then I think static_cast is preferable here. With a reinterpret cast you
could accidentally convert from integer to pointer type for example. I'd
personally go with no cast.

http://codereview.chromium.org/554053/diff/6058/6068#newcode38
gpu/demos/framework/main_pepper.cc:38: Plugin* plugin =
reinterpret_cast<Plugin*>(instance->pdata);
You can static cast from void* to any other pointer type. You'd reinterpret cast
to convert from, e.g., integer to pointer type. A reinterpret cast is stronger
than you need here.

[alokp, 2010-01-26 00:32:50]

Removed all unnecessary reinterpret_casts.

[apatrick, 2010-01-26 00:35:26]

LGTM