Fix for issues seen on frameless window (Windows) when at 200% resolution

[bsclifton]

First round of DPI issues we saw on Windows with BrowserWindow => frame:false were reported with #7347 and fixed with #7362

While creating a new release and testing our project with that fix, we ended up seeing more weird behavior on high DPI screens. After more investigation, we realized that the NativeWindowViews class was not calling DIPToScreenRect/ScreenToDIPSize for Frameless windows, which caused several issues (for screenshots / animated gifs, check out brave/browser-laptop#4365)

The symptoms for the original issue were ONLY noticeable / easily reproducible (to my knowledge) by using Windows 10 with the anniversary update (I'm not sure if arch or home vs pro matters- but the issue does not happen unless you have the anniversary update).

Special thanks to @bridiver for finding the issue and @bbondy / @jonathansampson for testing the fix

[zcbenz]

👍

[zcbenz]

When testing this change, I realized this would cause other side effects and I'm now reverting it.

The whole purpose of the DIPToScreenRect/ScreenToDIPSize calls is to calculate the window frame: win32 APIs like AdjustWindowRectEx take pixel size instead of DIP size, so we have to convert the DIP size to pixel size before calling it, and then convert the size back from pixel to DIP after the call. That's why ContentBoundsToWindowBounds and WindowBoundsToContentBounds do nothing for frameless window, because there is no window frame.

The problem with this change is, it calls AdjustWindowRectEx for frameless window, which adds the size of window frame for frameless window, and results in failing tests of content size. I tried to avoid calling AdjustWindowRectEx for frameless window, but it then results in code like this:

gfx::Rect NativeWindowViews::WindowBoundsToContentBounds(const gfx::Rect& bounds) {
  gfx::Rect content_bounds(bounds);
  content_bounds.set_size(
      display::win::ScreenWin::DIPToScreenSize(hwnd, content_bounds.size()));
  content_bounds.set_size(
      display::win::ScreenWin::ScreenToDIPSize(hwnd, content_bounds.size()));
  return content_bounds;
}

which is basically the same with:

gfx::Rect NativeWindowViews::WindowBoundsToContentBounds(const gfx::Rect& bounds) {
  return bounds
}

I don't know the reason of your problem on Windows 10 AU, but this change is fixing it by passing wrong sizes.

[bsclifton]

@zcbenz thanks for the info 😄 I'll have to look into this some more. Can you point me at the failing test? I'd like to dig into this some more

cc: @bridiver

[zcbenz]

@bsclifton You run ./script/test.py to see the failing tests:

not ok 58 browser-window module BrowserWindow.setContentSize(width, height) works for a frameless window
  AssertionError: 416 == 400
      at Context.<anonymous> (C:\projects\electron\spec\api-browser-window-spec.js:345:14)
      at callFn (C:\projects\electron\spec\node_modules\mocha\lib\runnable.js:251:21)
      at Test.Runnable.run (C:\projects\electron\spec\node_modules\mocha\lib\runnable.js:244:7)
      at Runner.runTest (C:\projects\electron\spec\node_modules\mocha\lib\runner.js:374:10)
      at C:\projects\electron\spec\node_modules\mocha\lib\runner.js:452:12
      at next (C:\projects\electron\spec\node_modules\mocha\lib\runner.js:299:14)
      at C:\projects\electron\spec\node_modules\mocha\lib\runner.js:309:7
      at next (C:\projects\electron\spec\node_modules\mocha\lib\runner.js:248:23)
      at Immediate.<anonymous> (C:\projects\electron\spec\node_modules\mocha\lib\runner.js:276:5)
      at runCallback (timers.js:574:20)
not ok 60 browser-window module BrowserWindow.setContentBounds(bounds) works for a frameless window
  AssertionError: { x: 2, y: -21, width: 266, height: 289 } deepEqual { x: 10, y: 10, width: 250, height: 250 }
      at C:\projects\electron\spec\api-browser-window-spec.js:370:16
      at CallbacksRegistry.apply (C:\projects\electron\out\D\resources\electron.asar\common\api\callbacks-registry.js:54:39)
      at EventEmitter.<anonymous> (C:\projects\electron\out\D\resources\electron.asar\renderer\api\remote.js:282:21)
      at emitThree (events.js:116:13)
      at EventEmitter.emit (events.js:194:7)

[bridiver]

I believe this was also a problem on surface devices
cc @jonathansampson

[jonathansampson]

@bridiver It was impacting Surface Books (coincidentally those with the Anniversary Update of Windows?) as they have high-DPI screens, and therefore scale the display up (200% in my case, though 150% and 175% for others) to makeup for the increase in on-screen pixels.

[bsclifton]

@zcbenz we did find the root cause for this (and other) issues, which @bridiver addressed here brave/browser-laptop@e8bf42e

More details about the issue can be found here:
https://bugs.chromium.org/p/chromium/issues/detail?id=613414

Besides breaking draggability and hit testing, it caused issues for us with regards to scrolling (inside of a guest view though, so regular electron use-cases may not be affected)