Allow printing/saving a plugin through right click menu, even if it's in a fr...

Allow printing/saving a plugin through right click menu, even if it's in a frame or an embed element.

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

[jam, 2011-01-12 02:01:20]

Credit to Nico for the Mac part.

[Nico, 2011-01-12 02:10:13]

I'm not qualified to review the stuff in chrome/renderer/print* or
view/controls/menu/native_menu_win.cc.

The rest LGTM.

[jam, 2011-01-12 17:26:34]

Brett: can you take a look at the rest?

[sanjeevr, 2011-01-13 01:03:19]

You need to change PrintPages in chrome/renderer/print_web_view_helper_linux.cc
as well.

On Wed, Jan 12, 2011 at 4:42 PM, <jam@chromium.org> wrote:

> http://codereview.chromium.org/6142013/
>

[viettrungluu, 2011-01-13 01:04:39]

I'm not thrilled by the overloading of Print(), but the rest of the print_web_*
files look okay. I'm also not qualified to review
view/controls/menu/native_menu_win.cc though.

http://codereview.chromium.org/6142013/diff/17001/chrome/renderer/print_web_v...
File chrome/renderer/print_web_view_helper.cc (right):

http://codereview.chromium.org/6142013/diff/17001/chrome/renderer/print_web_v...
chrome/renderer/print_web_view_helper.cc:34: using WebKit::WebPoint;
To be consistent with the file, maybe you should add a |using WebKit::WebNode;|
(and use it, obviously)?

http://codereview.chromium.org/6142013/diff/17001/chrome/renderer/print_web_v...
chrome/renderer/print_web_view_helper.cc:482: 
Could you change that to |WebFrame* frame| while you're here?

http://codereview.chromium.org/6142013/diff/17001/chrome/renderer/print_web_v...
File chrome/renderer/print_web_view_helper.h (right):

http://codereview.chromium.org/6142013/diff/17001/chrome/renderer/print_web_v...
chrome/renderer/print_web_view_helper.h:44: WebKit::WebNode* node,
Here and elsewhere (where both a frame and node are given): Can |node| always be
null? (Maybe you should add a comment on that?)

http://codereview.chromium.org/6142013/diff/17001/chrome/renderer/print_web_v...
chrome/renderer/print_web_view_helper.h:85: void Print(WebKit::WebNode* node,
I'm not thrilled by this overload. Can we name them differently?

http://codereview.chromium.org/6142013/diff/17001/chrome/renderer/print_web_v...
chrome/renderer/print_web_view_helper.h:138: void Print(WebKit::WebFrame* frame,
Maybe you should name this one differently too, to avoid confusion.
(PrintInternal?)

[jam, 2011-01-13 02:15:42]

all done, thanks.

The only win3 specific part that's being added to native_menu_win.cc only is to
dispatch a new message.  I give you my word that it's the right one for a menu
closing :)

[brettw, 2011-01-13 04:32:57]

NativeMenuWin LGTM