fix: improve the way frameless windows are handled on Windows (#16596)

* fix: improve the way frameless windows are handled on Windows * tidy up code * fix: return nullAcceleratedWidget instead of nullptr * fix: format, use reinterpret cast
electron · Jan 31, 2019 · cbb5164 · cbb5164
1 parent 49ec7e1
commit cbb5164
Show file tree

Hide file tree

Showing 6 changed files with 134 additions and 21 deletions.
diff --git a/atom/browser/native_window.cc b/atom/browser/native_window.cc
@@ -82,8 +82,13 @@ void NativeWindow::InitFromOptions(const mate::Dictionary& options) {
   } else if (options.Get(options::kCenter, &center) && center) {
     Center();
   }
+
+  bool use_content_size = false;
+  options.Get(options::kUseContentSize, &use_content_size);
+
   // On Linux and Window we may already have maximum size defined.
-  extensions::SizeConstraints size_constraints(GetContentSizeConstraints());
+  extensions::SizeConstraints size_constraints(
+      use_content_size ? GetContentSizeConstraints() : GetSizeConstraints());
   int min_height = 0, min_width = 0;
   if (options.Get(options::kMinHeight, &min_height) |
       options.Get(options::kMinWidth, &min_width)) {
@@ -94,8 +99,6 @@ void NativeWindow::InitFromOptions(const mate::Dictionary& options) {
       options.Get(options::kMaxWidth, &max_width)) {
     size_constraints.set_maximum_size(gfx::Size(max_width, max_height));
   }
-  bool use_content_size = false;
-  options.Get(options::kUseContentSize, &use_content_size);
   if (use_content_size) {
     SetContentSizeConstraints(size_constraints);
   } else {

diff --git a/atom/browser/native_window_views.cc b/atom/browser/native_window_views.cc
@@ -32,6 +32,7 @@
 #include "ui/aura/window_tree_host.h"
 #include "ui/base/hit_test.h"
 #include "ui/gfx/image/image.h"
+#include "ui/gfx/native_widget_types.h"
 #include "ui/views/background.h"
 #include "ui/views/controls/webview/unhandled_keyboard_event_handler.h"
 #include "ui/views/controls/webview/webview.h"
@@ -80,6 +81,27 @@ void FlipWindowStyle(HWND handle, bool on, DWORD flag) {
     style &= ~flag;
   ::SetWindowLong(handle, GWL_STYLE, style);
 }
+
+// Similar to the ones in display::win::ScreenWin, but with rounded values
+// These help to avoid problems that arise from unresizable windows where the
+// original ceil()-ed values can cause calculation errors, since converting
+// both ways goes through a ceil() call. Related issue: #15816
+gfx::Rect ScreenToDIPRect(HWND hwnd, const gfx::Rect& pixel_bounds) {
+  float scale_factor = display::win::ScreenWin::GetScaleFactorForHWND(hwnd);
+  gfx::Rect dip_rect = ScaleToRoundedRect(pixel_bounds, 1.0f / scale_factor);
+  dip_rect.set_origin(
+      display::win::ScreenWin::ScreenToDIPRect(hwnd, pixel_bounds).origin());
+  return dip_rect;
+}
+
+gfx::Rect DIPToScreenRect(HWND hwnd, const gfx::Rect& pixel_bounds) {
+  float scale_factor = display::win::ScreenWin::GetScaleFactorForHWND(hwnd);
+  gfx::Rect screen_rect = ScaleToRoundedRect(pixel_bounds, scale_factor);
+  screen_rect.set_origin(
+      display::win::ScreenWin::DIPToScreenRect(hwnd, pixel_bounds).origin());
+  return screen_rect;
+}
+
 #endif
 
 class NativeWindowClientView : public views::ClientView {
@@ -238,7 +260,7 @@ NativeWindowViews::NativeWindowViews(const mate::Dictionary& options,
   if (!has_frame()) {
     // Set Window style so that we get a minimize and maximize animation when
     // frameless.
-    DWORD frame_style = WS_CAPTION;
+    DWORD frame_style = WS_CAPTION | WS_OVERLAPPED;
     if (resizable_)
       frame_style |= WS_THICKFRAME;
     if (minimizable_)
@@ -1076,7 +1098,10 @@ bool NativeWindowViews::IsVisibleOnAllWorkspaces() {
 }
 
 gfx::AcceleratedWidget NativeWindowViews::GetAcceleratedWidget() const {
-  return GetNativeWindow()->GetHost()->GetAcceleratedWidget();
+  if (GetNativeWindow() && GetNativeWindow()->GetHost())
+    return GetNativeWindow()->GetHost()->GetAcceleratedWidget();
+  else
+    return gfx::kNullAcceleratedWidget;
 }
 
 NativeWindowHandle NativeWindowViews::GetNativeWindowHandle() const {
@@ -1091,8 +1116,8 @@ gfx::Rect NativeWindowViews::ContentBoundsToWindowBounds(
   gfx::Rect window_bounds(bounds);
 #if defined(OS_WIN)
   HWND hwnd = GetAcceleratedWidget();
-  gfx::Rect dpi_bounds = display::win::ScreenWin::DIPToScreenRect(hwnd, bounds);
-  window_bounds = display::win::ScreenWin::ScreenToDIPRect(
+  gfx::Rect dpi_bounds = DIPToScreenRect(hwnd, bounds);
+  window_bounds = ScreenToDIPRect(
       hwnd,
       widget()->non_client_view()->GetWindowBoundsForClientBounds(dpi_bounds));
 #endif
@@ -1113,17 +1138,15 @@ gfx::Rect NativeWindowViews::WindowBoundsToContentBounds(
   gfx::Rect content_bounds(bounds);
 #if defined(OS_WIN)
   HWND hwnd = GetAcceleratedWidget();
-  content_bounds.set_size(
-      display::win::ScreenWin::DIPToScreenSize(hwnd, content_bounds.size()));
+  content_bounds.set_size(DIPToScreenRect(hwnd, content_bounds).size());
   RECT rect;
   SetRectEmpty(&rect);
   DWORD style = ::GetWindowLong(hwnd, GWL_STYLE);
   DWORD ex_style = ::GetWindowLong(hwnd, GWL_EXSTYLE);
   AdjustWindowRectEx(&rect, style, FALSE, ex_style);
   content_bounds.set_width(content_bounds.width() - (rect.right - rect.left));
   content_bounds.set_height(content_bounds.height() - (rect.bottom - rect.top));
-  content_bounds.set_size(
-      display::win::ScreenWin::ScreenToDIPSize(hwnd, content_bounds.size()));
+  content_bounds.set_size(ScreenToDIPRect(hwnd, content_bounds).size());
 #endif
 
   if (root_view_->HasMenu() && root_view_->IsMenuBarVisible()) {

diff --git a/atom/browser/native_window_views.h b/atom/browser/native_window_views.h
@@ -242,6 +242,8 @@ class NativeWindowViews : public NativeWindow,
 
   ui::WindowShowState last_window_state_;
 
+  gfx::Rect last_normal_placement_bounds_;
+
   // There's an issue with restore on Windows, that sometimes causes the Window
   // to receive the wrong size (#2498). To circumvent that, we keep tabs on the
   // size of the window while in the normal state (not maximized, minimized or

diff --git a/atom/browser/native_window_views_win.cc b/atom/browser/native_window_views_win.cc
@@ -4,9 +4,12 @@
 
 #include "atom/browser/browser.h"
 #include "atom/browser/native_window_views.h"
+#include "atom/browser/ui/views/root_view.h"
 #include "atom/common/atom_constants.h"
 #include "content/public/browser/browser_accessibility_state.h"
 #include "ui/base/win/accessibility_misc_utils.h"
+#include "ui/display/win/screen_win.h"
+#include "ui/gfx/geometry/insets.h"
 
 // Must be included after other Windows headers.
 #include <UIAutomationCoreApi.h>
@@ -183,6 +186,48 @@ bool NativeWindowViews::PreHandleMSG(UINT message,
 
       return false;
     }
+    case WM_GETMINMAXINFO: {
+      WINDOWPLACEMENT wp;
+      wp.length = sizeof(WINDOWPLACEMENT);
+
+      // We do this to work around a Windows bug, where the minimized Window
+      // would report that the closest display to it is not the one that it was
+      // previously on (but the leftmost one instead). We restore the position
+      // of the window during the restore operation, this way chromium can
+      // use the proper display to calculate the scale factor to use.
+      if (!last_normal_placement_bounds_.IsEmpty() &&
+          GetWindowPlacement(GetAcceleratedWidget(), &wp)) {
+        last_normal_placement_bounds_.set_size(gfx::Size(0, 0));
+        wp.rcNormalPosition = last_normal_placement_bounds_.ToRECT();
+        SetWindowPlacement(GetAcceleratedWidget(), &wp);
+
+        last_normal_placement_bounds_ = gfx::Rect();
+      }
+
+      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);
+
+        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)
@@ -258,15 +303,40 @@ void NativeWindowViews::HandleSizeEvent(WPARAM w_param, LPARAM l_param) {
   // Here we handle the WM_SIZE event in order to figure out what is the current
   // window state and notify the user accordingly.
   switch (w_param) {
-    case SIZE_MAXIMIZED:
+    case SIZE_MAXIMIZED: {
+      // Frameless maximized windows are size compensated by Windows for a
+      // border that's not actually there, so we must conter-compensate.
+      // https://blogs.msdn.microsoft.com/wpfsdk/2008/09/08/custom-window-chrome-in-wpf/
+      if (!has_frame()) {
+        float scale_factor = display::win::ScreenWin::GetScaleFactorForHWND(
+            GetAcceleratedWidget());
+
+        int border =
+            GetSystemMetrics(SM_CXFRAME) + GetSystemMetrics(SM_CXPADDEDBORDER);
+        if (!thick_frame_) {
+          border -= GetSystemMetrics(SM_CXBORDER);
+        }
+        root_view_->SetInsets(gfx::Insets(border).Scale(1.0f / scale_factor));
+      }
+
       last_window_state_ = ui::SHOW_STATE_MAXIMIZED;
       if (consecutive_moves_) {
         last_normal_bounds_ = last_normal_bounds_before_move_;
       }
+
       NotifyWindowMaximize();
       break;
+    }
     case SIZE_MINIMIZED:
       last_window_state_ = ui::SHOW_STATE_MINIMIZED;
+
+      WINDOWPLACEMENT wp;
+      wp.length = sizeof(WINDOWPLACEMENT);
+
+      if (GetWindowPlacement(GetAcceleratedWidget(), &wp)) {
+        last_normal_placement_bounds_ = gfx::Rect(wp.rcNormalPosition);
+      }
+
       NotifyWindowMinimize();
       break;
     case SIZE_RESTORED:
@@ -278,10 +348,7 @@ void NativeWindowViews::HandleSizeEvent(WPARAM w_param, LPARAM l_param) {
         switch (last_window_state_) {
           case ui::SHOW_STATE_MAXIMIZED:
             last_window_state_ = ui::SHOW_STATE_NORMAL;
-
-            // Don't force out last known bounds onto the window as Windows
-            // actually gets these correct
-
+            root_view_->SetInsets(gfx::Insets(0));
             NotifyWindowUnmaximize();
             break;
           case ui::SHOW_STATE_MINIMIZED:
@@ -293,7 +360,9 @@ void NativeWindowViews::HandleSizeEvent(WPARAM w_param, LPARAM l_param) {
 
               // When the window is restored we resize it to the previous known
               // normal size.
-              SetBounds(last_normal_bounds_, false);
+              if (has_frame()) {
+                SetBounds(last_normal_bounds_, false);
+              }
 
               NotifyWindowRestore();
             }

diff --git a/atom/browser/ui/views/root_view.cc b/atom/browser/ui/views/root_view.cc
@@ -173,14 +173,18 @@ void RootView::Layout() {
     return;
 
   const auto menu_bar_bounds =
-      menu_bar_visible_ ? gfx::Rect(0, 0, size().width(), kMenuBarHeight)
-                        : gfx::Rect();
+      menu_bar_visible_
+          ? gfx::Rect(insets_.left(), insets_.top(),
+                      size().width() - insets_.width(), kMenuBarHeight)
+          : gfx::Rect();
   if (menu_bar_)
     menu_bar_->SetBoundsRect(menu_bar_bounds);
 
   window_->content_view()->SetBoundsRect(
-      gfx::Rect(0, menu_bar_bounds.height(), size().width(),
-                size().height() - menu_bar_bounds.height()));
+      gfx::Rect(insets_.left(),
+                menu_bar_visible_ ? menu_bar_bounds.bottom() : insets_.top(),
+                size().width() - insets_.width(),
+                size().height() - menu_bar_bounds.height() - insets_.height()));
 }
 
 gfx::Size RootView::GetMinimumSize() const {
@@ -217,4 +221,11 @@ void RootView::UnregisterAcceleratorsWithFocusManager() {
   focus_manager->UnregisterAccelerators(this);
 }
 
+void RootView::SetInsets(const gfx::Insets& insets) {
+  if (insets != insets_) {
+    insets_ = insets;
+    Layout();
+  }
+}
+
 }  // namespace atom
diff --git a/atom/browser/ui/views/root_view.h b/atom/browser/ui/views/root_view.h
@@ -8,6 +8,7 @@
 #include <memory>
 
 #include "atom/browser/ui/accelerator_util.h"
+#include "ui/gfx/geometry/insets.h"
 #include "ui/views/view.h"
 #include "ui/views/view_tracker.h"
 
@@ -39,6 +40,8 @@ class RootView : public views::View {
   // Register/Unregister accelerators supported by the menu model.
   void RegisterAcceleratorsWithFocusManager(AtomMenuModel* menu_model);
   void UnregisterAcceleratorsWithFocusManager();
+  void SetInsets(const gfx::Insets& insets);
+  gfx::Insets insets() const { return insets_; }
 
   // views::View:
   void Layout() override;
@@ -56,6 +59,8 @@ class RootView : public views::View {
   bool menu_bar_visible_ = false;
   bool menu_bar_alt_pressed_ = false;
 
+  gfx::Insets insets_;
+
   // Map from accelerator to menu item's command id.
   accelerator_util::AcceleratorTable accelerator_table_;