Gtk3: Render a GtkHeaderBar as the background of the tab strip

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

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/ui/views/frame/opaque_browser_frame_view.h"
 
 #include "build/build_config.h"
 #include "chrome/browser/themes/theme_properties.h"
 #include "chrome/browser/themes/theme_service_factory.h"
 #include "chrome/browser/ui/layout_constants.h"
@@ skipping 489 matching lines @@
       IsMaximized());
 }
 
 int OpaqueBrowserFrameView::GetTopAreaHeight() const {
   // The top area height in dp (only used when there's no frame image).
   // TODO(pkasting): investigate removing this constant. See crbug.com/590301
   const int kHeight = 64;
   const gfx::ImageSkia frame_image = GetFrameImage();
   int top_area_height = frame_image.isNull() ? kHeight : frame_image.height();
   if (browser_view()->IsTabStripVisible()) {
-    top_area_height =
-        std::max(top_area_height,
-                 GetBoundsForTabStrip(browser_view()->tabstrip()).bottom());
+    // TODO(thomasanderson): Figure out what this should be.  Do no land as is.
+    top_area_height = GetBoundsForTabStrip(browser_view()->tabstrip()).bottom();

[Tom (Use chromium acct), 2017/01/11 20:07:27]

pkasting@:  The "kHeight = 64" was messing this up.  The top area height should
be 44 when restored and 29 when maximized, but this code was causing it to
always be 64, which was causing the GtkHeaderBar to render stretched out.  Is
the fix just a matter of hard-coding these new constants?

[Peter Kasting, 2017/01/12 18:53:52]

44 and 29 are what the GetBoundsForTabStrip() call will end up returning, right?

I don't think we want to hardcode constants like those.  It's not those specific
values you want, it's "always use the height of the tabstrip bottom".  In other
words, I think you basically want what my TODO above suggests: to just remove
kHeight.

Looking at how this value will be used, I think it's safe to go ahead and do
this.

Effectively, you'll want to do:

  int top_area_height = frame_image.height();  // Returns 0 if isNull()

...and then leave the old body of this conditional.

What you'll want to check is what happens in windows with no tabstrip.  Reading
the above, I think this will return 0 then, and I'm not sure whether that's
right.  It depends how we draw things afterwards.

[Tom (Use chromium acct), 2017/01/12 21:51:09]

Yes
Done
I tested with a fullscreen window and this function never gets called.

[Peter Kasting, 2017/01/12 22:02:56]

Fullscreen wouldn't be the thing to check; you'd want a case where this gets
called with IsTabStripVisible() false, so a window without a tabstrip.  An app
window ought to work for this, I think?

[Tom (Use chromium acct), 2017/01/12 22:54:13]

Hm.. I tried with many types of windows and this still doesn't get called :S
I tried with:
* Chrome Remote Desktop window
* gnubbyd window
* Wrench menu
* Context menus
* Tooltips
* Chrome task manager
* JS alerts

[Peter Kasting, 2017/01/12 23:14:04]

Things like menus, tooltips, and alerts wouldn't get a frame, so you'd be
looking at app windows mostly; I dunno whether the remote desktop/gnubbyd cases
would fit this.  I'd do app>More Tools>Add To Desktop and make sure "open as
window" is checked.  Make sure to test in an environment where the opaque frame
is the native frame; I think for example Win 7 Aero Basic (non-glass) is this
case.

[Tom (Use chromium acct), 2017/01/13 00:59:51]

Ok that seemed to work.  It does return 0, but nothing is visually wrong

   }
   return top_area_height;
 }
 
 void OpaqueBrowserFrameView::PaintRestoredFrameBorder(
     gfx::Canvas* canvas) const {
   frame_background_->set_frame_color(GetFrameColor());
+  frame_background_->set_is_active(ShouldPaintAsActive());
   frame_background_->set_theme_image(GetFrameImage());
   frame_background_->set_theme_overlay_image(GetFrameOverlayImage());
   frame_background_->set_top_area_height(GetTopAreaHeight());
 
   const ui::ThemeProvider* tp = GetThemeProvider();
   frame_background_->SetSideImages(
       tp->GetImageSkiaNamed(IDR_WINDOW_LEFT_SIDE),
       tp->GetImageSkiaNamed(IDR_WINDOW_TOP_CENTER),
       tp->GetImageSkiaNamed(IDR_WINDOW_RIGHT_SIDE),
       tp->GetImageSkiaNamed(IDR_WINDOW_BOTTOM_CENTER));
   frame_background_->SetCornerImages(
       tp->GetImageSkiaNamed(IDR_WINDOW_TOP_LEFT_CORNER),
       tp->GetImageSkiaNamed(IDR_WINDOW_TOP_RIGHT_CORNER),
       tp->GetImageSkiaNamed(IDR_WINDOW_BOTTOM_LEFT_CORNER),
       tp->GetImageSkiaNamed(IDR_WINDOW_BOTTOM_RIGHT_CORNER));
   frame_background_->PaintRestored(canvas, this);
 
   // Note: When we don't have a toolbar, we need to draw some kind of bottom
   // edge here.  Because the App Window graphics we use for this have an
   // attached client edge and their sizing algorithm is a little involved, we do
   // all this in PaintRestoredClientEdge().
 }
 
 void OpaqueBrowserFrameView::PaintMaximizedFrameBorder(
     gfx::Canvas* canvas) const {
   frame_background_->set_frame_color(GetFrameColor());
+  frame_background_->set_is_active(ShouldPaintAsActive());
   frame_background_->set_theme_image(GetFrameImage());
   frame_background_->set_theme_overlay_image(GetFrameOverlayImage());
   frame_background_->set_top_area_height(GetTopAreaHeight());
   frame_background_->set_maximized_top_inset(
       GetTopInset(true) - GetTopInset(false));
   frame_background_->PaintMaximized(canvas, this);
 }
 
 void OpaqueBrowserFrameView::PaintToolbarBackground(gfx::Canvas* canvas) const {
   // TODO(estade): can this be shared with OpaqueBrowserFrameView?
@@ skipping 110 matching lines @@
   gfx::Rect side(x, y, kClientEdgeThickness, h);
   canvas->FillRect(side, color);
   if (draw_bottom) {
     canvas->FillRect(gfx::Rect(x, y + h, w + (2 * kClientEdgeThickness),
                                kClientEdgeThickness),
                      color);
   }
   side.Offset(w + kClientEdgeThickness, 0);
   canvas->FillRect(side, color);
 }