fix: allow chromium to handle WM_NCCALCSIZE for frameless windows (#21164) (#21205)

filenames.gni

@@ -353,6 +353,7 @@ filenames = {
     "shell/browser/ui/tray_icon_observer.h",
     "shell/browser/ui/tray_icon_win.cc",
     "shell/browser/ui/views/atom_views_delegate.cc",
+    "shell/browser/ui/views/atom_views_delegate_win.cc",
     "shell/browser/ui/views/atom_views_delegate.h",
     "shell/browser/ui/views/autofill_popup_view.cc",
     "shell/browser/ui/views/autofill_popup_view.h",

shell/browser/native_window_views_win.cc

@@ -143,162 +143,28 @@ bool IsScreenReaderActive() {
   return screenReader && UiaClientsAreListening();
 }
 
-// We use "enum" instead of "enum class" because we need to do bitwise compare.
-enum AppbarAutohideEdge {
-  TOP = 1 << 0,
-  LEFT = 1 << 1,
-  BOTTOM = 1 << 2,
-  RIGHT = 1 << 3,
-};
-
-// The thickness of an auto-hide taskbar in pixel.
-constexpr int kAutoHideTaskbarThicknessPx = 2;
-
-// Code is copied from chrome_views_delegate_win.cc.
-bool MonitorHasAutohideTaskbarForEdge(UINT edge, HMONITOR monitor) {
-  APPBARDATA taskbar_data = {sizeof(APPBARDATA), NULL, 0, edge};
-  taskbar_data.hWnd = ::GetForegroundWindow();
-
-  // MSDN documents an ABM_GETAUTOHIDEBAREX, which supposedly takes a monitor
-  // rect and returns autohide bars on that monitor.  This sounds like a good
-  // idea for multi-monitor systems.  Unfortunately, it appears to not work at
-  // least some of the time (erroneously returning NULL) and there's almost no
-  // online documentation or other sample code using it that suggests ways to
-  // address this problem. We do the following:-
-  // 1. Use the ABM_GETAUTOHIDEBAR message. If it works, i.e. returns a valid
-  //    window we are done.
-  // 2. If the ABM_GETAUTOHIDEBAR message does not work we query the auto hide
-  //    state of the taskbar and then retrieve its position. That call returns
-  //    the edge on which the taskbar is present. If it matches the edge we
-  //    are looking for, we are done.
-  // NOTE: This call spins a nested run loop.
-  HWND taskbar = reinterpret_cast<HWND>(
-      SHAppBarMessage(ABM_GETAUTOHIDEBAR, &taskbar_data));
-  if (!::IsWindow(taskbar)) {
-    APPBARDATA taskbar_data = {sizeof(APPBARDATA), 0, 0, 0};
-    unsigned int taskbar_state = SHAppBarMessage(ABM_GETSTATE, &taskbar_data);
-    if (!(taskbar_state & ABS_AUTOHIDE))
-      return false;
-
-    taskbar_data.hWnd = ::FindWindow(L"Shell_TrayWnd", NULL);
-    if (!::IsWindow(taskbar_data.hWnd))
-      return false;
-
-    SHAppBarMessage(ABM_GETTASKBARPOS, &taskbar_data);
-    if (taskbar_data.uEdge == edge)
-      taskbar = taskbar_data.hWnd;
-  }
-
-  // There is a potential race condition here:
-  // 1. A maximized chrome window is fullscreened.
-  // 2. It is switched back to maximized.
-  // 3. In the process the window gets a WM_NCCACLSIZE message which calls us to
-  //    get the autohide state.
-  // 4. The worker thread is invoked. It calls the API to get the autohide
-  //    state. On Windows versions  earlier than Windows 7, taskbars could
-  //    easily be always on top or not.
-  //    This meant that we only want to look for taskbars which have the topmost
-  //    bit set.  However this causes problems in cases where the window on the
-  //    main thread is still in the process of switching away from fullscreen.
-  //    In this case the taskbar might not yet have the topmost bit set.
-  // 5. The main thread resumes and does not leave space for the taskbar and
-  //    hence it does not pop when hovered.
-  //
-  // To address point 4 above, it is best to not check for the WS_EX_TOPMOST
-  // window style on the taskbar, as starting from Windows 7, the topmost
-  // style is always set. We don't support XP and Vista anymore.
-  if (::IsWindow(taskbar)) {
-    if (MonitorFromWindow(taskbar, MONITOR_DEFAULTTONEAREST) == monitor)
-      return true;
-    // In some cases like when the autohide taskbar is on the left of the
-    // secondary monitor, the MonitorFromWindow call above fails to return the
-    // correct monitor the taskbar is on. We fallback to MonitorFromPoint for
-    // the cursor position in that case, which seems to work well.
-    POINT cursor_pos = {0};
-    GetCursorPos(&cursor_pos);
-    if (MonitorFromPoint(cursor_pos, MONITOR_DEFAULTTONEAREST) == monitor)
-      return true;
-  }
-  return false;
-}
-
-int GetAppbarAutohideEdges(HWND hwnd) {
-  HMONITOR monitor = MonitorFromWindow(hwnd, MONITOR_DEFAULTTONULL);
-  if (!monitor)
-    return 0;
-
-  int edges = 0;
-  if (MonitorHasAutohideTaskbarForEdge(ABE_LEFT, monitor))
-    edges |= AppbarAutohideEdge::LEFT;
-  if (MonitorHasAutohideTaskbarForEdge(ABE_TOP, monitor))
-    edges |= AppbarAutohideEdge::TOP;
-  if (MonitorHasAutohideTaskbarForEdge(ABE_RIGHT, monitor))
-    edges |= AppbarAutohideEdge::RIGHT;
-  if (MonitorHasAutohideTaskbarForEdge(ABE_BOTTOM, monitor))
-    edges |= AppbarAutohideEdge::BOTTOM;
-  return edges;
-}
-
-void TriggerNCCalcSize(HWND hwnd) {
-  RECT rcClient;
-  ::GetWindowRect(hwnd, &rcClient);
-
-  ::SetWindowPos(hwnd, NULL, rcClient.left, rcClient.top,
-                 rcClient.right - rcClient.left, rcClient.bottom - rcClient.top,
-                 SWP_FRAMECHANGED);
-}
-
 }  // namespace
 
 std::set<NativeWindowViews*> NativeWindowViews::forwarding_windows_;
 HHOOK NativeWindowViews::mouse_hook_ = NULL;
 
 void NativeWindowViews::Maximize() {
-  int autohide_edges = 0;
-  if (!has_frame())
-    autohide_edges = GetAppbarAutohideEdges(GetAcceleratedWidget());
-
   // Only use Maximize() when:
   // 1. window has WS_THICKFRAME style;
   // 2. and window is not frameless when there is autohide taskbar.
-  if ((::GetWindowLong(GetAcceleratedWidget(), GWL_STYLE) & WS_THICKFRAME) &&
-      (has_frame() || autohide_edges == 0)) {
+  if (::GetWindowLong(GetAcceleratedWidget(), GWL_STYLE) & WS_THICKFRAME) {
     if (IsVisible())
       widget()->Maximize();
     else
       widget()->native_widget_private()->Show(ui::SHOW_STATE_MAXIMIZED,
                                               gfx::Rect());
     return;
+  } else {
+    restore_bounds_ = GetBounds();
+    auto display =
+        display::Screen::GetScreen()->GetDisplayNearestPoint(GetPosition());
+    SetBounds(display.work_area(), false);
   }
-
-  gfx::Insets insets;
-  if (!has_frame()) {
-    // When taskbar is autohide, we need to leave some space so the window
-    // isn't treated as a "fullscreen app", which would cause the taskbars
-    // to disappear.
-    //
-    // This trick comes from hwnd_message_handler.cc. While Chromium already
-    // does this for normal window, somehow it is not applying the trick when
-    // using frameless window, and we have to do it ourselves.
-    float scale_factor =
-        display::win::ScreenWin::GetScaleFactorForHWND(GetAcceleratedWidget());
-    int thickness = std::ceil(kAutoHideTaskbarThicknessPx / scale_factor);
-    if (autohide_edges & AppbarAutohideEdge::LEFT)
-      insets.set_left(-thickness);
-    if (autohide_edges & AppbarAutohideEdge::TOP)
-      insets.set_top(-thickness);
-    if (autohide_edges & AppbarAutohideEdge::RIGHT)
-      insets.set_right(thickness);
-    if (autohide_edges & AppbarAutohideEdge::BOTTOM)
-      insets.set_bottom(thickness);
-  }
-
-  restore_bounds_ = GetBounds();
-  auto display =
-      display::Screen::GetScreen()->GetDisplayNearestPoint(GetPosition());
-  gfx::Rect bounds = display.work_area();
-  bounds.Inset(insets);
-  SetBounds(bounds, false);
 }
 
 bool NativeWindowViews::ExecuteWindowsCommand(int command_id) {
@@ -363,45 +229,6 @@ bool NativeWindowViews::PreHandleMSG(UINT message,
 
       return false;
     }
-    case WM_NCCALCSIZE: {
-      if (!has_frame() && w_param == TRUE) {
-        NCCALCSIZE_PARAMS* params =
-            reinterpret_cast<NCCALCSIZE_PARAMS*>(l_param);
-        RECT PROPOSED = params->rgrc[0];
-        RECT BEFORE = params->rgrc[1];
-
-        // We need to call the default to have cascade and tile windows
-        // working
-        // (https://github.com/rossy/borderless-window/blob/master/borderless-window.c#L239),
-        // but we need to provide the proposed original value as suggested in
-        // https://blogs.msdn.microsoft.com/wpfsdk/2008/09/08/custom-window-chrome-in-wpf/
-        DefWindowProcW(GetAcceleratedWidget(), WM_NCCALCSIZE, w_param, l_param);
-
-        // When fullscreen the window has no border
-        int border = 0;
-        if (!IsFullscreen()) {
-          // When not fullscreen calculate the border size
-          border = GetSystemMetrics(SM_CXFRAME) +
-                   GetSystemMetrics(SM_CXPADDEDBORDER);
-          if (!thick_frame_) {
-            border -= GetSystemMetrics(SM_CXBORDER);
-          }
-        }
-
-        if (last_window_state_ == ui::SHOW_STATE_MAXIMIZED) {
-          // Position the top of the frame offset from where windows thinks by
-          // exactly the border amount.  When fullscreen this is 0.
-          params->rgrc[0].top = PROPOSED.top + border;
-        } else {
-          params->rgrc[0] = PROPOSED;
-          params->rgrc[1] = BEFORE;
-        }
-
-        return true;
-      } else {
-        return false;
-      }
-    }
     case WM_COMMAND:
       // Handle thumbar button click message.
       if (HIWORD(w_param) == THBN_CLICKED)
@@ -466,11 +293,6 @@ void NativeWindowViews::HandleSizeEvent(WPARAM w_param, LPARAM l_param) {
   switch (w_param) {
     case SIZE_MAXIMIZED: {
       last_window_state_ = ui::SHOW_STATE_MAXIMIZED;
-
-      if (!has_frame()) {
-        TriggerNCCalcSize(GetAcceleratedWidget());
-      }
-
       NotifyWindowMaximize();
       break;
     }
@@ -491,11 +313,6 @@ void NativeWindowViews::HandleSizeEvent(WPARAM w_param, LPARAM l_param) {
         case ui::SHOW_STATE_MAXIMIZED:
           last_window_state_ = ui::SHOW_STATE_NORMAL;
           NotifyWindowUnmaximize();
-
-          if (!has_frame()) {
-            TriggerNCCalcSize(GetAcceleratedWidget());
-          }
-
           break;
         case ui::SHOW_STATE_MINIMIZED:
           if (IsFullscreen()) {

shell/browser/ui/views/atom_views_delegate.cc

@@ -53,22 +53,7 @@ void ViewsDelegate::NotifyMenuItemFocused(const base::string16& menu_name,
                                           int item_count,
                                           bool has_submenu) {}
 
-#if defined(OS_WIN)
-HICON ViewsDelegate::GetDefaultWindowIcon() const {
-  // Use current exe's icon as default window icon.
-  return LoadIcon(GetModuleHandle(NULL),
-                  MAKEINTRESOURCE(1 /* IDR_MAINFRAME */));
-}
-
-HICON ViewsDelegate::GetSmallWindowIcon() const {
-  return GetDefaultWindowIcon();
-}
-
-bool ViewsDelegate::IsWindowInMetro(gfx::NativeWindow window) const {
-  return false;
-}
-
-#elif defined(OS_LINUX) && !defined(OS_CHROMEOS)
+#if defined(OS_LINUX) && !defined(OS_CHROMEOS)
 gfx::ImageSkia* ViewsDelegate::GetDefaultWindowIcon() const {
   return NULL;
 }

shell/browser/ui/views/atom_views_delegate.h

@@ -5,6 +5,7 @@
 #ifndef SHELL_BROWSER_UI_VIEWS_ATOM_VIEWS_DELEGATE_H_
 #define SHELL_BROWSER_UI_VIEWS_ATOM_VIEWS_DELEGATE_H_
 
+#include <map>
 #include <string>
 
 #include "base/compiler_specific.h"
@@ -37,6 +38,8 @@ class ViewsDelegate : public views::ViewsDelegate {
   HICON GetDefaultWindowIcon() const override;
   HICON GetSmallWindowIcon() const override;
   bool IsWindowInMetro(gfx::NativeWindow window) const override;
+  int GetAppbarAutohideEdges(HMONITOR monitor,
+                             base::OnceClosure callback) override;
 #elif defined(OS_LINUX) && !defined(OS_CHROMEOS)
   gfx::ImageSkia* GetDefaultWindowIcon() const override;
 #endif
@@ -50,6 +53,24 @@ class ViewsDelegate : public views::ViewsDelegate {
   bool WindowManagerProvidesTitleBar(bool maximized) override;
 
  private:
+#if defined(OS_WIN)
+  using AppbarAutohideEdgeMap = std::map<HMONITOR, int>;
+
+  // Callback on main thread with the edges. |returned_edges| is the value that
+  // was returned from the call to GetAutohideEdges() that initiated the lookup.
+  void OnGotAppbarAutohideEdges(base::OnceClosure callback,
+                                HMONITOR monitor,
+                                int returned_edges,
+                                int edges);
+
+  AppbarAutohideEdgeMap appbar_autohide_edge_map_;
+  // If true we're in the process of notifying a callback from
+  // GetAutohideEdges().start a new query.
+  bool in_autohide_edges_callback_ = false;
+
+  base::WeakPtrFactory<ViewsDelegate> weak_factory_{this};
+#endif
+
   DISALLOW_COPY_AND_ASSIGN(ViewsDelegate);
 };
 

shell/browser/ui/views/atom_views_delegate_win.cc

@@ -0,0 +1,158 @@
+// Copyright (c) 2017 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-CHROMIUM file.
+
+#include "shell/browser/ui/views/atom_views_delegate.h"
+
+#include <dwmapi.h>
+#include <shellapi.h>
+
+#include <utility>
+
+#include "base/bind.h"
+#include "base/task/post_task.h"
+
+namespace {
+
+bool MonitorHasAutohideTaskbarForEdge(UINT edge, HMONITOR monitor) {
+  APPBARDATA taskbar_data = {sizeof(APPBARDATA), NULL, 0, edge};
+  taskbar_data.hWnd = ::GetForegroundWindow();
+
+  // MSDN documents an ABM_GETAUTOHIDEBAREX, which supposedly takes a monitor
+  // rect and returns autohide bars on that monitor.  This sounds like a good
+  // idea for multi-monitor systems.  Unfortunately, it appears to not work at
+  // least some of the time (erroneously returning NULL) and there's almost no
+  // online documentation or other sample code using it that suggests ways to
+  // address this problem. We do the following:-
+  // 1. Use the ABM_GETAUTOHIDEBAR message. If it works, i.e. returns a valid
+  //    window we are done.
+  // 2. If the ABM_GETAUTOHIDEBAR message does not work we query the auto hide
+  //    state of the taskbar and then retrieve its position. That call returns
+  //    the edge on which the taskbar is present. If it matches the edge we
+  //    are looking for, we are done.
+  // NOTE: This call spins a nested run loop.
+  HWND taskbar = reinterpret_cast<HWND>(
+      SHAppBarMessage(ABM_GETAUTOHIDEBAR, &taskbar_data));
+  if (!::IsWindow(taskbar)) {
+    APPBARDATA taskbar_data = {sizeof(APPBARDATA), 0, 0, 0};
+    unsigned int taskbar_state = SHAppBarMessage(ABM_GETSTATE, &taskbar_data);
+    if (!(taskbar_state & ABS_AUTOHIDE))
+      return false;
+
+    taskbar_data.hWnd = ::FindWindow(L"Shell_TrayWnd", NULL);
+    if (!::IsWindow(taskbar_data.hWnd))
+      return false;
+
+    SHAppBarMessage(ABM_GETTASKBARPOS, &taskbar_data);
+    if (taskbar_data.uEdge == edge)
+      taskbar = taskbar_data.hWnd;
+  }
+
+  // There is a potential race condition here:
+  // 1. A maximized chrome window is fullscreened.
+  // 2. It is switched back to maximized.
+  // 3. In the process the window gets a WM_NCCACLSIZE message which calls us to
+  //    get the autohide state.
+  // 4. The worker thread is invoked. It calls the API to get the autohide
+  //    state. On Windows versions  earlier than Windows 7, taskbars could
+  //    easily be always on top or not.
+  //    This meant that we only want to look for taskbars which have the topmost
+  //    bit set.  However this causes problems in cases where the window on the
+  //    main thread is still in the process of switching away from fullscreen.
+  //    In this case the taskbar might not yet have the topmost bit set.
+  // 5. The main thread resumes and does not leave space for the taskbar and
+  //    hence it does not pop when hovered.
+  //
+  // To address point 4 above, it is best to not check for the WS_EX_TOPMOST
+  // window style on the taskbar, as starting from Windows 7, the topmost
+  // style is always set. We don't support XP and Vista anymore.
+  if (::IsWindow(taskbar)) {
+    if (MonitorFromWindow(taskbar, MONITOR_DEFAULTTONEAREST) == monitor)
+      return true;
+    // In some cases like when the autohide taskbar is on the left of the
+    // secondary monitor, the MonitorFromWindow call above fails to return the
+    // correct monitor the taskbar is on. We fallback to MonitorFromPoint for
+    // the cursor position in that case, which seems to work well.
+    POINT cursor_pos = {0};
+    GetCursorPos(&cursor_pos);
+    if (MonitorFromPoint(cursor_pos, MONITOR_DEFAULTTONEAREST) == monitor)
+      return true;
+  }
+  return false;
+}
+
+int GetAppbarAutohideEdgesOnWorkerThread(HMONITOR monitor) {
+  DCHECK(monitor);
+
+  int edges = 0;
+  if (MonitorHasAutohideTaskbarForEdge(ABE_LEFT, monitor))
+    edges |= views::ViewsDelegate::EDGE_LEFT;
+  if (MonitorHasAutohideTaskbarForEdge(ABE_TOP, monitor))
+    edges |= views::ViewsDelegate::EDGE_TOP;
+  if (MonitorHasAutohideTaskbarForEdge(ABE_RIGHT, monitor))
+    edges |= views::ViewsDelegate::EDGE_RIGHT;
+  if (MonitorHasAutohideTaskbarForEdge(ABE_BOTTOM, monitor))
+    edges |= views::ViewsDelegate::EDGE_BOTTOM;
+  return edges;
+}
+
+}  // namespace
+
+namespace electron {
+
+HICON ViewsDelegate::GetDefaultWindowIcon() const {
+  // Use current exe's icon as default window icon.
+  return LoadIcon(GetModuleHandle(NULL),
+                  MAKEINTRESOURCE(1 /* IDR_MAINFRAME */));
+}
+
+HICON ViewsDelegate::GetSmallWindowIcon() const {
+  return GetDefaultWindowIcon();
+}
+
+bool ViewsDelegate::IsWindowInMetro(gfx::NativeWindow window) const {
+  return false;
+}
+
+int ViewsDelegate::GetAppbarAutohideEdges(HMONITOR monitor,
+                                          base::OnceClosure callback) {
+  // Initialize the map with EDGE_BOTTOM. This is important, as if we return an
+  // initial value of 0 (no auto-hide edges) then we'll go fullscreen and
+  // windows will automatically remove WS_EX_TOPMOST from the appbar resulting
+  // in us thinking there is no auto-hide edges. By returning at least one edge
+  // we don't initially go fullscreen until we figure out the real auto-hide
+  // edges.
+  if (!appbar_autohide_edge_map_.count(monitor))
+    appbar_autohide_edge_map_[monitor] = EDGE_BOTTOM;
+
+  // We use the SHAppBarMessage API to get the taskbar autohide state. This API
+  // spins a modal loop which could cause callers to be reentered. To avoid
+  // that we retrieve the taskbar state in a worker thread.
+  if (monitor && !in_autohide_edges_callback_) {
+    // TODO(robliao): Annotate this task with .WithCOM() once supported.
+    // https://crbug.com/662122
+    base::PostTaskAndReplyWithResult(
+        FROM_HERE,
+        {base::ThreadPool(), base::MayBlock(),
+         base::TaskPriority::USER_BLOCKING},
+        base::BindOnce(&GetAppbarAutohideEdgesOnWorkerThread, monitor),
+        base::BindOnce(&ViewsDelegate::OnGotAppbarAutohideEdges,
+                       weak_factory_.GetWeakPtr(), std::move(callback), monitor,
+                       appbar_autohide_edge_map_[monitor]));
+  }
+  return appbar_autohide_edge_map_[monitor];
+}
+
+void ViewsDelegate::OnGotAppbarAutohideEdges(base::OnceClosure callback,
+                                             HMONITOR monitor,
+                                             int returned_edges,
+                                             int edges) {
+  appbar_autohide_edge_map_[monitor] = edges;
+  if (returned_edges == edges)
+    return;
+
+  base::AutoReset<bool> in_callback_setter(&in_autohide_edges_callback_, true);
+  std::move(callback).Run();
+}
+
+}  // namespace electron

shell/browser/ui/win/atom_desktop_window_tree_host_win.cc

@@ -4,6 +4,8 @@
 
 #include "shell/browser/ui/win/atom_desktop_window_tree_host_win.h"
 
+#include "ui/base/win/hwnd_metrics.h"
+
 namespace electron {
 
 AtomDesktopWindowTreeHostWin::AtomDesktopWindowTreeHostWin(
@@ -29,4 +31,17 @@ bool AtomDesktopWindowTreeHostWin::HasNativeFrame() const {
   return true;
 }
 
+bool AtomDesktopWindowTreeHostWin::GetClientAreaInsets(gfx::Insets* insets,
+                                                       HMONITOR monitor) const {
+  if (IsMaximized() && !native_window_view_->has_frame()) {
+    // Windows automatically adds a standard width border to all sides when a
+    // window is maximized.
+    int frame_thickness = ui::GetFrameThickness(monitor) - 1;
+    *insets = gfx::Insets(frame_thickness, frame_thickness, frame_thickness,
+                          frame_thickness);
+    return true;
+  }
+  return false;
+}
+
 }  // namespace electron

shell/browser/ui/win/atom_desktop_window_tree_host_win.h

@@ -25,6 +25,8 @@ class AtomDesktopWindowTreeHostWin : public views::DesktopWindowTreeHostWin {
                     LPARAM l_param,
                     LRESULT* result) override;
   bool HasNativeFrame() const override;
+  bool GetClientAreaInsets(gfx::Insets* insets,
+                           HMONITOR monitor) const override;
 
  private:
   NativeWindowViews* native_window_view_;  // weak ref