Update ScreenWin Cached State When An Inconsistency Is Detected |
||||||||||||||||||
Issue description
OS Version: Windows 10 and possibly Windows 8.
On Windows 10 it is possible to change display DPI on the fly without login/logout procedure.
HWNDMessageHandler class handles that event in OnDpiChanged() handler and then notifies the renderer process and Blink via WebContentsImpl::UpdateDeviceScaleFactor() function.
But properties of the Display class will not change at that moment.
And the Display object will keep an old DSF value until WM_DISPLAYCHANGE is handled.
Unfortunately, WindowTreeHost will query and use outdated DSF value before WM_DISPLAYCHANGE is handled. That leads to sending wrong value of the display DSF to the renderer.
The sequence is something like that:
1. Change display scale to 150% in Windows settings.
2. HWNDMessageHandler::OnDpiChanged()
|- calls WebContentsImpl::UpdateDeviceScaleFactor(1.5f)
|- sends PageMsg_SetDeviceScaleFactor(1.5) to rendeder.
3. Resize occured, calls to WindowTreeHost::OnHostResized()
|- query GetDeviceScaleFactorFromDisplay() and return old value of 1.0
|- calls Compositor::SetScaleAndSize(1.0)
|- calls WebContentsImpl::UpdateDeviceScaleFactor(1.0)
|- sends PageMsg_SetDeviceScaleFactor(1.0) to rendeder.
4. Windows emits WM_DISPLAYCHANGE message and ScreenWin class will update all Displays and its values.
At that moment renderer will have wrong DSF value and will get correct value only when the content will be resized.
It seems wrong to me, but I'm not sure how to fix it correctly.
I will send a CL with fix to discuss.
,
Nov 16 2016
When I change the DPI on Windows, I see this sequence in the following order: 1) Request to change the DPI from the SystemSettings.exe. 2) WM_DISPLAYCHANGED (handled by ScreenWin and the displays DPIs are updated). 3) WM_DPICHANGED (handled by HwndMessageHandler). What's the repro that manages to reorder (2) and (3). Windows fires WM_DISPLAYCHANGED first because it needs to let apps that are tracking displays first know that the display configuration is changing. WM_DPICHANGED is fired after WM_DISPLAYCHANGED so that clients can use the data from WM_DISPLAYCHANGED. Per-Monitor DPI is only enabled on Windows 10 and WM_DPICHANGED should not be fired on Windows 8.1. Windows 8 predates WM_DPICHANGED.
,
Nov 16 2016
,
Nov 17 2016
Hi! I have Windows 10 in VirtualBox, configured for two displays. I just move Chromium window to the second display and then change display scale to 150%. After that configuration WM_DPICHANGED will be handled first every time I tried. I see that in the log or when I connect my debugger to the remote running instance of Chromium in VirtualBox. On virtual machine I use release build so there are no DCHECKs. Also, I've tested Debug build of Chromium 56.0.2906.0 on real PC with Windows 10 and two physical monitors. Here is screen-cast of this run in attach. Chromium was launched on the main monitor and we see log output in the background. Then I changed DSF to 150% in settings and got a DCHECK. As we can see, WM_DPICHANGED message handled first, then we hit a DCHECK at https://cs.chromium.org/chromium/src/content/browser/renderer_host/render_widget_host_view_aura.cc?q=render_widget_host_view_aura.c&sq=package:chromium&l=1534 And then WM_DISPLAYCHANGED message arrived (it can be seen in console output). AFAIK, it means that cached values of DSF are outdated and WM_DISPLAYCHANGED is not handled at that moment. Looks like in my setup these two Windows messages always handled in the following order: WM_DPICHANGED, WM_DISPLAYCHANGED. Here is backtrace in plain text: http://pastebin.com/iia4ZqXX Can the order of these messages be guaranteed to us anyhow? Is there any document from MS or something?
,
Nov 23 2016
,
Nov 28 2016
,
Nov 28 2016
I'll handle confirmation and triage here.
,
Nov 30 2016
tl;dr. It appears the ordering of the messages changes depending on your app!
In Chrome with two displays changing the DPI of one display with Chrome on that display results in the following breakpoints hit in the order below (WM_DISPLAYCHANGE then WM_DPICHANGE)
0:000> kn20
# ChildEBP RetAddr
00 0019ed70 0f0e90ee display!display::win::ScreenWin::OnWndProc+0x2d
01 0019ed8c 0f0e92c8 display!base::internal::FunctorTraits<void (__thiscall display::win::ScreenWin::*)(HWND__ *,unsigned int,unsigned int,long),void>::Invoke<display::win::ScreenWin *,HWND__ *,unsigned int,unsigned int,long>+0x4e
02 0019edac 0f0e935f display!base::internal::InvokeHelper<0,void>::MakeItSo<void (__thiscall display::win::ScreenWin::*const &)(HWND__ *,unsigned int,unsigned int,long),display::win::ScreenWin *,HWND__ *,unsigned int,unsigned int,long>+0x58
03 0019edd0 0f0ef338 display!base::internal::Invoker<base::internal::BindState<void (__thiscall display::win::ScreenWin::*)(HWND__ *,unsigned int,unsigned int,long),base::internal::UnretainedWrapper<display::win::ScreenWin> >,void __cdecl(HWND__ *,unsigned int,unsigned int,long)>::RunImpl<void (__thiscall display::win::ScreenWin::*const &)(HWND__ *,unsigned int,unsigned int,long),std::tuple<base::internal::UnretainedWrapper<display::win::ScreenWin> > const &,0>+0x6f
04 0019edfc 01727153 display!base::internal::Invoker<base::internal::BindState<void (__thiscall display::win::ScreenWin::*)(HWND__ *,unsigned int,unsigned int,long),base::internal::UnretainedWrapper<display::win::ScreenWin> >,void __cdecl(HWND__ *,unsigned int,unsigned int,long)>::Run+0x58
05 0019ee28 017270c2 gfx!base::internal::RunMixin<base::Callback<void __cdecl(HWND__ *,unsigned int,unsigned int,long),1,1> >::Run+0x73
06 0019ee44 0172566a gfx!gfx::SingletonHwndObserver::OnWndProc+0x22
07 0019ee88 017297fe gfx!gfx::SingletonHwnd::ProcessWindowMessage+0x6a
08 0019eeb4 01729f21 gfx!gfx::WindowImpl::OnWndProc+0x4e
09 0019ef94 01727e36 gfx!gfx::WindowImpl::WndProc+0x121
0a 0019efd8 74c5d2b3 gfx!base::win::WrappedWindowProc<&gfx::WindowImpl::WndProc>+0x56
0b 0019f004 74c3e88a USER32!_InternalCallWinProc+0x2b
0c 0019f0ec 74c3e4c0 USER32!UserCallWinProcCheckWow+0x30a
0d 0019f14c 74c4ad70 USER32!DispatchClientMessage+0xf0
0e 0019f190 77ee0006 USER32!__fnEMPTY+0x40
0f 0019f1c8 7545249c ntdll!KiUserCallbackDispatcher+0x36
10 0019f1cc 74c46ad7 win32u!NtUserPeekMessage+0xc
11 0019f1fc 74c46a6d USER32!_PeekMessage+0x27
12 0019f234 10110081 USER32!PeekMessageW+0x14d
13 0019f278 1010f512 base!base::MessagePumpForUI::ProcessNextWindowsMessage+0x41
14 0019f28c 101107db base!base::MessagePumpForUI::DoRunLoop+0x12
15 0019f2c0 10106bdc base!base::MessagePumpWin::Run+0x7b
16 0019f508 101b47a4 base!base::MessageLoop::RunHandler+0x1bc
17 0019f530 04b86ae5 base!base::RunLoop::Run+0x34
18 0019f618 1172d3a1 chrome_30d0000!ChromeBrowserMainParts::MainMessageLoopRun+0x125
19 0019f680 11732f1f content!content::BrowserMainLoop::RunMainMessageLoopParts+0xe1
1a 0019f804 1171f3cb content!content::BrowserMainRunnerImpl::Run+0x14f
1b 0019f82c 13039537 content!content::BrowserMain+0x9b
1c 0019f8f8 130393f8 content!content::RunNamedProcessTypeMain+0x87
1d 0019fac8 13037314 content!content::ContentMainRunnerImpl::Run+0x1e8
1e 0019faec 036fb1a8 content!content::ContentMain+0x64
1f 0019fbf0 0042e136 chrome_30d0000!ChromeMain+0x108
0:000> dv
this = 0x44fd5de8
hwnd = 0x000c07a0
message = 0x7e (WM_DISPLAYCHANGE)
wparam = 0x20
lparam = 0n131075000
old_displays = { size=2 }
Then
0:000> kn20
# ChildEBP RetAddr
00 0019ea64 1c9d27f7 views!views::HWNDMessageHandler::OnDpiChanged
01 0019ec6c 1c9e4503 views!views::HWNDMessageHandler::_ProcessWindowMessage+0x237
02 0019ecb8 01729f21 views!views::HWNDMessageHandler::OnWndProc+0x93
03 0019ed98 01727e36 gfx!gfx::WindowImpl::WndProc+0x121
04 0019eddc 74c5d2b3 gfx!base::win::WrappedWindowProc<&gfx::WindowImpl::WndProc>+0x56
05 0019ee08 74c3e88a USER32!_InternalCallWinProc+0x2b
06 0019eef0 74c3e4c0 USER32!UserCallWinProcCheckWow+0x30a
07 0019ef50 74c4aee3 USER32!DispatchClientMessage+0xf0
08 0019ef90 77ee0006 USER32!__fnINOUTLPRECT+0x43
09 0019efd0 7545249c ntdll!KiUserCallbackDispatcher+0x36
0a 0019efd4 74c46ad7 win32u!NtUserPeekMessage+0xc
0b 0019f004 74c46a6d USER32!_PeekMessage+0x27
0c 0019f03c 101100d5 USER32!PeekMessageW+0x14d
0d 0019f200 1010fe90 base!base::MessagePumpForUI::ProcessPumpReplacementMessage+0x25
0e 0019f244 10110091 base!base::MessagePumpForUI::ProcessMessageHelper+0x180
0f 0019f278 1010f512 base!base::MessagePumpForUI::ProcessNextWindowsMessage+0x51
10 0019f28c 101107db base!base::MessagePumpForUI::DoRunLoop+0x12
11 0019f2c0 10106bdc base!base::MessagePumpWin::Run+0x7b
12 0019f508 101b47a4 base!base::MessageLoop::RunHandler+0x1bc
13 0019f530 04b86ae5 base!base::RunLoop::Run+0x34
14 0019f618 1172d3a1 chrome_30d0000!ChromeBrowserMainParts::MainMessageLoopRun+0x125
15 0019f680 11732f1f content!content::BrowserMainLoop::RunMainMessageLoopParts+0xe1
16 0019f804 1171f3cb content!content::BrowserMainRunnerImpl::Run+0x14f
17 0019f82c 13039537 content!content::BrowserMain+0x9b
18 0019f8f8 130393f8 content!content::RunNamedProcessTypeMain+0x87
19 0019fac8 13037314 content!content::ContentMainRunnerImpl::Run+0x1e8
1a 0019faec 036fb1a8 content!content::ContentMain+0x64
1b 0019fbf0 0042e136 chrome_30d0000!ChromeMain+0x108
1c 0019fd04 00428f3e chrome!MainDllLoader::Launch+0x396
1d 0019ff00 004c80ae chrome!wWinMain+0x22e
1e 0019ff18 004c7f10 chrome!invoke_main+0x1e
1f 0019ff70 004c7dad chrome!__scrt_common_main_seh+0x150
I also wrote up a toy app just to make sure and the ordering is the opposite!
WM_DPICHANGED
WM_DISPLAYCHANGE
Something interesting is certainly going on here. This will require a bit more digging.
,
Dec 1 2016
Thanks for the digging into that! If you have any idea how I can help with that, please share it. I have a quite stable WTR for this situation with Chromium app.
,
Dec 3 2016
I'll go ahead and pick this up. I'm engaging with the Windows team to determine the right steps to take here.
,
Dec 3 2016
,
Dec 16 2016
The short of it here is that WM_DPICHANGED and WM_DISPLAYCHANGE will not have a guaranteed ordering. We'll have to think through the right thing to do here.
,
Apr 4 2017
I think the real solution here is probably to detect changes from Windows and update when we detect an inconsistency in addition to WM_DPICHANGED and WM_DISPLAYCHANGED
,
Apr 12 2017
Because we can't rely on message ordering from Windows to detect when there are changes in the screen configuration, instead of DCHECKing the inconsistency, we need to recompute on the spot.
,
Apr 12 2017
,
Sep 19
tbergquist: Does this sound like it would be relevant to your HighDPI changes.
,
Sep 20
No, I don't think I was ever quite in that area.
,
Oct 24
,
Nov 2
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/88e77ae7b6216ca24d11c6438f6d92a988bff2aa commit 88e77ae7b6216ca24d11c6438f6d92a988bff2aa Author: Scott Violet <sky@chromium.org> Date: Fri Nov 02 22:50:55 2018 aura: updates common on device scale factor caching. BUG=665965 TEST=none Change-Id: Icf59d9537180eeb11f572d66414e04808be23603 Reviewed-on: https://chromium-review.googlesource.com/c/1316105 Reviewed-by: Michael Wasserman <msw@chromium.org> Commit-Queue: Michael Wasserman <msw@chromium.org> Cr-Commit-Position: refs/heads/master@{#605079} [modify] https://crrev.com/88e77ae7b6216ca24d11c6438f6d92a988bff2aa/ui/aura/window_tree_host.h
,
Nov 6
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/e0e48885d523be60a79913ae9e0e122217188deb commit e0e48885d523be60a79913ae9e0e122217188deb Author: Mike Wasserman <msw@chromium.org> Date: Tue Nov 06 00:40:52 2018 ws: Fix scaling in DesktopWindowTreeHostMus during Init Initialize the cached display device scale factor before usage. Otherwise, the scale factor is 1 and the window is misplaced/sized. I'll land a unit test separately, to make merging easier. Bug: 899084 , 665965 Test: KSV (Ctrl-Alt-/) search context menu supports display & UI scales. Change-Id: Ie74cb70af33110aa09022d4dbb5c6d0069b6cc7c Reviewed-on: https://chromium-review.googlesource.com/c/1311080 Commit-Queue: Michael Wasserman <msw@chromium.org> Reviewed-by: Scott Violet <sky@chromium.org> Cr-Commit-Position: refs/heads/master@{#605543} [modify] https://crrev.com/e0e48885d523be60a79913ae9e0e122217188deb/ui/aura/window_tree_host.cc [modify] https://crrev.com/e0e48885d523be60a79913ae9e0e122217188deb/ui/aura/window_tree_host.h [modify] https://crrev.com/e0e48885d523be60a79913ae9e0e122217188deb/ui/views/mus/desktop_window_tree_host_mus.cc
,
Nov 7
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/25883ff3a49a2777e53788bde64f8a32b57ecee4 commit 25883ff3a49a2777e53788bde64f8a32b57ecee4 Author: Mike Wasserman <msw@chromium.org> Date: Wed Nov 07 01:50:45 2018 ws: Fix scaling in DesktopWindowTreeHostMus during Init Initialize the cached display device scale factor before usage. Otherwise, the scale factor is 1 and the window is misplaced/sized. I'll land a unit test separately, to make merging easier. Bug: 899084 , 665965 Test: KSV (Ctrl-Alt-/) search context menu supports display & UI scales. Change-Id: Ie74cb70af33110aa09022d4dbb5c6d0069b6cc7c Reviewed-on: https://chromium-review.googlesource.com/c/1311080 Commit-Queue: Michael Wasserman <msw@chromium.org> Reviewed-by: Scott Violet <sky@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#605543}(cherry picked from commit e0e48885d523be60a79913ae9e0e122217188deb) Reviewed-on: https://chromium-review.googlesource.com/c/1321867 Reviewed-by: Michael Wasserman <msw@chromium.org> Cr-Commit-Position: refs/branch-heads/3578@{#556} Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034} [modify] https://crrev.com/25883ff3a49a2777e53788bde64f8a32b57ecee4/ui/aura/window_tree_host.cc [modify] https://crrev.com/25883ff3a49a2777e53788bde64f8a32b57ecee4/ui/aura/window_tree_host.h [modify] https://crrev.com/25883ff3a49a2777e53788bde64f8a32b57ecee4/ui/views/mus/desktop_window_tree_host_mus.cc
,
Nov 7
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/25883ff3a49a2777e53788bde64f8a32b57ecee4 Commit: 25883ff3a49a2777e53788bde64f8a32b57ecee4 Author: msw@chromium.org Commiter: msw@chromium.org Date: 2018-11-07 01:50:45 +0000 UTC ws: Fix scaling in DesktopWindowTreeHostMus during Init Initialize the cached display device scale factor before usage. Otherwise, the scale factor is 1 and the window is misplaced/sized. I'll land a unit test separately, to make merging easier. Bug: 899084 , 665965 Test: KSV (Ctrl-Alt-/) search context menu supports display & UI scales. Change-Id: Ie74cb70af33110aa09022d4dbb5c6d0069b6cc7c Reviewed-on: https://chromium-review.googlesource.com/c/1311080 Commit-Queue: Michael Wasserman <msw@chromium.org> Reviewed-by: Scott Violet <sky@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#605543}(cherry picked from commit e0e48885d523be60a79913ae9e0e122217188deb) Reviewed-on: https://chromium-review.googlesource.com/c/1321867 Reviewed-by: Michael Wasserman <msw@chromium.org> Cr-Commit-Position: refs/branch-heads/3578@{#556} Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}
,
Nov 21
*** UI Mass Triage*** Seems like WIP and bug is valid, hence tagging with appropriate label. |
||||||||||||||||||
►
Sign in to add a comment |
||||||||||||||||||
Comment 1 by ice...@yandex-team.ru
, Nov 16 2016