Fix app window titlebar blending with direct composition

Clear the DWM frame area on WM_ERASEBKGND so that Chrome's client area
blends with DWM frame for app windows.  Refactor the code to update and
clear DWM frame and move it to hwnd message handler so that browser and
app windows can share the same code.

This mimics existing logic for updating the DWM frame and adds the clear
DWM frame behavior to app windows, but one notable change is that it
will clear on every WM_ERASEBKGND message, and not just the first one.
This shouldn't have a performance impact and seems more correct anyway.

Bug: 904322
Change-Id: I70c3ae97a94114dd63110736a35cef4dd887b1aa
Reviewed-on: https://chromium-review.googlesource.com/c/1382928Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Bret Sepulveda <bsep@chromium.org>
Commit-Queue: Sunny Sachanandani <sunnyps@chromium.org>
Cr-Commit-Position: refs/heads/master@{#618684}

chrome/browser/ui/views/apps/app_window_desktop_window_tree_host_win.cc

@@ -4,7 +4,10 @@
 
 #include "chrome/browser/ui/views/apps/app_window_desktop_window_tree_host_win.h"
 
+#include <windows.h>
+
 #include <dwmapi.h>
+#include <uxtheme.h>
 
 #include "base/win/windows_version.h"
 #include "chrome/browser/ui/views/apps/chrome_native_app_window_views_win.h"
@@ -39,43 +42,29 @@ bool AppWindowDesktopWindowTreeHostWin::GetClientAreaInsets(
   return true;
 }
 
-void AppWindowDesktopWindowTreeHostWin::HandleFrameChanged() {
-  // We need to update the glass region on or off before the base class adjusts
-  // the window region.
-  app_window_->OnCanHaveAlphaEnabledChanged();
-  UpdateDWMFrame();
-  DesktopWindowTreeHostWin::HandleFrameChanged();
-}
-
-void AppWindowDesktopWindowTreeHostWin::PostHandleMSG(UINT message,
-                                                      WPARAM w_param,
-                                                      LPARAM l_param) {
-  switch (message) {
-    case WM_WINDOWPOSCHANGED: {
-      UpdateDWMFrame();
-      break;
-    }
-  }
-}
-
-void AppWindowDesktopWindowTreeHostWin::UpdateDWMFrame() {
-  if (!GetWidget()->client_view() || !app_window_->glass_frame_view())
-    return;
-
-  MARGINS margins = {0};
+bool AppWindowDesktopWindowTreeHostWin::GetDwmFrameInsetsInPixels(
+    gfx::Insets* insets) const {
+  // If there's no glass view we never need to change DWM frame insets.
+  if (!GetWidget()->client_view() || !app_window_->glass_frame_view() ||
+      !DesktopWindowTreeHostWin::ShouldUseNativeFrame())
+    return false;
 
-  // 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 (app_window_->glass_frame_view()) {
-    gfx::Insets insets = app_window_->glass_frame_view()->GetGlassInsets();
+  if (GetWidget()->IsFullscreen()) {
+    *insets = gfx::Insets();
+  } else {
+    // If the opaque frame is visible, we use the default (zero) margins.
+    // Otherwise, we need to figure out how to extend the glass in.
+    *insets = app_window_->glass_frame_view()->GetGlassInsets();
     // The DWM API's expect values in pixels. We need to convert from DIP to
     // pixels here.
-    insets = insets.Scale(display::win::GetDPIScale());
-    margins.cxLeftWidth = insets.left();
-    margins.cxRightWidth = insets.right();
-    margins.cyBottomHeight = insets.bottom();
-    margins.cyTopHeight = insets.top();
+    *insets = insets->Scale(display::win::GetDPIScale());
   }
+  return true;
+}
 
-  DwmExtendFrameIntoClientArea(GetHWND(), &margins);
+void AppWindowDesktopWindowTreeHostWin::HandleFrameChanged() {
+  // We need to update the glass region on or off before the base class adjusts
+  // the window region.
+  app_window_->OnCanHaveAlphaEnabledChanged();
+  DesktopWindowTreeHostWin::HandleFrameChanged();
 }

chrome/browser/ui/views/apps/app_window_desktop_window_tree_host_win.h

@@ -29,12 +29,8 @@ class AppWindowDesktopWindowTreeHostWin
   // Overridden from DesktopWindowTreeHostWin:
   bool GetClientAreaInsets(gfx::Insets* insets,
                            HMONITOR monitor) const override;
+  bool GetDwmFrameInsetsInPixels(gfx::Insets* insets) const override;
   void HandleFrameChanged() override;
-  void PostHandleMSG(UINT message, WPARAM w_param, LPARAM l_param) override;
-
-  // Updates the glass frame area by calling the DwmExtendFrameIntoClientArea
-  // Windows function.
-  void UpdateDWMFrame();
 
   ChromeNativeAppWindowViewsWin* app_window_;
 

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

@@ -4,7 +4,10 @@
 
 #include "chrome/browser/ui/views/frame/browser_desktop_window_tree_host_win.h"
 
+#include <windows.h>
+
 #include <dwmapi.h>
+#include <uxtheme.h>
 
 #include "base/macros.h"
 #include "base/process/process_handle.h"
@@ -36,9 +39,7 @@ BrowserDesktopWindowTreeHostWin::BrowserDesktopWindowTreeHostWin(
     : DesktopWindowTreeHostWin(native_widget_delegate,
                                desktop_native_widget_aura),
       browser_view_(browser_view),
-      browser_frame_(browser_frame),
-      did_gdi_clear_(false) {
-}
+      browser_frame_(browser_frame) {}
 
 BrowserDesktopWindowTreeHostWin::~BrowserDesktopWindowTreeHostWin() {
 }
@@ -107,6 +108,44 @@ bool BrowserDesktopWindowTreeHostWin::GetClientAreaInsets(
   return true;
 }
 
+bool BrowserDesktopWindowTreeHostWin::GetDwmFrameInsetsInPixels(
+    gfx::Insets* insets) const {
+  // For "normal" windows on Aero, we always need to reset the glass area
+  // correctly, even if we're not currently showing the native frame (e.g.
+  // because a theme is showing), so we explicitly check for that case rather
+  // than checking ShouldUseNativeFrame() here.  Using that here would mean we
+  // wouldn't reset the glass area to zero when moving from the native frame to
+  // an opaque frame, leading to graphical glitches behind the opaque frame.
+  // Instead, we use that function below to tell us whether the frame is
+  // currently native or opaque.
+  if (!GetWidget()->client_view() || !browser_view_->IsBrowserTypeNormal() ||
+      !DesktopWindowTreeHostWin::ShouldUseNativeFrame())
+    return false;
+
+  // Don't extend the glass in at all if it won't be visible.
+  if (!ShouldUseNativeFrame() || GetWidget()->IsFullscreen() ||
+      ShouldCustomDrawSystemTitlebar()) {
+    *insets = gfx::Insets();
+  } else {
+    // The glass should extend to the bottom of the tabstrip.
+    HWND hwnd = GetHWND();
+    gfx::Rect tabstrip_bounds(
+        browser_frame_->GetBoundsForTabStrip(browser_view_->tabstrip()));
+    tabstrip_bounds =
+        display::win::ScreenWin::DIPToClientRect(hwnd, tabstrip_bounds);
+
+    // The 2 px (not DIP) at the inner edges of Win 7 glass are a light and dark
+    // line, so we must inset further to account for those.
+    constexpr int kWin7GlassInset = 2;
+    const int inset = (base::win::GetVersion() < base::win::VERSION_WIN8)
+                          ? kWin7GlassInset
+                          : 0;
+    *insets =
+        gfx::Insets(tabstrip_bounds.bottom() + inset, inset, inset, inset);
+  }
+  return true;
+}
+
 void BrowserDesktopWindowTreeHostWin::HandleCreate() {
   DesktopWindowTreeHostWin::HandleCreate();
   browser_window_property_manager_ =
@@ -123,10 +162,6 @@ void BrowserDesktopWindowTreeHostWin::HandleFrameChanged() {
   // Reinitialize the status bubble, since it needs to be initialized
   // differently depending on whether or not DWM composition is enabled
   browser_view_->InitStatusBubble();
-
-  // We need to update the glass region on or off before the base class adjusts
-  // the window region.
-  UpdateDWMFrame();
   DesktopWindowTreeHostWin::HandleFrameChanged();
 }
 
@@ -159,14 +194,11 @@ bool BrowserDesktopWindowTreeHostWin::PreHandleMSG(UINT message,
 void BrowserDesktopWindowTreeHostWin::PostHandleMSG(UINT message,
                                                     WPARAM w_param,
                                                     LPARAM l_param) {
-  HWND hwnd = GetHWND();
   switch (message) {
     case WM_CREATE:
-      minimize_button_metrics_.Init(hwnd);
+      minimize_button_metrics_.Init(GetHWND());
       break;
     case WM_WINDOWPOSCHANGED: {
-      UpdateDWMFrame();
-
       // Windows lies to us about the position of the minimize button before a
       // window is visible. We use this position to place the incognito avatar
       // in RTL mode, so when the window is shown, we need to re-layout and
@@ -185,27 +217,6 @@ void BrowserDesktopWindowTreeHostWin::PostHandleMSG(UINT message,
       }
       break;
     }
-    case WM_ERASEBKGND: {
-      gfx::Insets insets;
-      if (!did_gdi_clear_ &&
-          GetClientAreaInsets(
-              &insets, MonitorFromWindow(hwnd, MONITOR_DEFAULTTONEAREST))) {
-        // This is necessary to avoid white flashing in the titlebar area around
-        // the minimize/maximize/close buttons.
-        DCHECK_EQ(0, insets.top());
-        HDC dc = GetDC(hwnd);
-        MARGINS margins = GetDWMFrameMargins();
-        RECT client_rect;
-        GetClientRect(hwnd, &client_rect);
-        HBRUSH brush = CreateSolidBrush(0);
-        RECT rect = {0, 0, client_rect.right, margins.cyTopHeight};
-        FillRect(dc, &rect, brush);
-        DeleteObject(brush);
-        ReleaseDC(hwnd, dc);
-        did_gdi_clear_ = true;
-      }
-      break;
-    }
     case WM_DWMCOLORIZATIONCOLORCHANGED: {
       // The activation border may have changed color.
       views::NonClientView* non_client_view = GetWidget()->non_client_view();
@@ -268,53 +279,8 @@ bool BrowserDesktopWindowTreeHostWin::ShouldWindowContentsBeTransparent()
          views::DesktopWindowTreeHostWin::ShouldWindowContentsBeTransparent();
 }
 
-void BrowserDesktopWindowTreeHostWin::FrameTypeChanged() {
-  views::DesktopWindowTreeHostWin::FrameTypeChanged();
-  did_gdi_clear_ = false;
-}
-
 ////////////////////////////////////////////////////////////////////////////////
 // BrowserDesktopWindowTreeHostWin, private:
-
-void BrowserDesktopWindowTreeHostWin::UpdateDWMFrame() {
-  // For "normal" windows on Aero, we always need to reset the glass area
-  // correctly, even if we're not currently showing the native frame (e.g.
-  // because a theme is showing), so we explicitly check for that case rather
-  // than checking browser_frame_->ShouldUseNativeFrame() here.  Using that here
-  // would mean we wouldn't reset the glass area to zero when moving from the
-  // native frame to an opaque frame, leading to graphical glitches behind the
-  // opaque frame.  Instead, we use that function below to tell us whether the
-  // frame is currently native or opaque.
-  if (!GetWidget()->client_view() || !browser_view_->IsBrowserTypeNormal() ||
-      !DesktopWindowTreeHostWin::ShouldUseNativeFrame())
-    return;
-
-  MARGINS margins = GetDWMFrameMargins();
-
-  DwmExtendFrameIntoClientArea(GetHWND(), &margins);
-}
-
-MARGINS BrowserDesktopWindowTreeHostWin::GetDWMFrameMargins() const {
-  // Don't extend the glass in at all if it won't be visible.
-  if (!ShouldUseNativeFrame() || GetWidget()->IsFullscreen() ||
-      ShouldCustomDrawSystemTitlebar())
-    return MARGINS{0};
-
-  // The glass should extend to the bottom of the tabstrip.
-  HWND hwnd = GetHWND();
-  gfx::Rect tabstrip_bounds(
-      browser_frame_->GetBoundsForTabStrip(browser_view_->tabstrip()));
-  tabstrip_bounds =
-      display::win::ScreenWin::DIPToClientRect(hwnd, tabstrip_bounds);
-
-  // The 2 px (not DIP) at the inner edges of Win 7 glass are a light and dark
-  // line, so we must inset further to account for those.
-  constexpr int kWin7GlassInset = 2;
-  const int inset =
-      (base::win::GetVersion() < base::win::VERSION_WIN8) ? kWin7GlassInset : 0;
-  return MARGINS{inset, inset, tabstrip_bounds.bottom() + inset, inset};
-}
-
 bool BrowserDesktopWindowTreeHostWin::IsOpaqueHostedAppFrame() const {
   // TODO(https://crbug.com/868239): Support Windows 7 Aero glass for hosted app
   // window titlebar controls.

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

@@ -5,9 +5,6 @@
 #ifndef CHROME_BROWSER_UI_VIEWS_FRAME_BROWSER_DESKTOP_WINDOW_TREE_HOST_WIN_H_
 #define CHROME_BROWSER_UI_VIEWS_FRAME_BROWSER_DESKTOP_WINDOW_TREE_HOST_WIN_H_
 
-#include <windows.h>
-#include <uxtheme.h>
-
 #include "base/macros.h"
 #include "chrome/browser/ui/views/frame/browser_desktop_window_tree_host.h"
 #include "chrome/browser/ui/views/frame/minimize_button_metrics_win.h"
@@ -44,6 +41,7 @@ class BrowserDesktopWindowTreeHostWin : public BrowserDesktopWindowTreeHost,
   int GetInitialShowState() const override;
   bool GetClientAreaInsets(gfx::Insets* insets,
                            HMONITOR monitor) const override;
+  bool GetDwmFrameInsetsInPixels(gfx::Insets* insets) const override;
   void HandleCreate() override;
   void HandleDestroying() override;
   void HandleFrameChanged() override;
@@ -56,10 +54,6 @@ class BrowserDesktopWindowTreeHostWin : public BrowserDesktopWindowTreeHost,
   views::FrameMode GetFrameMode() const override;
   bool ShouldUseNativeFrame() const override;
   bool ShouldWindowContentsBeTransparent() const override;
-  void FrameTypeChanged() override;
-
-  void UpdateDWMFrame();
-  MARGINS GetDWMFrameMargins() const;
 
   bool IsOpaqueHostedAppFrame() const;
 
@@ -74,9 +68,6 @@ class BrowserDesktopWindowTreeHostWin : public BrowserDesktopWindowTreeHost,
   // The wrapped system menu itself.
   std::unique_ptr<views::NativeMenuWin> system_menu_;
 
-  // Necessary to avoid corruption on NC paint in Aero mode.
-  bool did_gdi_clear_;
-
   DISALLOW_COPY_AND_ASSIGN(BrowserDesktopWindowTreeHostWin);
 };
 

ui/views/widget/desktop_aura/desktop_window_tree_host_win.cc

@@ -735,6 +735,11 @@ bool DesktopWindowTreeHostWin::GetClientAreaInsets(gfx::Insets* insets,
   return false;
 }
 
+bool DesktopWindowTreeHostWin::GetDwmFrameInsetsInPixels(
+    gfx::Insets* insets) const {
+  return false;
+}
+
 void DesktopWindowTreeHostWin::GetMinMaxSize(gfx::Size* min_size,
                                              gfx::Size* max_size) const {
   *min_size = native_widget_delegate_->GetMinimumSize();

ui/views/widget/desktop_aura/desktop_window_tree_host_win.h

@@ -172,6 +172,7 @@ class VIEWS_EXPORT DesktopWindowTreeHostWin
   void GetWindowMask(const gfx::Size& size, gfx::Path* path) override;
   bool GetClientAreaInsets(gfx::Insets* insets,
                            HMONITOR monitor) const override;
+  bool GetDwmFrameInsetsInPixels(gfx::Insets* insets) const override;
   void GetMinMaxSize(gfx::Size* min_size, gfx::Size* max_size) const override;
   gfx::Size GetRootViewSize() const override;
   gfx::Size DIPToScreenSize(const gfx::Size& dip_size) const override;

ui/views/win/hwnd_message_handler.cc

@@ -1689,6 +1689,18 @@ void HWNDMessageHandler::OnEnterSizeMove() {
 }
 
 LRESULT HWNDMessageHandler::OnEraseBkgnd(HDC dc) {
+  gfx::Insets insets;
+  if (ui::win::IsAeroGlassEnabled() &&
+      delegate_->GetDwmFrameInsetsInPixels(&insets) && !insets.IsEmpty()) {
+    // This is necessary to avoid white flashing in the titlebar area around the
+    // minimize/maximize/close buttons.
+    RECT client_rect;
+    GetClientRect(hwnd(), &client_rect);
+    base::win::ScopedGDIObject<HBRUSH> brush(CreateSolidBrush(0));
+    // The DC and GetClientRect operate in client area coordinates.
+    RECT rect = {0, 0, client_rect.right, insets.top()};
+    FillRect(dc, &rect, brush.get());
+  }
   // Needed to prevent resize flicker.
   return 1;
 }
@@ -2747,7 +2759,7 @@ void HWNDMessageHandler::OnWindowPosChanged(WINDOWPOS* window_pos) {
   } else if (window_pos->flags & SWP_HIDEWINDOW) {
     delegate_->HandleVisibilityChanged(false);
   }
-
+  UpdateDwmFrame();
   SetMsgHandled(FALSE);
 }
 
@@ -3105,6 +3117,9 @@ void HWNDMessageHandler::PerformDwmTransition() {
   ResetWindowRegion(true, false);
   // The non-client view needs to update too.
   delegate_->HandleFrameChanged();
+  // This calls DwmExtendFrameIntoClientArea which must be called when DWM
+  // composition state changes.
+  UpdateDwmFrame();
 
   if (IsVisible() && IsFrameSystemDrawn()) {
     // For some reason, we need to hide the window after we change from a custom
@@ -3126,6 +3141,16 @@ void HWNDMessageHandler::PerformDwmTransition() {
   EnumChildWindows(hwnd(), &SendDwmCompositionChanged, NULL);
 }
 
+void HWNDMessageHandler::UpdateDwmFrame() {
+  gfx::Insets insets;
+  if (ui::win::IsAeroGlassEnabled() &&
+      delegate_->GetDwmFrameInsetsInPixels(&insets)) {
+    MARGINS margins = {insets.left(), insets.right(), insets.top(),
+                       insets.bottom()};
+    DwmExtendFrameIntoClientArea(hwnd(), &margins);
+  }
+}
+
 void HWNDMessageHandler::GenerateTouchEvent(ui::EventType event_type,
                                             const gfx::Point& point,
                                             size_t id,

ui/views/win/hwnd_message_handler.h

@@ -532,6 +532,9 @@ class VIEWS_EXPORT HWNDMessageHandler : public gfx::WindowImpl,
   // Provides functionality to transition a frame to DWM.
   void PerformDwmTransition();
 
+  // Updates DWM frame to extend into client area if needed.
+  void UpdateDwmFrame();
+
   // Generates a touch event and adds it to the |touch_events| parameter.
   // |point| is the point where the touch was initiated.
   // |id| is the event id associated with the touch event.

ui/views/win/hwnd_message_handler_delegate.h

@@ -94,6 +94,10 @@ class VIEWS_EXPORT HWNDMessageHandlerDelegate {
   virtual bool GetClientAreaInsets(gfx::Insets* insets,
                                    HMONITOR monitor) const = 0;
 
+  // Returns true if DWM frame should be extended into client area by |insets|.
+  // Insets are specified in screen pixels not DIP because that's what DWM uses.
+  virtual bool GetDwmFrameInsetsInPixels(gfx::Insets* insets) const = 0;
+
   // Returns the minimum and maximum size the window can be resized to by the
   // user.
   virtual void GetMinMaxSize(gfx::Size* min_size,