Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(715)

Issue 1976923004: Reduce the size of the fullscreen window on activation loss only if the window being activated is … (Closed)

Created:
4 years, 7 months ago by ananta
Modified:
4 years, 7 months ago
Reviewers:
sky
CC:
chromium-reviews, tfarina
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Reduce the size of the fullscreen window on activation loss only if the window being activated is on the same monitor Currently we reduce the size of the fullscreen window on activation loss by 1 px to ensure that other maximized windows on the same thread don't get treated as fullscreen windows, i.e. they draw over the taskbar, autohide taskbar does not work etc. Based on some testing it looks like this problem only happens if the fullscreen window and the window being activated are on the same monitor. The fix is to reduce the fullscreen window bounds only if it is on the same monitor as the window being activated. For this we have a map in HWNDMessageHandler which is keyed on HMONITOR. We use this to determine the same. This map is populated when a window becomes fullscreen. BUG=604359 TEST=The fullscreen window size reducing is already covered by the interactive_ui_test WidgetTestInteractive. FullscreenBoundsReducedOnActivationLoss. Don't have a good way to test the multiple monitor case. Committed: https://crrev.com/b14dd7c0ea2957b091eb98c7b5f461bfc8129fe0 Cr-Commit-Position: refs/heads/master@{#394000}

Patch Set 1 #

Patch Set 2 : Remove unused include and fix indent #

Patch Set 3 : Fix include order #

Total comments: 5

Patch Set 4 : Address review comments #

Patch Set 5 : Save the HWNDMessageHandler instance in the fullscreen map #

Patch Set 6 : Restore newline #

Total comments: 11

Patch Set 7 : git cl format #

Patch Set 8 : Fix loop #

Unified diffs Side-by-side diffs Delta from patch set Stats (+88 lines, -10 lines) Patch
M ui/views/win/hwnd_message_handler.h View 1 2 3 4 5 6 3 chunks +18 lines, -0 lines 0 comments Download
M ui/views/win/hwnd_message_handler.cc View 1 2 3 4 5 6 7 7 chunks +70 lines, -10 lines 0 comments Download

Depends on Patchset:

Messages

Total messages: 14 (4 generated)
ananta
4 years, 7 months ago (2016-05-13 22:05:57 UTC) #2
sky
https://codereview.chromium.org/1976923004/diff/40001/ui/views/win/hwnd_message_handler.cc File ui/views/win/hwnd_message_handler.cc (right): https://codereview.chromium.org/1976923004/diff/40001/ui/views/win/hwnd_message_handler.cc#newcode305 ui/views/win/hwnd_message_handler.cc:305: HWNDMessageHandler::FullscreenWindowMonitorMap* Use LazyInstance, it's less error prone than what ...
4 years, 7 months ago (2016-05-13 23:32:36 UTC) #3
ananta
https://codereview.chromium.org/1976923004/diff/40001/ui/views/win/hwnd_message_handler.cc File ui/views/win/hwnd_message_handler.cc (right): https://codereview.chromium.org/1976923004/diff/40001/ui/views/win/hwnd_message_handler.cc#newcode305 ui/views/win/hwnd_message_handler.cc:305: HWNDMessageHandler::FullscreenWindowMonitorMap* On 2016/05/13 23:32:36, sky wrote: > Use LazyInstance, ...
4 years, 7 months ago (2016-05-16 19:56:43 UTC) #4
sky
On Mon, May 16, 2016 at 12:56 PM, <ananta@chromium.org> wrote: > > https://codereview.chromium.org/1976923004/diff/40001/ui/views/win/hwnd_message_handler.cc > File ...
4 years, 7 months ago (2016-05-16 21:14:28 UTC) #5
ananta
https://codereview.chromium.org/1976923004/diff/40001/ui/views/win/hwnd_message_handler.cc File ui/views/win/hwnd_message_handler.cc (right): https://codereview.chromium.org/1976923004/diff/40001/ui/views/win/hwnd_message_handler.cc#newcode2755 ui/views/win/hwnd_message_handler.cc:2755: PostMessage(index->second, On 2016/05/16 19:56:43, ananta wrote: > On 2016/05/13 ...
4 years, 7 months ago (2016-05-16 21:35:42 UTC) #6
sky
LGTM https://codereview.chromium.org/1976923004/diff/100001/ui/views/win/hwnd_message_handler.cc File ui/views/win/hwnd_message_handler.cc (right): https://codereview.chromium.org/1976923004/diff/100001/ui/views/win/hwnd_message_handler.cc#newcode828 ui/views/win/hwnd_message_handler.cc:828: FullscreenWindowMonitorMap::iterator index = index->iter https://codereview.chromium.org/1976923004/diff/100001/ui/views/win/hwnd_message_handler.cc#newcode1366 ui/views/win/hwnd_message_handler.cc:1366: FullscreenWindowMonitorMap::iterator index; ...
4 years, 7 months ago (2016-05-16 23:05:10 UTC) #7
ananta
https://codereview.chromium.org/1976923004/diff/100001/ui/views/win/hwnd_message_handler.cc File ui/views/win/hwnd_message_handler.cc (right): https://codereview.chromium.org/1976923004/diff/100001/ui/views/win/hwnd_message_handler.cc#newcode1366 ui/views/win/hwnd_message_handler.cc:1366: FullscreenWindowMonitorMap::iterator index; On 2016/05/16 23:05:10, sky wrote: > index->iter ...
4 years, 7 months ago (2016-05-16 23:31:03 UTC) #8
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1976923004/140001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1976923004/140001
4 years, 7 months ago (2016-05-16 23:57:03 UTC) #11
commit-bot: I haz the power
Committed patchset #8 (id:140001)
4 years, 7 months ago (2016-05-17 01:05:20 UTC) #12
commit-bot: I haz the power
4 years, 7 months ago (2016-05-17 01:07:53 UTC) #14
Message was sent while issue was closed.
Patchset 8 (id:??) landed as
https://crrev.com/b14dd7c0ea2957b091eb98c7b5f461bfc8129fe0
Cr-Commit-Position: refs/heads/master@{#394000}

Powered by Google App Engine
This is Rietveld 408576698