Add context popup menu support for content shell[GTK].

Add context popup menu support for content shell[GTK].

- For GTK content shell, add the navigation/editing/inspector context menu.
- For other platforms, specific implementation to be add.

BUG=90445
TEST=


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

[Shouqun Liu, 2012-08-31 20:16:15]

Hi, this patch adds context popup menu support for GTK content shell. For other
platforms (Win/Mac), implementations will be add later.  Thanks for review

[jochen (gone - plz use gerrit), 2012-08-31 20:58:58]

lgtm with those nits adressed

http://codereview.chromium.org/10913035/diff/2001/content/shell/shell_content...
File content/shell/shell_content_browser_client.cc (right):

http://codereview.chromium.org/10913035/diff/2001/content/shell/shell_content...
content/shell/shell_content_browser_client.cc:68: content::WebContents*
web_contents) {
no content::

http://codereview.chromium.org/10913035/diff/2001/content/shell/shell_content...
File content/shell/shell_content_browser_client.h (right):

http://codereview.chromium.org/10913035/diff/2001/content/shell/shell_content...
content/shell/shell_content_browser_client.h:36: content::WebContents*
web_contents) OVERRIDE;
no content::

http://codereview.chromium.org/10913035/diff/2001/content/shell/shell_web_con...
File content/shell/shell_web_contents_view_delegate.h (right):

http://codereview.chromium.org/10913035/diff/2001/content/shell/shell_web_con...
content/shell/shell_web_contents_view_delegate.h:66: };
please add DISALLOW_COPY_AND_ASSIGN(ShellWebContentsViewDelegate);

http://codereview.chromium.org/10913035/diff/2001/content/shell/shell_web_con...
File content/shell/shell_web_contents_view_delegate_gtk.cc (right):

http://codereview.chromium.org/10913035/diff/2001/content/shell/shell_web_con...
content/shell/shell_web_contents_view_delegate_gtk.cc:51:
g_signal_connect(back_menu, "activate",
each parameter should go on its own line

http://codereview.chromium.org/10913035/diff/2001/content/shell/shell_web_con...
content/shell/shell_web_contents_view_delegate_gtk.cc:58:
g_signal_connect(forward_menu, "activate",
same here and below

http://codereview.chromium.org/10913035/diff/2001/content/shell/shell_web_con...
content/shell/shell_web_contents_view_delegate_gtk.cc:117: 
no empty line

http://codereview.chromium.org/10913035/diff/2001/content/shell/shell_web_con...
content/shell/shell_web_contents_view_delegate_gtk.cc:223: url, NULL,
MSG_ROUTING_NONE, NULL);
either each parameter on it's own line or all parameters in one line

[Shouqun Liu, 2012-08-31 21:34:20]

Hi Jochen, patch updated with nits fixed, thanks!

One concern is that from  chromium coding
style(http://www.chromium.org/developers/coding-style), when calling functions,
parameters can be collapsed into one line to save space, so is it required to
make each parameter having its own line?

http://codereview.chromium.org/10913035/diff/2001/content/shell/shell_content...
File content/shell/shell_content_browser_client.cc (right):

http://codereview.chromium.org/10913035/diff/2001/content/shell/shell_content...
content/shell/shell_content_browser_client.cc:68: content::WebContents*
web_contents) {
On 2012/08/31 20:58:58, jochen wrote:
> no content::

Done.

http://codereview.chromium.org/10913035/diff/2001/content/shell/shell_content...
File content/shell/shell_content_browser_client.h (right):

http://codereview.chromium.org/10913035/diff/2001/content/shell/shell_content...
content/shell/shell_content_browser_client.h:36: content::WebContents*
web_contents) OVERRIDE;
On 2012/08/31 20:58:58, jochen wrote:
> no content::

Done.

http://codereview.chromium.org/10913035/diff/2001/content/shell/shell_web_con...
File content/shell/shell_web_contents_view_delegate.h (right):

http://codereview.chromium.org/10913035/diff/2001/content/shell/shell_web_con...
content/shell/shell_web_contents_view_delegate.h:66: };
On 2012/08/31 20:58:58, jochen wrote:
> please add DISALLOW_COPY_AND_ASSIGN(ShellWebContentsViewDelegate);

Done.

http://codereview.chromium.org/10913035/diff/2001/content/shell/shell_web_con...
File content/shell/shell_web_contents_view_delegate_gtk.cc (right):

http://codereview.chromium.org/10913035/diff/2001/content/shell/shell_web_con...
content/shell/shell_web_contents_view_delegate_gtk.cc:51:
g_signal_connect(back_menu, "activate",
On 2012/08/31 20:58:58, jochen wrote:
> each parameter should go on its own line

Done.

[jochen (gone - plz use gerrit), 2012-08-31 22:04:42]

On 2012/08/31 21:34:20, Shouqun Liu wrote:
> Hi Jochen, patch updated with nits fixed, thanks!
> 
> One concern is that from  chromium coding
> style(http://www.chromium.org/developers/coding-style), when calling
functions,
> parameters can be collapsed into one line to save space, so is it required to
> make each parameter having its own line?

well, yes, if it also helps readability. However, there's also the rule (from
the google c++ style guide) that parameters should be aligned if you wrap them
(which is in contradiction to the other rule - hurray for precise style guides).

[Shouqun Liu, 2012-08-31 22:10:49]

Yes, understand it, thanks:)   
Is it OK to commit this patch with adding TBR? 

On 2012/08/31 22:04:42, jochen wrote:
> On 2012/08/31 21:34:20, Shouqun Liu wrote:
> > Hi Jochen, patch updated with nits fixed, thanks!
> > 
> > One concern is that from  chromium coding
> > style(http://www.chromium.org/developers/coding-style), when calling
> functions,
> > parameters can be collapsed into one line to save space, so is it required
to
> > make each parameter having its own line?
> 
> well, yes, if it also helps readability. However, there's also the rule (from
> the google c++ style guide) that parameters should be aligned if you wrap them
> (which is in contradiction to the other rule - hurray for precise style
guides).

[jam, 2012-08-31 22:23:38]

lgtm, thanks for doing this!
and yes, feel free to tbr trivial changes to the gypi files in the future

https://codereview.chromium.org/10913035/diff/7/content/shell/shell_web_conte...
File content/shell/shell_web_contents_view_delegate_gtk.cc (right):

https://codereview.chromium.org/10913035/diff/7/content/shell/shell_web_conte...
content/shell/shell_web_contents_view_delegate_gtk.cc:213: RenderViewHost* rvh =
web_contents_->GetRenderViewHost();
nit: here and below, i would personally just write

web_contents_->GetRenderViewHost()->Cut() since it fits on one line

[Shouqun Liu, 2012-08-31 22:30:53]

Done, merged into one line,  thanks for review:-) 
On 2012/08/31 22:23:38, John Abd-El-Malek wrote:
> lgtm, thanks for doing this!
> and yes, feel free to tbr trivial changes to the gypi files in the future
> 
>
https://codereview.chromium.org/10913035/diff/7/content/shell/shell_web_conte...
> File content/shell/shell_web_contents_view_delegate_gtk.cc (right):
> 
>
https://codereview.chromium.org/10913035/diff/7/content/shell/shell_web_conte...
> content/shell/shell_web_contents_view_delegate_gtk.cc:213: RenderViewHost* rvh
=
> web_contents_->GetRenderViewHost();
> nit: here and below, i would personally just write
> 
> web_contents_->GetRenderViewHost()->Cut() since it fits on one line

[commit-bot: I haz the power, 2012-08-31 22:33:36]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/shouqun.liu@intel.com/10913035/9002

[commit-bot: I haz the power, 2012-08-31 23:02:24]

Try job failure for 10913035-9002 (retry) on mac for step "compile" (clobber
build).
It's a second try, previously, step "compile" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac&number...

[Shouqun Liu, 2012-09-01 00:20:09]

Fix the Mac/Mac_rel build issue.

Add "shell_web_contents_view_delegate_creator.h" to avoid  including 
"[shell_]web_contents_view_delegate.h".
Since "web_contents_view_delegate.h" includes <Cocoa/Cocoa.h>, it leads to
compile errors when it is included by .cc files in Mac. 

So add the CreateShellWebContentsViewDelegate function to   avoid including
<Cocoa/Cocoa.h>.

[commit-bot: I haz the power, 2012-09-02 22:29:07]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/shouqun.liu@intel.com/10913035/8009

[commit-bot: I haz the power, 2012-09-03 01:29:43]

Change committed as 154650