content: Split off ChromeTabContentsViewGtk from TabContentsViewGtk. This new

content: Split off ChromeTabContentsViewGtk from TabContentsViewGtk. This new
class contains almost all chrome specific details from TabContentsViewGtk,
leaving TabContentsViewGtk as a pure content/ class (soon to be moved out of
chrome/).

Why subclass instead of composing? First, because the X vs ChromeX is fairly
common already, but mostly because large chunks of the gtk ui code assume
there's a single widget that can be switched in and out on tab showing and that
this is TabContentsViewGtk::GetNativeView(). (This also means it is easy to
address the other join points, like sad tabs or menus.)

BUG=none
TEST=none


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

[Elliot Glaysher, 2011-10-19 21:26:53]

I'm asking for a desgin review instead of a code review at this early stage.
(i.e., NavigationControllerTest.RestoreNavigate is a real failure and the cause
needs to be analyzed and fixed.)

Is this an appropriate way to split things up?

[jam, 2011-10-20 00:18:07]

On 2011/10/19 21:26:53, Elliot Glaysher wrote:
> I'm asking for a desgin review instead of a code review at this early stage.
> (i.e., NavigationControllerTest.RestoreNavigate is a real failure and the
cause
> needs to be analyzed and fixed.)
> 
> Is this an appropriate way to split things up?

If the browser code would stay like this, then inheritance would be fine. but
with the ongoing work to hide content implementation classes from chrome, then
composition would be much better. what do you think?

[Elliot Glaysher, 2011-11-15 19:09:45]

Now that avi's SadTab patch is in, here's a reworking of this patch that uses
interfaces instead of inheritance.

I am not sure what to do about the right click menu. It feels like it might not
belong where I put it, but I can't think of a better place for it. :(

[jam, 2011-11-15 21:40:50]

lgtm

not sure what to do about right click menu stuff, I dont have this code in my
head now to offer better alternatives. seems fine to do this and if we come up
with better ways in the future we can change it.

http://codereview.chromium.org/8344061/diff/13014/chrome/browser/tab_contents...
File chrome/browser/tab_contents/chrome_tab_contents_view_wrapper_gtk.h (right):

http://codereview.chromium.org/8344061/diff/13014/chrome/browser/tab_contents...
chrome/browser/tab_contents/chrome_tab_contents_view_wrapper_gtk.h:24: explicit
ChromeTabContentsViewWrapperGtk();
nit: don't need explicit

[commit-bot: I haz the power, 2011-11-15 22:30:38]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/erg@chromium.org/8344061/13015

[commit-bot: I haz the power, 2011-11-15 23:56:14]

Try job failure for 8344061-13015 (retry) on linux_clang for step "compile"
(clobber build).
It's a second try, previously, step "compile" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_clan...

[commit-bot: I haz the power, 2011-11-16 00:08:29]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/erg@chromium.org/8344061/17001

[commit-bot: I haz the power, 2011-11-16 01:24:47]

Change committed as 110236