Get rid of the 1-pixel client inset hack.

ui/views/win/hwnd_message_handler.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 "ui/views/win/hwnd_message_handler.h"
 
 #include <dwmapi.h>
 #include <shellapi.h>
 
 #include "base/bind.h"
@@ skipping 281 matching lines @@
   }
 
   return false;
 }
 
 #endif
 
 // The thickness of an auto-hide taskbar in pixels.
 const int kAutoHideTaskbarThicknessPx = 2;
 
-// For windows with the standard frame removed, the client area needs to be
-// different from the window area to avoid a "feature" in Windows's handling of
-// WM_NCCALCSIZE data. See the comment near the bottom of GetClientAreaInsets
-// for more details.
-const int kClientAreaBottomInsetHack = -1;
-
 }  // namespace
 
 // A scoping class that prevents a window from being able to redraw in response
 // to invalidations that may occur within it for the lifetime of the object.
 //
 // Why would we want such a thing? Well, it turns out Windows has some
 // "unorthodox" behavior when it comes to painting its non-client areas.
 // Occasionally, Windows will paint portions of the default non-client area
 // right over the top of the custom frame. This is not simply fixed by handling
 // WM_NCPAINT/WM_PAINT, with some investigation it turns out that this
@@ skipping 461 matching lines @@
     // change from opaque to glass. "Non client rendering enabled" means that
     // the DWM's glass non-client rendering is enabled, which is why
     // DWMNCRP_ENABLED is used for the native frame case. _DISABLED means the
     // DWM doesn't render glass, and so is used in the custom frame case.
     DWMNCRENDERINGPOLICY policy = !delegate_->IsUsingCustomFrame() ?
         DWMNCRP_ENABLED : DWMNCRP_DISABLED;
     DwmSetWindowAttribute(hwnd(), DWMWA_NCRENDERING_POLICY,
                           &policy, sizeof(DWMNCRENDERINGPOLICY));
   }
 
-  ResetWindowRegion(true);
+  // Don't redraw the window here, because we need to hide and show the window
+  // which will also trigger a redraw.
+  ResetWindowRegion(true, false);
+
+  ::SetWindowPos(hwnd(), NULL, 0, 0, 0, 0,
+      SWP_NOMOVE | SWP_NOSIZE | SWP_FRAMECHANGED);

[Peter Kasting, 2013/11/12 01:47:03]

Nit: All lines of args should begin at the same horizontal location  (3 places)

 
   // The non-client view needs to update too.

[Peter Kasting, 2013/11/12 01:47:03]

Does this explicitly need to happen before some of the code below?  If so note
what restrictions apply so no one ever reintroduces subtle bugs with later
refactorings.

   delegate_->HandleFrameChanged();
 
+  // For some reason, we need to hide the window after we change the frame type.
+  // If we don't, the client area will be filled with black.  This is somehow
+  // related to an interaction between SetWindowRgn and Dwm, but it's not clear
+  // exactly what.
+  if (IsVisible()) {
+    SetWindowPos(hwnd(), NULL, 0, 0, 0, 0,
+        SWP_NOMOVE | SWP_NOSIZE | SWP_HIDEWINDOW);
+    SetWindowPos(hwnd(), NULL, 0, 0, 0, 0,
+        SWP_NOMOVE | SWP_NOSIZE | SWP_SHOWWINDOW);

[Peter Kasting, 2013/11/12 01:47:03]

How are these calls different than what you could do with a pair of ShowWindow()
calls with SW_HIDE and SW_SHOW?

+  }
+
+  UpdateWindow(hwnd());
+
   // WM_DWMCOMPOSITIONCHANGED is only sent to top level windows, however we want
   // to notify our children too, since we can have MDI child windows who need to
   // update their appearance.
   EnumChildWindows(hwnd(), &SendDwmCompositionChanged, NULL);
 }
 
 void HWNDMessageHandler::SchedulePaintInRect(const gfx::Rect& rect) {
   if (use_layered_buffer_) {
     // We must update the back-buffer immediately, since Windows' handling of
     // invalid rects is somewhat mysterious.
@@ skipping 232 matching lines @@
     TrackMouseEvents(active_mouse_tracking_flags_ | TME_CANCEL);
     TrackMouseEvents(mouse_tracking_flags);
   }
 }
 
 void HWNDMessageHandler::ClientAreaSizeChanged() {
   RECT r = {0, 0, 0, 0};
   // In case of minimized window GetWindowRect can return normally unexpected
   // coordinates.
   if (!IsMinimized()) {
-    if (delegate_->WidgetSizeIsClientSize()) {
+    if (delegate_->WidgetSizeIsClientSize())
       GetClientRect(hwnd(), &r);
-      gfx::Insets insets;
-      bool got_insets = GetClientAreaInsets(&insets);
-      if (got_insets) {
-        // This is needed due to a hack that works around a "feature" in
-        // Windows's handling of WM_NCCALCSIZE. See the comment near the end of
-        // GetClientAreaInsets for more details.
-        if ((remove_standard_frame_ && !IsMaximized()) ||
-            !fullscreen_handler_->fullscreen()) {
-          r.bottom += kClientAreaBottomInsetHack;
-        }
-      }
-    } else {
+    else
       GetWindowRect(hwnd(), &r);
-    }
   }
   gfx::Size s(std::max(0, static_cast<int>(r.right - r.left)),
               std::max(0, static_cast<int>(r.bottom - r.top)));
   delegate_->HandleClientSizeChanged(s);
   if (use_layered_buffer_)
     layered_window_contents_.reset(new gfx::Canvas(s, 1.0f, false));
 }
 
 bool HWNDMessageHandler::GetClientAreaInsets(gfx::Insets* insets) const {
   if (delegate_->GetClientAreaInsets(insets))
@@ skipping 11 matching lines @@
     // Windows automatically adds a standard width border to all sides when a
     // window is maximized.
     int border_thickness = GetSystemMetrics(SM_CXSIZEFRAME);
     if (remove_standard_frame_)
       border_thickness -= 1;
     *insets = gfx::Insets(
         border_thickness, border_thickness, border_thickness, border_thickness);
     return true;
   }
 
-  // Returning empty insets for a window with the standard frame removed seems
-  // to cause Windows to treat the window specially, treating black as
-  // transparent and changing around some of the painting logic. I suspect it's
-  // some sort of horrible backwards-compatability hack, but the upshot of it
-  // is that if the insets are empty then in certain conditions (it seems to
-  // be subtly related to timing), the contents of windows with the standard
-  // frame removed will flicker to transparent during resize.
-  //
-  // To work around this, we increase the size of the client area by 1px
-  // *beyond* the bottom of the window. This prevents Windows from having a
-  // hissy fit and flashing the window incessantly during resizes, but it also
-  // means that the client area is reported 1px larger than it really is, so
-  // user code has to compensate by making its content shorter if it wants
-  // everything to appear inside the window.
-  if (remove_standard_frame_) {
-    *insets =
-        gfx::Insets(0, 0, IsMaximized() ? 0 : kClientAreaBottomInsetHack, 0);
-    return true;
-  }
-
-  // This is weird, but highly essential. If we don't offset the bottom edge
-  // of the client rect, the window client area and window area will match,
-  // and when returning to glass rendering mode from non-glass, the client
-  // area will not paint black as transparent. This is because (and I don't
-  // know why) the client area goes from matching the window rect to being
-  // something else. If the client area is not the window rect in both
-  // modes, the blackness doesn't occur. Because of this, we need to tell
-  // the RootView to lay out to fit the window rect, rather than the client
-  // rect when using the opaque frame.
-  // Note: this is only required for non-fullscreen windows. Note that
-  // fullscreen windows are in restored state, not maximized.
-  // Note that previously we used to inset by 1 instead of outset, but that
-  // doesn't work with Aura: http://crbug.com/172099 http://crbug.com/277228
-  *insets = gfx::Insets(
-      0, 0,
-      fullscreen_handler_->fullscreen() ? 0 : kClientAreaBottomInsetHack, 0);
+  *insets = gfx::Insets(0, 0, 0, 0);

[Peter Kasting, 2013/11/12 01:47:03]

Nit: Just use gfx::Insets()

   return true;
 }
 
-void HWNDMessageHandler::ResetWindowRegion(bool force) {
+void HWNDMessageHandler::ResetWindowRegion(bool force, bool redraw) {
   // A native frame uses the native window region, and we don't want to mess
   // with it.
   // WS_EX_COMPOSITED is used instead of WS_EX_LAYERED under aura. WS_EX_LAYERED
   // automatically makes clicks on transparent pixels fall through, that isn't
   // the case with WS_EX_COMPOSITED. So, we route WS_EX_COMPOSITED through to
   // the delegate to allow for a custom hit mask.
   if ((window_ex_style() & WS_EX_COMPOSITED) == 0 &&
       (!delegate_->IsUsingCustomFrame() || !delegate_->IsWidgetWindow())) {
     if (force)
-      SetWindowRgn(hwnd(), NULL, TRUE);
+      SetWindowRgn(hwnd(), NULL, redraw);
     return;
   }
 
   // Changing the window region is going to force a paint. Only change the
   // window region if the region really differs.
   HRGN current_rgn = CreateRectRgn(0, 0, 0, 0);
   int current_rgn_result = GetWindowRgn(hwnd(), current_rgn);
 
   CRect window_rect;
   GetWindowRect(hwnd(), &window_rect);
   HRGN new_region;
   if (IsMaximized()) {
     HMONITOR monitor = MonitorFromWindow(hwnd(), MONITOR_DEFAULTTONEAREST);
     MONITORINFO mi;
     mi.cbSize = sizeof mi;
     base::win::GetMonitorInfoWrapper(monitor, &mi);
     CRect work_rect = mi.rcWork;
     work_rect.OffsetRect(-window_rect.left, -window_rect.top);
     new_region = CreateRectRgnIndirect(&work_rect);
   } else {
     gfx::Path window_mask;
     delegate_->GetWindowMask(
         gfx::Size(window_rect.Width(), window_rect.Height()), &window_mask);
     new_region = gfx::CreateHRGNFromSkPath(window_mask);
   }
 
   if (current_rgn_result == ERROR || !EqualRgn(current_rgn, new_region)) {
     // SetWindowRgn takes ownership of the HRGN created by CreateNativeRegion.
-    SetWindowRgn(hwnd(), new_region, TRUE);
+    SetWindowRgn(hwnd(), new_region, redraw);
   } else {
     DeleteObject(new_region);
   }
 
   DeleteObject(current_rgn);
 }
 
 LRESULT HWNDMessageHandler::DefWindowProcWithRedrawLock(UINT message,
                                                         WPARAM w_param,
                                                         LPARAM l_param) {
@@ skipping 192 matching lines @@
   delegate_->HandleDisplayChange();
 }
 
 LRESULT HWNDMessageHandler::OnDwmCompositionChanged(UINT msg,
                                                     WPARAM w_param,
                                                     LPARAM l_param) {
   if (!delegate_->IsWidgetWindow()) {
     SetMsgHandled(FALSE);
     return 0;
   }
-  // For some reason, we need to hide the window while we're changing the frame
-  // type only when we're changing it in response to WM_DWMCOMPOSITIONCHANGED.
-  // If we don't, the client area will be filled with black. I'm suspecting
-  // something skia-ey.
-  // Frame type toggling caused by the user (e.g. switching theme) doesn't seem
-  // to have this requirement.
+
   FrameTypeChanged();
   return 0;
 }
 
 void HWNDMessageHandler::OnEnterSizeMove() {
   delegate_->HandleBeginWMSizeMove();
   SetMsgHandled(FALSE);
 }
 
 LRESULT HWNDMessageHandler::OnEraseBkgnd(HDC dc) {
@@ skipping 10 matching lines @@
   gfx::Size min_window_size;
   gfx::Size max_window_size;
   delegate_->GetMinMaxSize(&min_window_size, &max_window_size);
 
   // Add the native frame border size to the minimum and maximum size if the
   // view reports its size as the client size.
   if (delegate_->WidgetSizeIsClientSize()) {
     CRect client_rect, window_rect;
     GetClientRect(hwnd(), &client_rect);
     GetWindowRect(hwnd(), &window_rect);
-    // Due to the client area bottom inset hack (detailed elsewhere), adjust
-    // the reported size of the client area in the case that the standard frame
-    // has been removed.
-    if (remove_standard_frame_)
-      client_rect.bottom += kClientAreaBottomInsetHack;
     window_rect -= client_rect;
     min_window_size.Enlarge(window_rect.Width(), window_rect.Height());
     if (!max_window_size.IsEmpty())
       max_window_size.Enlarge(window_rect.Width(), window_rect.Height());
   }
   minmax_info->ptMinTrackSize.x = min_window_size.width();
   minmax_info->ptMinTrackSize.y = min_window_size.height();
   if (max_window_size.width() || max_window_size.height()) {
     if (!max_window_size.width())
       max_window_size.set_width(GetSystemMetrics(SM_CXMAXTRACK));
@@ skipping 586 matching lines @@
     if (flags == SPI_SETWORKAREA)
       delegate_->HandleWorkAreaChanged();
     SetMsgHandled(FALSE);
   }
 }
 
 void HWNDMessageHandler::OnSize(UINT param, const CSize& size) {
   RedrawWindow(hwnd(), NULL, NULL, RDW_INVALIDATE | RDW_ALLCHILDREN);
   // ResetWindowRegion is going to trigger WM_NCPAINT. By doing it after we've
   // invoked OnSize we ensure the RootView has been laid out.
-  ResetWindowRegion(false);
+  ResetWindowRegion(false, true);
 }
 
 void HWNDMessageHandler::OnSysCommand(UINT notification_code,
                                       const CPoint& point) {
   if (!delegate_->ShouldHandleSystemCommands())
     return;
 
   // Windows uses the 4 lower order bits of |notification_code| for type-
   // specific information so we must exclude this when comparing.
   static const int sc_mask = 0xFFF0;
@@ skipping 199 matching lines @@
     delegate_->HandleVisibilityChanged(false);
   SetMsgHandled(FALSE);
 }
 
 void HWNDMessageHandler::HandleTouchEvents(const TouchEvents& touch_events) {
   for (size_t i = 0; i < touch_events.size(); ++i)
     delegate_->HandleTouchEvent(touch_events[i]);
 }
 
 }  // namespace views