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

chrome/browser/ui/views/frame/opaque_browser_frame_view.cc

Index: chrome/browser/ui/views/frame/opaque_browser_frame_view.cc
diff --git a/chrome/browser/ui/views/frame/opaque_browser_frame_view.cc b/chrome/browser/ui/views/frame/opaque_browser_frame_view.cc
index 90d2eac814858560039662804b4d7bbd0989af65..b9b1887340da8a0ef03723ca2baa3fa5fa43c1d6 100644
--- a/chrome/browser/ui/views/frame/opaque_browser_frame_view.cc
+++ b/chrome/browser/ui/views/frame/opaque_browser_frame_view.cc
@@ -436,6 +436,11 @@ bool OpaqueBrowserFrameView::IsTabStripVisible() const {
   return browser_view()->IsTabStripVisible();
 }
 
+bool OpaqueBrowserFrameView::IsToolbarVisible() const {
+  return browser_view()->IsToolbarVisible() &&
+      !browser_view()->GetToolbarBounds().IsEmpty();

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

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]

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.

+}
+
 int OpaqueBrowserFrameView::GetTabStripHeight() const {
   return browser_view()->GetTabStripHeight();
 }
@@ -466,10 +471,9 @@ void OpaqueBrowserFrameView::OnPaint(gfx::Canvas* canvas) {
    * So we'd need to sample the background color at the right location and
    * synthesize a good shadow color. */
 
-  if (browser_view()->IsToolbarVisible())
+  if (IsToolbarVisible())
     PaintToolbarBackground(canvas);
-  if (!layout_->IsTitleBarCondensed())
-    PaintRestoredClientEdge(canvas);
+  PaintClientEdge(canvas);
 }
 
 // BrowserNonClientFrameView:
@@ -773,7 +777,7 @@ void OpaqueBrowserFrameView::PaintToolbarBackground(gfx::Canvas* canvas) {
   }
 }
 
-void OpaqueBrowserFrameView::PaintRestoredClientEdge(gfx::Canvas* canvas) {
+void OpaqueBrowserFrameView::PaintClientEdge(gfx::Canvas* canvas) {
   ui::ThemeProvider* tp = GetThemeProvider();
   int client_area_top = frame()->client_view()->y();
   int image_top = client_area_top;
@@ -782,14 +786,13 @@ void OpaqueBrowserFrameView::PaintRestoredClientEdge(gfx::Canvas* canvas) {
       layout_->CalculateClientAreaBounds(width(), height());
   SkColor toolbar_color = tp->GetColor(ThemeProperties::COLOR_TOOLBAR);
 
-  if (browser_view()->IsToolbarVisible()) {
+  if (IsToolbarVisible()) {
     // The client edge images start below the toolbar.
     gfx::Rect toolbar_bounds(browser_view()->GetToolbarBounds());
     client_area_top += toolbar_bounds.bottom();
     image_top = client_area_top;
-  } else if (!browser_view()->IsTabStripVisible()) {
-    // The toolbar isn't going to draw a client edge for us, so draw one
-    // ourselves.
+  } else {
+    // The toolbar isn't going to draw a top edge for us, so draw one ourselves.
     gfx::ImageSkia* top_left = tp->GetImageSkiaNamed(IDR_APP_TOP_LEFT);
     gfx::ImageSkia* top_center = tp->GetImageSkiaNamed(IDR_APP_TOP_CENTER);
     gfx::ImageSkia* top_right = tp->GetImageSkiaNamed(IDR_APP_TOP_RIGHT);
@@ -812,6 +815,10 @@ void OpaqueBrowserFrameView::PaintRestoredClientEdge(gfx::Canvas* canvas) {
         kClientEdgeThickness), toolbar_color);
   }
 
+  // In maximized mode, the only edge to draw is the top one, so we're done.
+  if (layout_->IsTitleBarCondensed())
+    return;
+
   int client_area_bottom =
       std::max(client_area_top, height() - NonClientBorderThickness());
   int image_height = client_area_bottom - image_top;