Fix opaque frame app mode windows to have correct toolbar/content separator.

Fix opaque frame app mode windows to have correct toolbar/content separator.

BUG=none
TEST=On Windows XP, use "More tools->Add to desktop" and check "Open as window" to create an app shortcut.  Then open it and ensure the title bar has a rounded-looking bottom and divider line above the content area, and doesn't just abruptly turn into the content.

Committed: https://crrev.com/a962c414a493e21c092c1d8a9a649048c6f028eb
Cr-Commit-Position: refs/heads/master@{#360422}

[Peter Kasting, 2015-11-18 06:43:35]

pkasting@chromium.org changed reviewers:
+ sky@chromium.org

[Peter Kasting, 2015-11-18 06:43:36]

A screenshot comparing the new rendering (center) with the old rendering (left)
and a popup window (right) is at http://imgur.com/DNVPIz0 .

The primary problem was that BrowserView::IsToolbarVisible() returns true for
app windows; apparently they can theoretically have a location bar or something?
 By changing the opaque frame's concept of "toolbar visible" to be "toolbar
supported and has nonzero size" we can distinguish app mode windows from popups.

A few tweaks were required to the titlebar height calculation, since with popup
windows, the "3D part" of the toolbar bottom is actually part of the toolbar,
whereas for app windows it isn't.  Without this tweak, app window titlebars
would be 3 px shorter than popup windows.

I also removed the conditional about "else if !tabstrip" in the client edge
drawing code, because I don't believe a case exists where the window supports a
tabstrip but not a toolbar.

[sky, 2015-11-18 17:00:34]

https://codereview.chromium.org/1460483003/diff/20001/chrome/browser/ui/views...
File chrome/browser/ui/views/frame/opaque_browser_frame_view.cc (right):

https://codereview.chromium.org/1460483003/diff/20001/chrome/browser/ui/views...
chrome/browser/ui/views/frame/opaque_browser_frame_view.cc:441:
!browser_view()->GetToolbarBounds().IsEmpty();
It seems like this is called from code from layout like code. Shouldn't this
query whether the toolbar will be given a valid bounds vs whether it currently
has a valid bounds?

[Peter Kasting, 2015-11-18 19:46:24]

PTAL.

https://codereview.chromium.org/1460483003/diff/20001/chrome/browser/ui/views...
File chrome/browser/ui/views/frame/opaque_browser_frame_view.cc (right):

https://codereview.chromium.org/1460483003/diff/20001/chrome/browser/ui/views...
chrome/browser/ui/views/frame/opaque_browser_frame_view.cc:441:
!browser_view()->GetToolbarBounds().IsEmpty();
On 2015/11/18 17:00:34, sky wrote:
> It seems like this is called from code from layout like code. Shouldn't this
> query whether the toolbar will be given a valid bounds vs whether it currently
> has a valid bounds?

I think you're right.  I changed this to use the toolbar's preferred size.

There is one case where I know these would report different values, and that's
when the location bar height is being animated and is current 0.  In that case
it probably makes sense to treat this as "has a toolbar" since presumably we're
about to have a nonzero height.  I'm not certain, because I can't tell what that
animation code is actually used for, so I don't know how to test it.

[sky, 2015-11-18 21:04:23]

LGTM

[Peter Kasting, 2015-11-18 21:05:03]

The CQ bit was checked by pkasting@chromium.org

[commit-bot: I haz the power, 2015-11-18 21:05:26]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1460483003/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1460483003/60001

[commit-bot: I haz the power, 2015-11-18 21:48:47]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-11-18 21:49:31]

Patchset 4 (id:??) landed as
https://crrev.com/a962c414a493e21c092c1d8a9a649048c6f028eb
Cr-Commit-Position: refs/heads/master@{#360422}