Refactored GlassBrowserFrameView and BrowserDesktopTreeHostWin.

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

Index: chrome/browser/ui/views/frame/browser_desktop_window_tree_host_win.cc
diff --git a/chrome/browser/ui/views/frame/browser_desktop_window_tree_host_win.cc b/chrome/browser/ui/views/frame/browser_desktop_window_tree_host_win.cc
index 372482c2247b4a136866de75e7ca76388c701819..cc9a8e80adb9a22589874abea9ba22261cb5c456 100644
--- a/chrome/browser/ui/views/frame/browser_desktop_window_tree_host_win.cc
+++ b/chrome/browser/ui/views/frame/browser_desktop_window_tree_host_win.cc
@@ -13,19 +13,21 @@
 #include "chrome/browser/themes/theme_service.h"
 #include "chrome/browser/themes/theme_service_factory.h"
 #include "chrome/browser/ui/views/frame/browser_frame.h"
-#include "chrome/browser/ui/views/frame/browser_frame_common_win.h"
 #include "chrome/browser/ui/views/frame/browser_view.h"
 #include "chrome/browser/ui/views/frame/browser_window_property_manager_win.h"
 #include "chrome/browser/ui/views/frame/system_menu_insertion_delegate_win.h"
 #include "chrome/browser/ui/views/tabs/tab_strip.h"
 #include "chrome/common/chrome_constants.h"
 #include "ui/base/theme_provider.h"
+#include "ui/gfx/geometry/point.h"
 #include "ui/gfx/win/dpi.h"
 #include "ui/views/controls/menu/native_menu_win.h"
 
 namespace {
 
-const int kClientEdgeThickness = 3;
+// The amount of additional non-client area to draw beyond what we have Windows
+// draw, in DIPs. Only used in Windows versions pre-10.
+const int kDWMFrameBorderExtension = 3;

[ananta, 2016/04/11 21:12:11]

Please add the DIPs suffix to the constant. It would make it easier for folks
who don't read comments :)

[Bret, 2016/04/11 23:44:47]

Done.

 
 }  // namespace
 
@@ -95,12 +97,15 @@ bool BrowserDesktopWindowTreeHostWin::GetClientAreaInsets(
   }
 
   int border_thickness = GetSystemMetrics(SM_CXSIZEFRAME);
-  // In fullscreen mode, we have no frame. In restored mode, we draw our own
-  // client edge over part of the default frame.
-  if (GetWidget()->IsFullscreen())
+  if (GetWidget()->IsFullscreen()) {
+    // In fullscreen mode there is no frame.
     border_thickness = 0;
-  else if (!IsMaximized() && base::win::GetVersion() < base::win::VERSION_WIN10)
-    border_thickness -= kClientEdgeThickness;
+  } else if (!IsMaximized() &&
+             base::win::GetVersion() < base::win::VERSION_WIN10) {
+    // Reduce the Windows non-client border size because we extended the border
+    // into our client area in ::UpdateDWMFrame().
+    border_thickness -= kDWMFrameBorderExtension;
+  }
   insets->Set(0, border_thickness, border_thickness, border_thickness);
   return true;
 }
@@ -226,8 +231,14 @@ bool BrowserDesktopWindowTreeHostWin::ShouldUseNativeFrame() const {
   // context of the BrowserView destructor.
   if (!browser_view_->browser())
     return false;
-  return chrome::ShouldUseNativeFrame(browser_view_,
-                                      GetWidget()->GetThemeProvider());
+  // We don't theme popup or app windows, so regardless of whether or not a
+  // theme is active for normal browser windows, we don't want to use the custom
+  // frame for popups/apps.
+  if (!browser_view_->IsBrowserTypeNormal())
+    return true;
+  // Otherwise, we use the native frame when we're told we should by the theme
+  // provider (e.g. no custom theme is active).
+  return GetWidget()->GetThemeProvider()->ShouldUseNativeFrame();
 }
 
 void BrowserDesktopWindowTreeHostWin::FrameTypeChanged() {
@@ -259,43 +270,40 @@ void BrowserDesktopWindowTreeHostWin::UpdateDWMFrame() {
 MARGINS BrowserDesktopWindowTreeHostWin::GetDWMFrameMargins() const {
   MARGINS margins = { 0 };
 
-  // If the opaque frame is visible, we use the default (zero) margins.
-  // Otherwise, we need to figure out how to extend the glass in.
-  if (GetWidget()->ShouldUseNativeFrame()) {
-    // In fullscreen mode, we don't extend glass into the client area at all,
-    // because the GDI-drawn text in the web content composited over it will
-    // become semi-transparent over any glass area.
-    if (!IsMaximized() && !GetWidget()->IsFullscreen()) {
-      margins.cyTopHeight = kClientEdgeThickness + 1;
-      // On Windows 10, we don't draw our own window border, so don't extend the
-      // nonclient area in for it. The top is extended so that the MARGINS isn't
-      // treated as an empty (ignored) extension.
-      if (base::win::GetVersion() >= base::win::VERSION_WIN10) {
-        margins.cxLeftWidth = 0;
-        margins.cxRightWidth = 0;
-        margins.cyBottomHeight = 0;
-      } else {
-        margins.cxLeftWidth = kClientEdgeThickness + 1;
-        margins.cxRightWidth = kClientEdgeThickness + 1;
-        margins.cyBottomHeight = kClientEdgeThickness + 1;
-      }
-    }
-    // In maximized mode, we only have a titlebar strip of glass, no side/bottom
-    // borders.
-    if (!browser_view_->IsFullscreen()) {
-      gfx::Rect tabstrip_bounds(
-          browser_frame_->GetBoundsForTabStrip(browser_view_->tabstrip()));
-      tabstrip_bounds = gfx::win::DIPToScreenRect(tabstrip_bounds);
-      margins.cyTopHeight = tabstrip_bounds.bottom();
-
-      // On pre-Win 10, we need to offset the DWM frame into the toolbar so that
-      // the blackness doesn't show up on our rounded toolbar corners.  In Win
-      // 10 and above there are no rounded corners, so this is unnecessary.
-      const int kDWMFrameTopOffset = 3;
-      if (base::win::GetVersion() < base::win::VERSION_WIN10)
-        margins.cyTopHeight += kDWMFrameTopOffset;
+  // If the opaque frame is visible or we're fullscreen, we don't extend the
+  // glass in at all because it won't be visible.
+  if (!GetWidget()->ShouldUseNativeFrame() || GetWidget()->IsFullscreen())
+    return margins;
+
+  if (!IsMaximized()) {
+    if (base::win::GetVersion() < base::win::VERSION_WIN10) {
+      gfx::Point dip_margin(kDWMFrameBorderExtension, kDWMFrameBorderExtension);
+      gfx::Point pixel_margin = gfx::win::DIPToScreenPoint(dip_margin);

[ananta, 2016/04/11 21:12:11]

Please make sure that this works as expected for fractional scales like
1.25, 1.5 etc. The original code was just returning kClientEdgeThickness + 1 all
the time which looks broken.

[Bret, 2016/04/11 23:44:47]

I tested it and it looks fine at 1.25 and 1.5.

+      margins.cxLeftWidth = pixel_margin.x();
+      margins.cxRightWidth = pixel_margin.x();
+      margins.cyBottomHeight = pixel_margin.y();
+
+      // Even though we're extending the frame to the bottom of the tabstrip
+      // below we need to offset the DWM frame into the toolbar so that the
+      // blackness doesn't show up on our rounded toolbar corners.
+      margins.cyTopHeight = pixel_margin.y();
+    } else {
+      // On Windows 10 we don't need extra border thickness because we draw
+      // right up to the edge of the client area and there are no rounded
+      // corners anywhere that we need to make sure are covered.
+      margins.cxLeftWidth = 0;
+      margins.cxRightWidth = 0;
+      margins.cyTopHeight = 0;
+      margins.cyBottomHeight = 0;
     }
   }
+
+  // Extend top for the tabstrip background.
+  gfx::Rect tabstrip_bounds(
+      browser_frame_->GetBoundsForTabStrip(browser_view_->tabstrip()));
+  tabstrip_bounds = gfx::win::DIPToScreenRect(tabstrip_bounds);
+  margins.cyTopHeight += tabstrip_bounds.bottom();
+
   return margins;
 }