/
fix_hi-dpi_transitions_on_catalina.patch
250 lines (219 loc) · 10.8 KB
/
fix_hi-dpi_transitions_on_catalina.patch
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
216
217
218
219
220
221
222
223
224
225
226
227
228
229
230
231
232
233
234
235
236
237
238
239
240
241
242
243
244
245
246
247
248
249
250
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Christopher Cameron <ccameron@chromium.org>
Date: Thu, 7 Nov 2019 18:28:51 +0000
Subject: Fix hi-dpi transitions on Catalina
A few issues here:
First, actually wire up NativeWidgetNSWindowBridge as a DisplayObserver.
In the past, it didn't matter that this code was missing, because we
were getting notified via windowDidChangeBackingProperties. In Catalina,
that call is happening at a different time, resulting us using an
invalid cached version (which is the second issue).
Second, change GetDisplayNearestWindow to call UpdateDisplaysIfNeeded.
There was a bug here wherein we would return displays_[0], even if we
knew (because of displays_require_update_) that that value was out of
date.
Thid, make GetCachedDisplayForScreen be robust to getting a surprise
NSScreen* that we didn't know about. On Catalina, it happens that we
can read -[NSScreen screens] and see that it has changed, before having
received any notifications that such a change would happen (!).
Fourth, listen for NSApplicationDidChangeScreenParametersNotification
notifications. Just because it sounds like a healthy thing to be doing.
Bug: 1021340
Change-Id: Ibe5a6469d9e2c39cd81d0fb19ee2cfe3aedb1488
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1902508
Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org>
Commit-Queue: ccameron <ccameron@chromium.org>
Cr-Commit-Position: refs/heads/master@{#713490}
diff --git a/components/remote_cocoa/app_shim/native_widget_ns_window_bridge.h b/components/remote_cocoa/app_shim/native_widget_ns_window_bridge.h
index 9aa274565161d8ddc7257011ad5a3e345549631e..e91ba1d54b1466fc4437692ee6f084f4f5d146c6 100644
--- a/components/remote_cocoa/app_shim/native_widget_ns_window_bridge.h
+++ b/components/remote_cocoa/app_shim/native_widget_ns_window_bridge.h
@@ -187,6 +187,8 @@ class REMOTE_COCOA_APP_SHIM_EXPORT NativeWidgetNSWindowBridge
bool RedispatchKeyEvent(NSEvent* event);
// display::DisplayObserver:
+ void OnDisplayAdded(const display::Display& new_display) override;
+ void OnDisplayRemoved(const display::Display& old_display) override;
void OnDisplayMetricsChanged(const display::Display& display,
uint32_t metrics) override;
diff --git a/components/remote_cocoa/app_shim/native_widget_ns_window_bridge.mm b/components/remote_cocoa/app_shim/native_widget_ns_window_bridge.mm
index 3db03e957c0325417747a952f2dffb45b3c81916..ab9caba9ba54a6fbe85b3ca2ffac470d9498900d 100644
--- a/components/remote_cocoa/app_shim/native_widget_ns_window_bridge.mm
+++ b/components/remote_cocoa/app_shim/native_widget_ns_window_bridge.mm
@@ -313,9 +313,11 @@ NativeWidgetNSWindowBridge::NativeWidgetNSWindowBridge(
bridge_mojo_binding_(this) {
DCHECK(GetIdToWidgetImplMap().find(id_) == GetIdToWidgetImplMap().end());
GetIdToWidgetImplMap().insert(std::make_pair(id_, this));
+ display::Screen::GetScreen()->AddObserver(this);
}
NativeWidgetNSWindowBridge::~NativeWidgetNSWindowBridge() {
+ display::Screen::GetScreen()->RemoveObserver(this);
// The delegate should be cleared already. Note this enforces the precondition
// that -[NSWindow close] is invoked on the hosted window before the
// destructor is called.
@@ -1098,7 +1100,17 @@ DragDropClient* NativeWidgetNSWindowBridge::drag_drop_client() {
}
////////////////////////////////////////////////////////////////////////////////
-// NativeWidgetNSWindowBridge, ui::CATransactionObserver
+// NativeWidgetNSWindowBridge, display::DisplayObserver:
+
+void NativeWidgetNSWindowBridge::OnDisplayAdded(
+ const display::Display& display) {
+ UpdateWindowDisplay();
+}
+
+void NativeWidgetNSWindowBridge::OnDisplayRemoved(
+ const display::Display& display) {
+ UpdateWindowDisplay();
+}
void NativeWidgetNSWindowBridge::OnDisplayMetricsChanged(
const display::Display& display,
diff --git a/ui/display/mac/screen_mac.mm b/ui/display/mac/screen_mac.mm
index 4e0a2cb6991cafa96c1e2077f38ef315544890fc..9f0d860316d2aabc6f344e47a1c5b4d550b6335e 100644
--- a/ui/display/mac/screen_mac.mm
+++ b/ui/display/mac/screen_mac.mm
@@ -32,8 +32,8 @@ Boolean CGDisplayUsesForceToGray(void);
namespace display {
namespace {
-// The delay to handle the display configuration changes.
-// See comments in ScreenMac::HandleDisplayReconfiguration.
+// The delay to handle the display configuration changes. This is in place to
+// coalesce display update notifications and thereby avoid thrashing.
const int64_t kConfigureDelayMs = 500;
NSScreen* GetMatchingScreen(const gfx::Rect& match_rect) {
@@ -159,20 +159,27 @@ class ScreenMac : public Screen {
CGDisplayRegisterReconfigurationCallback(
ScreenMac::DisplayReconfigurationCallBack, this);
+ auto update_block = ^(NSNotification* notification) {
+ OnNSScreensMayHaveChanged();
+ };
+
NSNotificationCenter* center = [NSNotificationCenter defaultCenter];
screen_color_change_observer_.reset(
[[center addObserverForName:NSScreenColorSpaceDidChangeNotification
object:nil
queue:nil
- usingBlock:^(NSNotification* notification) {
- configure_timer_.Reset();
- displays_require_update_ = true;
- }] retain]);
+ usingBlock:update_block] retain]);
+ screen_params_change_observer_.reset([[center
+ addObserverForName:NSApplicationDidChangeScreenParametersNotification
+ object:nil
+ queue:nil
+ usingBlock:update_block] retain]);
}
~ScreenMac() override {
NSNotificationCenter* center = [NSNotificationCenter defaultCenter];
[center removeObserver:screen_color_change_observer_];
+ [center removeObserver:screen_params_change_observer_];
CGDisplayRemoveReconfigurationCallback(
ScreenMac::DisplayReconfigurationCallBack, this);
@@ -197,16 +204,18 @@ class ScreenMac : public Screen {
int GetNumDisplays() const override { return GetAllDisplays().size(); }
const std::vector<Display>& GetAllDisplays() const override {
+ UpdateDisplaysIfNeeded();
return displays_;
}
Display GetDisplayNearestWindow(
gfx::NativeWindow native_window) const override {
- NSWindow* window = native_window.GetNativeNSWindow();
- EnsureDisplaysValid();
+ UpdateDisplaysIfNeeded();
+
if (displays_.size() == 1)
return displays_[0];
+ NSWindow* window = native_window.GetNativeNSWindow();
if (!window)
return GetPrimaryDisplay();
@@ -279,31 +288,30 @@ class ScreenMac : public Screen {
static void DisplayReconfigurationCallBack(CGDirectDisplayID display,
CGDisplayChangeSummaryFlags flags,
void* userInfo) {
- if (flags & kCGDisplayBeginConfigurationFlag)
- return;
-
ScreenMac* screen_mac = static_cast<ScreenMac*>(userInfo);
-
- // Timer::Reset() ensures at least another interval passes before the
- // associated task runs, effectively coalescing these events.
- screen_mac->configure_timer_.Reset();
- screen_mac->displays_require_update_ = true;
+ screen_mac->OnNSScreensMayHaveChanged();
}
private:
Display GetCachedDisplayForScreen(NSScreen* screen) const {
- EnsureDisplaysValid();
+ UpdateDisplaysIfNeeded();
const CGDirectDisplayID display_id = [[[screen deviceDescription]
objectForKey:@"NSScreenNumber"] unsignedIntValue];
for (const Display& display : displays_) {
if (display_id == display.id())
return display;
}
- NOTREACHED(); // Asked for a hidden/sleeping/mirrored screen?
+ // In theory, this should not be reached, because |displays_require_update_|
+ // should have been set prior to -[NSScreen screens] changing. In practice,
+ // on Catalina, it has been observed that -[NSScreen screens] changes before
+ // any notifications are received.
+ // https://crbug.com/1021340.
+ OnNSScreensMayHaveChanged();
+ DLOG(ERROR) << "Value of -[NSScreen screens] changed before notification.";
return BuildDisplayForScreen(screen);
}
- void EnsureDisplaysValid() const {
+ void UpdateDisplaysIfNeeded() const {
if (displays_require_update_) {
displays_ = BuildDisplaysFromQuartz();
displays_require_update_ = false;
@@ -311,7 +319,7 @@ class ScreenMac : public Screen {
}
void ConfigureTimerFired() {
- EnsureDisplaysValid();
+ UpdateDisplaysIfNeeded();
change_notifier_.NotifyDisplaysChanged(old_displays_, displays_);
old_displays_ = displays_;
}
@@ -325,7 +333,7 @@ class ScreenMac : public Screen {
// It would be ridiculous to have this many displays connected, but
// CGDirectDisplayID is just an integer, so supporting up to this many
// doesn't hurt.
- CGDirectDisplayID online_displays[128];
+ CGDirectDisplayID online_displays[1024];
CGDisplayCount online_display_count = 0;
if (CGGetOnlineDisplayList(base::size(online_displays), online_displays,
&online_display_count) != kCGErrorSuccess) {
@@ -361,21 +369,32 @@ class ScreenMac : public Screen {
: displays;
}
- // The displays currently attached to the device. Cached.
+ void OnNSScreensMayHaveChanged() const {
+ // Timer::Reset() ensures at least another interval passes before the
+ // associated task runs, effectively coalescing these events.
+ configure_timer_.Reset();
+ displays_require_update_ = true;
+ }
+
+ // The displays currently attached to the device. Updated by
+ // UpdateDisplaysIfNeeded.
mutable std::vector<Display> displays_;
- // Set whenever the CGDisplayRegisterReconfigurationCallback is invoked and
- // cleared when |displays_| is updated by BuildDisplaysFromQuartz().
+ // Whether or not |displays_| might need to be upated. Set in
+ // OnNSScreensMayHaveChanged, and un-set by UpdateDisplaysIfNeeded.
mutable bool displays_require_update_ = false;
- // The displays last communicated to DisplayChangeNotifier.
- std::vector<Display> old_displays_;
+ // The timer to delay configuring outputs and notifying observers (to coalesce
+ // several updates into one update).
+ mutable base::RetainingOneShotTimer configure_timer_;
- // The timer to delay configuring outputs and notifying observers.
- base::RetainingOneShotTimer configure_timer_;
+ // The displays last communicated to the DisplayChangeNotifier.
+ std::vector<Display> old_displays_;
- // The observer notified by NSScreenColorSpaceDidChangeNotification.
+ // The observers notified by NSScreenColorSpaceDidChangeNotification and
+ // NSApplicationDidChangeScreenParametersNotification.
base::scoped_nsobject<id> screen_color_change_observer_;
+ base::scoped_nsobject<id> screen_params_change_observer_;
DisplayChangeNotifier change_notifier_;