Linux Aura: Hide title bar when CustomFrameView maximized on Unity.

Linux Aura: Hide title bar when CustomFrameView maximized on Unity.

This avoids showing a double title bar on Task Manager and V2 app
windows.

BUG=330090
TEST=On Unity, maximize Task Manager and V2 app window. There should be
only the Unity title and caption buttons, no blue custom title bar.

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=246544

[Matt Giuca, 2013-12-20 04:28:40]

FYI: Sister CL to do the same thing for browser windows:
https://codereview.chromium.org/117233008

[Matt Giuca, 2014-01-08 23:14:35]

Ping.

[Ben Goodger (Google), 2014-01-09 23:03:27]

https://codereview.chromium.org/98793009/diff/1/ui/views/window/custom_frame_...
File ui/views/window/custom_frame_view.cc (right):

https://codereview.chromium.org/98793009/diff/1/ui/views/window/custom_frame_...
ui/views/window/custom_frame_view.cc:35: #include "ui/views/linux_ui/linux_ui.h"
Hrm... I would actually like to not have this type of #include seep into views.
I understand linux_ui is already in views, but linux_ui pulls in other stuff
that views by and large does not depend on, e.g. shell dialogs, and your code in
this file mentions things I'd rather keep out of the views layer (e.g. mentions
of unity etc).

Is there a way to inject this dependency instead?

[Matt Giuca, 2014-01-10 01:07:49]

https://codereview.chromium.org/98793009/diff/1/ui/views/window/custom_frame_...
File ui/views/window/custom_frame_view.cc (right):

https://codereview.chromium.org/98793009/diff/1/ui/views/window/custom_frame_...
ui/views/window/custom_frame_view.cc:35: #include "ui/views/linux_ui/linux_ui.h"
Is the style used by OpaqueBrowserFrameView (having a specific
OpaqueBrowserFrameViewPlatformSpecific class that is overridden by
OpaqueBrowserFrameViewLinux) appropriate here?

It's a little over-engineered for my liking, but it would get the Linux (and
Unity)-specific stuff out of this generic class.

Just wanted to check that this is what you mean before I go implementing it.

[Ben Goodger (Google), 2014-01-10 16:41:50]

https://codereview.chromium.org/98793009/diff/1/ui/views/window/custom_frame_...
File ui/views/window/custom_frame_view.cc (right):

https://codereview.chromium.org/98793009/diff/1/ui/views/window/custom_frame_...
ui/views/window/custom_frame_view.cc:339: // title and caption buttons, so we
should not add our own.
is this true of all frame types within views? if so, then a ViewsDelegate method
might be in order.

[Matt Giuca, 2014-01-17 01:29:51]

Hi Ben,

Sorry for the long delay. Since your comment required a refactor I made it a
lower priority.

https://codereview.chromium.org/98793009/diff/1/ui/views/window/custom_frame_...
File ui/views/window/custom_frame_view.cc (right):

https://codereview.chromium.org/98793009/diff/1/ui/views/window/custom_frame_...
ui/views/window/custom_frame_view.cc:339: // title and caption buttons, so we
should not add our own.
OK, I have added a ViewsDelegate method for this. Two things that I wasn't sure
about:
- I've still got the Unity-specific check in, but it's in
chrome/browser/ui/views now, instead of ui/views. Does that satisfy your above
concern about not wanting this sort of code to creep into Views?
- I've noticed that ViewsDelegate is used in very few places in the code. I'm
not sure if it's appropriate to add random methods in there (it seems like it's
reserved for low-level Views things, not specific needs of particular Views
clients).

To answer your first question, I've updated OpaqueBrowserFrameViews to use this
new scheme, and it's a lot shorter, so yes, this is useful in more than one
location.

[Ben Goodger (Google), 2014-01-21 17:34:20]

It's OK to add random methods to ViewsDelegate. It's not a perfect solution and
I'd like to come up with something better in the fullness of time, but for now
it's the primary way of injecting dependencies on stuff I don't want Views core
to know about.

LGTM

[commit-bot: I haz the power, 2014-01-23 03:37:30]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mgiuca@chromium.org/98793009/390001

[commit-bot: I haz the power, 2014-01-23 09:27:04]

Change committed as 246544