Moved Pepper delegate definition to chrome\renderer to allow it to use...

Moved Pepper delegate definition to chrome\renderer to allow it to use
code from chrome\common.

Also added canvas support and hooked up NPAPI interface functions.
No tests have been added yet, so this is probably not fully ready.


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

[sehr (please use chromium), 2009-10-16 19:03:06]

I've uploaded the first version of hooking up the graphics apis.  Please feel
free to slice and dice.

[jam, 2009-10-16 19:21:05]

nit: the description should be that this stuff is moving to the renderer to
allow it to use chrome\common code.  webkit glue can already reference
third_party\skia, base\gfx

http://codereview.chromium.org/291001/diff/1/10
File chrome/renderer/webplugin_delegate_pepper_impl.cc (right):

http://codereview.chromium.org/291001/diff/1/10#newcode125
Line 125: if (window_rect_ != window_rect) {
nit: the convention is to early return if possible, to keep tabbing to a minimum

http://codereview.chromium.org/291001/diff/1/10#newcode399
Line 399: {
nit: I think there's an extra level of tabbing here.

http://codereview.chromium.org/291001/diff/1/11
File chrome/renderer/webplugin_delegate_pepper_impl.h (right):

http://codereview.chromium.org/291001/diff/1/11#newcode1
Line 1: // Copyright (c) 2006-2008 The Chromium Authors. All rights reserved.
nit: since you're moving this file, do you mind naming the class
WebPluginDelegatePepper without the impl?  We usually use impl when there's an
interface class web_x and then we have the implementation in web_x_impl, and
proxy/stub if appropriate in web_x_proxy and web_x_stub.  In this case, there's
no webplugindelegate_pepper so no need for the _impl.

http://codereview.chromium.org/291001/diff/1/6
File webkit/glue/plugins/plugin_instance.cc (right):

http://codereview.chromium.org/291001/diff/1/6#newcode365
Line 365: webkit_glue::WebPluginDelegate* delegate = webplugin()->delegate();
Can the PluginInstance changes just be deleted they're unnecessary?  Sorry I
didn't catch this in the previous review.

http://codereview.chromium.org/291001/diff/1/5
File webkit/glue/webplugin_impl.h (right):

http://codereview.chromium.org/291001/diff/1/5#newcode61
Line 61: virtual WebPluginDelegate* delegate() { return delegate_; }
nit: we usually put the document in the interface, and leave the derived classes
with no comments just to avoid having the comment in two places and then
diverging.

[sehr (please use chromium), 2009-10-16 23:13:25]

Thanks.  Next version ready.

http://codereview.chromium.org/291001/diff/1/10
File chrome/renderer/webplugin_delegate_pepper_impl.cc (right):

http://codereview.chromium.org/291001/diff/1/10#newcode125
Line 125: if (window_rect_ != window_rect) {
On 2009/10/16 19:21:05, John Abd-El-Malek wrote:
> nit: the convention is to early return if possible, to keep tabbing to a
minimum

Done.

http://codereview.chromium.org/291001/diff/1/10#newcode399
Line 399: {
On 2009/10/16 19:21:05, John Abd-El-Malek wrote:
> nit: I think there's an extra level of tabbing here.

Done.

http://codereview.chromium.org/291001/diff/1/11
File chrome/renderer/webplugin_delegate_pepper_impl.h (right):

http://codereview.chromium.org/291001/diff/1/11#newcode1
Line 1: // Copyright (c) 2006-2008 The Chromium Authors. All rights reserved.
On 2009/10/16 19:21:05, John Abd-El-Malek wrote:
> nit: since you're moving this file, do you mind naming the class
> WebPluginDelegatePepper without the impl?  We usually use impl when there's an
> interface class web_x and then we have the implementation in web_x_impl, and
> proxy/stub if appropriate in web_x_proxy and web_x_stub.  In this case,
there's
> no webplugindelegate_pepper so no need for the _impl.

Done.

http://codereview.chromium.org/291001/diff/1/6
File webkit/glue/plugins/plugin_instance.cc (right):

http://codereview.chromium.org/291001/diff/1/6#newcode365
Line 365: webkit_glue::WebPluginDelegate* delegate = webplugin()->delegate();
On 2009/10/16 19:21:05, John Abd-El-Malek wrote:
> Can the PluginInstance changes just be deleted they're unnecessary?  Sorry I
> didn't catch this in the previous review.

No problem.  Removed.

http://codereview.chromium.org/291001/diff/1/5
File webkit/glue/webplugin_impl.h (right):

http://codereview.chromium.org/291001/diff/1/5#newcode61
Line 61: virtual WebPluginDelegate* delegate() { return delegate_; }
On 2009/10/16 19:21:05, John Abd-El-Malek wrote:
> nit: we usually put the document in the interface, and leave the derived
classes
> with no comments just to avoid having the comment in two places and then
> diverging.

Done.

[jam, 2009-10-16 23:21:59]

lgtm

http://codereview.chromium.org/291001/diff/8001/3027
File chrome/renderer/webplugin_delegate_pepper.cc (right):

http://codereview.chromium.org/291001/diff/8001/3027#newcode126
Line 126: return;
nit: no brace brackets needed for single line statements, also to be consistent
with line 137

http://codereview.chromium.org/291001/diff/8001/3026
File chrome/renderer/webplugin_delegate_pepper.h (right):

http://codereview.chromium.org/291001/diff/8001/3026#newcode5
Line 5: #ifndef WEBKIT_GLUE_PLUGIN_WEBPLUGIN_DELEGATE_PEPPER_H_
nit: the header guard needs to get updated

[sehr (please use chromium), 2009-10-17 15:55:53]

Fixed those issues.  Waiting for trybots for submission.

http://codereview.chromium.org/291001/diff/8001/3027
File chrome/renderer/webplugin_delegate_pepper.cc (right):

http://codereview.chromium.org/291001/diff/8001/3027#newcode126
Line 126: return;
On 2009/10/16 23:22:00, John Abd-El-Malek wrote:
> nit: no brace brackets needed for single line statements, also to be
consistent
> with line 137

Done.

http://codereview.chromium.org/291001/diff/8001/3026
File chrome/renderer/webplugin_delegate_pepper.h (right):

http://codereview.chromium.org/291001/diff/8001/3026#newcode5
Line 5: #ifndef WEBKIT_GLUE_PLUGIN_WEBPLUGIN_DELEGATE_PEPPER_H_
On 2009/10/16 23:22:00, John Abd-El-Malek wrote:
> nit: the header guard needs to get updated

Fixed the last half, forgot the directory... Fixed.