Merge ~3v1n0/ubuntu/+source/gnome-shell:ubuntu/bionic into ~ubuntu-desktop/ubuntu/+source/gnome-shell:ubuntu/bionic

Proposed by Marco Trevisan (Treviño) on 2018-07-24
Status: Merged
Merge reported by: Iain Lane
Merged at revision: 7d48524cf3055aa3e23d7762d81b730c5261be78
Proposed branch: ~3v1n0/ubuntu/+source/gnome-shell:ubuntu/bionic
Merge into: ~ubuntu-desktop/ubuntu/+source/gnome-shell:ubuntu/bionic
Diff against target: 439 lines (+102/-210)
3 files modified
debian/changelog (+3/-1)
debian/patches/js-fix-invalid-access-errors.patch (+81/-172)
debian/patches/workspace-fix-repositioned-windows-in-activities.patch (+18/-37)
Reviewer Review Type Date Requested Status
Iain Lane 2018-07-24 Approve on 2018-07-25
Review via email: mp+350755@code.launchpad.net
To post a comment you must log in.
Iain Lane (laney) wrote :

Thanks.

Since the previous SRU was uploaded + tagged, I rebased this on top of the ubuntu/bionic branch and made a new changelog entry for .2. I checked the diff vs. the upload that went to cosmic just now and there's nothing in the content, just metadata.

This is merged but since there was a rebase Launchpad isn't going to know that.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/debian/changelog b/debian/changelog
2index 8468ec3..6839363 100644
3--- a/debian/changelog
4+++ b/debian/changelog
5@@ -9,7 +9,9 @@ gnome-shell (3.28.3-0ubuntu0.18.04.1) bionic; urgency=medium
6 debian/patches/ui-Theme-lookup-should-respect-XDG_DATA_DIRS.patch:
7 - Drop patches applied on the 3.28 branch
8 * debian/patches/js-fix-invalid-access-errors.patch:
9- - Refreshed to reflect upstream changes
10+ - Updated to include upstream requested changes
11+ * debian/patches/workspace-fix-repositioned-windows-in-activities.patch:
12+ - Cherry-pick from upstream 3.28 branch
13
14 -- Marco Trevisan (Treviño) <marco@ubuntu.com> Tue, 24 Jul 2018 12:46:27 +0100
15
16diff --git a/debian/patches/js-fix-invalid-access-errors.patch b/debian/patches/js-fix-invalid-access-errors.patch
17index c86fc8b..a272d79 100644
18--- a/debian/patches/js-fix-invalid-access-errors.patch
19+++ b/debian/patches/js-fix-invalid-access-errors.patch
20@@ -24,47 +24,37 @@ Remove duplicated handler on 'destroy' and just use a generic one.
21
22 https://bugzilla.gnome.org/show_bug.cgi?id=791233
23
24-workspaceThumbnail: Disconnect from window signals on destruction
25-
26-Avoid to try to destroy the window clone content more than once
27-when a window is destroyed, and do it in proper order.
28+automountManager: remove allowAutorun expire timeout on volume removal
29+
30+If the volume is removed before AUTORUN_EXPIRE_TIMEOUT_SECS seconds, we can stop
31+the timeout earlier as there's nothing to unset, while the volume instance
32+won't be valid anymore.
33
34 https://bugzilla.gnome.org/show_bug.cgi?id=791233
35
36-workspaceThumbnail: Remove WindowClone's from _windows when destroyed
37-
38-A WindowClone might be destroyed earlier than its MetaWindow counterpart
39-as its WindowActor could be destroyed earlier, thus when happens it's safer
40-to remove the clone from the windows list, without waiting for the workspace
41-to request to do so.
42+messageList: stop syncing if closeButton has been destroyed
43
44-WindowClone now emits a 'destroy' signals earlier enough and this now
45-triggers a _doRemoveWindow on WorkspaceThumbnail which will lead
46-to the proper cleanup; keeping track of the signal connections, in
47-order to avoid callback loops (not really harmful in this case, but
48-good practice).
49+The _sync function for Message only updates the close button visibility,
50+so we can safely stop doing that if the close button get get destroyed earlier
51+(as it happens when clicking on it).
52
53 https://bugzilla.gnome.org/show_bug.cgi?id=791233
54
55-workspace: Disconnect from window signals on destruction
56+workspaceThumbnail: Don't keep stale clones in list
57
58-Avoid to try to destroy the window clone content more than once
59-when a window is destroyed, and do it in proper order.
60+If a clone gets destroyed before the corresponding MetaWindow is
61+removed from the workspace, we will still find it in the list of
62+clones and try to destroy it again. Avoid the resulting warnings
63+by updating the list of clones immediately when a clone is destroyed.
64
65 https://bugzilla.gnome.org/show_bug.cgi?id=791233
66
67-workspace: Remove WindowClone's from _windows when destroyed
68-
69-A WindowClone might be destroyed earlier than its MetaWindow counterpart
70-as its WindowActor could be destroyed earlier, thus when happens it's safer
71-to remove the clone from the windows list, without waiting for the workspace
72-to request to do so.
73+workspace: Don't keep stale clones in list
74
75-WindowClone now emits a 'destroy' signals earlier enough and this now
76-triggers a _doRemoveWindow on WorkspaceThumbnail which will lead
77-to the proper cleanup; keeping track of the signal connections, in
78-order to avoid callback loops (not really harmful in this case, but
79-good practice).
80+If a clone gets destroyed before the corresponding MetaWindow is
81+removed from the workspace, we will still find it in the list of
82+clones and try to destroy it again. Avoid the resulting warnings
83+by updating the list of clones immediately when a clone is destroyed.
84
85 https://bugzilla.gnome.org/show_bug.cgi?id=791233
86
87@@ -72,12 +62,52 @@ Bug-GNOME: https://bugzilla.gnome.org/show_bug.cgi?id=791233
88 Bug-Ubuntu: https://bugs.launchpad.net/ubuntu/bionic/+source/gnome-shell/+bug/1747566
89 Forwarded: yes, https://gitlab.gnome.org/GNOME/gnome-shell/merge_requests/4
90 ---
91- js/ui/dnd.js | 65 +++++++++++++++++++++++++++------------------
92- js/ui/tweener.js | 64 +++++++++++++++++++++++++++++++++++---------
93- js/ui/workspace.js | 26 +++++++++++-------
94- js/ui/workspaceThumbnail.js | 21 ++++++++++-----
95- 4 files changed, 122 insertions(+), 54 deletions(-)
96+ js/ui/calendar.js | 2 ++
97+ js/ui/components/automountManager.js | 6 ++++
98+ js/ui/dnd.js | 65 +++++++++++++++++++++---------------
99+ js/ui/messageList.js | 3 +-
100+ js/ui/tweener.js | 63 ++++++++++++++++++++++++++--------
101+ 5 files changed, 99 insertions(+), 40 deletions(-)
102
103+diff --git a/js/ui/calendar.js b/js/ui/calendar.js
104+index 651aac6..fd133cc 100644
105+--- a/js/ui/calendar.js
106++++ b/js/ui/calendar.js
107+@@ -802,6 +802,8 @@ var NotificationMessage = new Lang.Class({
108+ },
109+
110+ _onDestroy() {
111++ this.parent();
112++
113+ if (this._updatedId)
114+ this.notification.disconnect(this._updatedId);
115+ this._updatedId = 0;
116+diff --git a/js/ui/components/automountManager.js b/js/ui/components/automountManager.js
117+index 2d8f3f8..a6cd857 100644
118+--- a/js/ui/components/automountManager.js
119++++ b/js/ui/components/automountManager.js
120+@@ -210,6 +210,10 @@ var AutomountManager = new Lang.Class({
121+ },
122+
123+ _onVolumeRemoved(monitor, volume) {
124++ if (volume._allowAutorunExpireId && volume._allowAutorunExpireId > 0) {
125++ Mainloop.source_remove(volume._allowAutorunExpireId);
126++ delete volume._allowAutorunExpireId;
127++ }
128+ this._volumeQueue =
129+ this._volumeQueue.filter(element => (element != volume));
130+ },
131+@@ -234,8 +238,10 @@ var AutomountManager = new Lang.Class({
132+ _allowAutorunExpire(volume) {
133+ let id = Mainloop.timeout_add_seconds(AUTORUN_EXPIRE_TIMEOUT_SECS, () => {
134+ volume.allowAutorun = false;
135++ delete volume._allowAutorunExpireId;
136+ return GLib.SOURCE_REMOVE;
137+ });
138++ volume._allowAutorunExpireId = id;
139+ GLib.Source.set_name_by_id(id, '[gnome-shell] volume.allowAutorun');
140+ }
141+ });
142 diff --git a/js/ui/dnd.js b/js/ui/dnd.js
143 index a38607c..431c60d 100644
144 --- a/js/ui/dnd.js
145@@ -275,11 +305,25 @@ index a38607c..431c60d 100644
146 currentDraggable = null;
147 }
148 });
149+diff --git a/js/ui/messageList.js b/js/ui/messageList.js
150+index aff201e..2d397c1 100644
151+--- a/js/ui/messageList.js
152++++ b/js/ui/messageList.js
153+@@ -362,7 +362,8 @@ var Message = new Lang.Class({
154+ this.setBody(body);
155+
156+ this._closeButton.connect('clicked', this.close.bind(this));
157+- this.actor.connect('notify::hover', this._sync.bind(this));
158++ let actorHoverId = this.actor.connect('notify::hover', this._sync.bind(this));
159++ this._closeButton.connect('destroy', this.actor.disconnect.bind(this.actor, actorHoverId));
160+ this.actor.connect('clicked', this._onClicked.bind(this));
161+ this.actor.connect('destroy', this._onDestroy.bind(this));
162+ this._sync();
163 diff --git a/js/ui/tweener.js b/js/ui/tweener.js
164-index 1a85e2f..663b97b 100644
165+index 1a85e2f..22818ba 100644
166 --- a/js/ui/tweener.js
167 +++ b/js/ui/tweener.js
168-@@ -69,30 +69,68 @@ function _getTweenState(target) {
169+@@ -69,30 +69,67 @@ function _getTweenState(target) {
170 return target.__ShellTweenerState;
171 }
172
173@@ -320,8 +364,7 @@ index 1a85e2f..663b97b 100644
174 + let wrapperNeeded = false;
175 + let tweenerHandlers = _ensureHandlers(target);
176 +
177-+ if (!(name in tweenerHandlers))
178-+ {
179++ if (!(name in tweenerHandlers)) {
180 + tweenerHandlers[name] = [];
181 + wrapperNeeded = true;
182 + }
183@@ -361,137 +404,3 @@ index 1a85e2f..663b97b 100644
184 }
185
186 function _actorDestroyed(target) {
187-diff --git a/js/ui/workspace.js b/js/ui/workspace.js
188-index 8b56932..c148ccb 100644
189---- a/js/ui/workspace.js
190-+++ b/js/ui/workspace.js
191-@@ -139,14 +139,8 @@ var WindowClone = new Lang.Class({
192-
193- this._windowClone._updateId = this.metaWindow.connect('size-changed',
194- this._onRealWindowSizeChanged.bind(this));
195-- this._windowClone._destroyId =
196-- this.realWindow.connect('destroy', () => {
197-- // First destroy the clone and then destroy everything
198-- // This will ensure that we never see it in the
199-- // _disconnectSignals loop
200-- this._windowClone.destroy();
201-- this.destroy();
202-- });
203-+ this._windowClone._destroyId = this.realWindow.connect('destroy',
204-+ this.destroy.bind(this));
205-
206- this._updateAttachedDialogs();
207- this._computeBoundingBox();
208-@@ -310,6 +304,14 @@ var WindowClone = new Lang.Class({
209- },
210-
211- destroy() {
212-+ this.emit('destroy');
213-+
214-+ // First destroy the clone and then destroy everything
215-+ // This will ensure that we never see it in the _disconnectSignals loop
216-+ this.metaWindow.disconnect(this._windowClone._updateId);
217-+ this.realWindow.disconnect(this._windowClone._destroyId);
218-+ this._windowClone.destroy();
219-+
220- this.actor.destroy();
221- },
222-
223-@@ -1131,6 +1133,7 @@ var Workspace = new Lang.Class({
224- // Create clones for windows that should be
225- // visible in the Overview
226- this._windows = [];
227-+ this._windowsDestroyedIds = [];
228- this._windowOverlays = [];
229- for (let i = 0; i < windows.length; i++) {
230- if (this._isOverviewWindow(windows[i])) {
231-@@ -1428,7 +1431,7 @@ var Workspace = new Lang.Class({
232- return GLib.SOURCE_REMOVE;
233- },
234-
235-- _doRemoveWindow(metaWin) {
236-+ _doRemoveWindow(metaWin, {cloneDestroy}={cloneDestroy: true}) {
237- let win = metaWin.get_compositor_private();
238-
239- let clone = this._removeWindowClone(metaWin);
240-@@ -1792,7 +1795,11 @@ var Workspace = new Lang.Class({
241- this._actualGeometryLater = 0;
242- }
243-
244-+ for (let index = 0; index < this._windows.length; ++index)
245-+ this._windows[index].disconnect(this._windowsDestroyedIds[index]);
246-+
247- this._windows = [];
248-+ this._windowsDestroyedIds = [];
249- },
250-
251- // Sets this.leavingOverview flag to false.
252-@@ -1859,6 +1866,7 @@ var Workspace = new Lang.Class({
253- clone.setStackAbove(this._windows[this._windows.length - 1].actor);
254-
255- this._windows.push(clone);
256-+ this._windowsDestroyedIds.push(cloneDestroyedId);
257- this._windowOverlays.push(overlay);
258-
259- return [clone, overlay];
260-diff --git a/js/ui/workspaceThumbnail.js b/js/ui/workspaceThumbnail.js
261-index 381169e..016dda6 100644
262---- a/js/ui/workspaceThumbnail.js
263-+++ b/js/ui/workspaceThumbnail.js
264-@@ -70,12 +70,7 @@ var WindowClone = new Lang.Class({
265-
266- this.clone._updateId = this.metaWindow.connect('position-changed',
267- this._onPositionChanged.bind(this));
268-- this.clone._destroyId = this.realWindow.connect('destroy', () => {
269-- // First destroy the clone and then destroy everything
270-- // This will ensure that we never see it in the _disconnectSignals loop
271-- this.clone.destroy();
272-- this.destroy();
273-- });
274-+ this.clone._destroyId = this.realWindow.connect('destroy', this.destroy.bind(this));
275- this._onPositionChanged();
276-
277- this.actor.connect('button-release-event',
278-@@ -142,6 +137,14 @@ var WindowClone = new Lang.Class({
279- },
280-
281- destroy() {
282-+ this.emit('destroy');
283-+
284-+ // First destroy the clone and then destroy everything
285-+ // This will ensure that we never see it in the _disconnectSignals loop
286-+ this.metaWindow.disconnect(this.clone._updateId);
287-+ this.realWindow.disconnect(this.clone._destroyId);
288-+ this.clone.destroy();
289-+
290- this.actor.destroy();
291- },
292-
293-@@ -285,6 +288,7 @@ var WorkspaceThumbnail = new Lang.Class({
294-
295- // Create clones for windows that should be visible in the Overview
296- this._windows = [];
297-+ this._windowsDestroyedIds = [];
298- this._allWindows = [];
299- this._minimizedChangedIds = [];
300- for (let i = 0; i < windows.length; i++) {
301-@@ -493,7 +497,11 @@ var WorkspaceThumbnail = new Lang.Class({
302- this._bgManager = null;
303- }
304-
305-+ for (let index = 0; index < this._windows.length; ++index)
306-+ this._windows[index].disconnect(this._windowsDestroyedIds[index]);
307-+
308- this._windows = [];
309-+ this._windowsDestroyedIds = [];
310- this.actor = null;
311- },
312-
313-@@ -537,6 +545,7 @@ var WorkspaceThumbnail = new Lang.Class({
314- clone.setStackAbove(this._windows[this._windows.length - 1].actor);
315-
316- this._windows.push(clone);
317-+ this._windowsDestroyedIds.push(cloneDestroyedId);
318-
319- return clone;
320- },
321diff --git a/debian/patches/workspace-fix-repositioned-windows-in-activities.patch b/debian/patches/workspace-fix-repositioned-windows-in-activities.patch
322index 5ff0ae2..7da103a 100644
323--- a/debian/patches/workspace-fix-repositioned-windows-in-activities.patch
324+++ b/debian/patches/workspace-fix-repositioned-windows-in-activities.patch
325@@ -20,12 +20,12 @@ Bug-GNOME: https://bugzilla.gnome.org/show_bug.cgi?id=776588
326 Bug-Ubuntu: https://bugs.launchpad.net/gnome-shell/+bug/1653153
327 Forwarded: yes, https://gitlab.gnome.org/GNOME/gnome-shell/merge_requests/8
328 ---
329- js/ui/workspace.js | 28 +++++++++++++++-------------
330- js/ui/workspaceThumbnail.js | 9 ++++-----
331- 2 files changed, 19 insertions(+), 18 deletions(-)
332+ js/ui/workspace.js | 25 +++++++++++++------------
333+ js/ui/workspaceThumbnail.js | 7 +++----
334+ 2 files changed, 16 insertions(+), 16 deletions(-)
335
336 diff --git a/js/ui/workspace.js b/js/ui/workspace.js
337-index 8836537..f06b78e 100644
338+index 8b56932..112c54e 100644
339 --- a/js/ui/workspace.js
340 +++ b/js/ui/workspace.js
341 @@ -137,8 +137,10 @@ var WindowClone = new Lang.Class({
342@@ -38,10 +38,10 @@ index 8836537..f06b78e 100644
343 + this._onMetaWindowSizeChanged.bind(this));
344 + this._windowClone._posChangedId = this.metaWindow.connect('position-changed',
345 + this._computeBoundingBox.bind(this));
346- this._windowClone._destroyId = this.realWindow.connect('destroy',
347- this.destroy.bind(this));
348-
349-@@ -200,8 +202,7 @@ var WindowClone = new Lang.Class({
350+ this._windowClone._destroyId =
351+ this.realWindow.connect('destroy', () => {
352+ // First destroy the clone and then destroy everything
353+@@ -206,8 +208,7 @@ var WindowClone = new Lang.Class({
354
355 addAttachedDialog(win) {
356 this._doAddAttachedDialog(win, win.get_compositor_private());
357@@ -51,7 +51,7 @@ index 8836537..f06b78e 100644
358 },
359
360 hasAttachedDialogs() {
361-@@ -210,15 +211,14 @@ var WindowClone = new Lang.Class({
362+@@ -216,15 +217,14 @@ var WindowClone = new Lang.Class({
363
364 _doAddAttachedDialog(metaWin, realWin) {
365 let clone = new Clutter.Clone({ source: realWin });
366@@ -68,21 +68,11 @@ index 8836537..f06b78e 100644
367
368 - this._computeBoundingBox();
369 - this.emit('size-changed');
370-+ this._onMetaWindowSizeChanged.bind(this);
371++ this._onMetaWindowSizeChanged();
372 });
373 this.actor.add_child(clone);
374 },
375-@@ -308,7 +308,8 @@ var WindowClone = new Lang.Class({
376-
377- // First destroy the clone and then destroy everything
378- // This will ensure that we never see it in the _disconnectSignals loop
379-- this.metaWindow.disconnect(this._windowClone._updateId);
380-+ this.metaWindow.disconnect(this._windowClone._sizeChangedId);
381-+ this.metaWindow.disconnect(this._windowClone._posChangedId);
382- this.realWindow.disconnect(this._windowClone._destroyId);
383- this._windowClone.destroy();
384-
385-@@ -323,12 +324,13 @@ var WindowClone = new Lang.Class({
386+@@ -321,12 +321,13 @@ var WindowClone = new Lang.Class({
387 else
388 realWindow = child.source;
389
390@@ -99,7 +89,7 @@ index 8836537..f06b78e 100644
391 this.emit('size-changed');
392 },
393 diff --git a/js/ui/workspaceThumbnail.js b/js/ui/workspaceThumbnail.js
394-index 0c72e74..4db8c19 100644
395+index 381169e..417296d 100644
396 --- a/js/ui/workspaceThumbnail.js
397 +++ b/js/ui/workspaceThumbnail.js
398 @@ -68,7 +68,7 @@ var WindowClone = new Lang.Class({
399@@ -109,27 +99,18 @@ index 0c72e74..4db8c19 100644
400 - this.clone._updateId = this.metaWindow.connect('position-changed',
401 + this.clone._updateId = this.realWindow.connect('notify::position',
402 this._onPositionChanged.bind(this));
403- this.clone._destroyId = this.realWindow.connect('destroy', this.destroy.bind(this));
404- this._onPositionChanged();
405-@@ -141,7 +141,7 @@ var WindowClone = new Lang.Class({
406-
407- // First destroy the clone and then destroy everything
408- // This will ensure that we never see it in the _disconnectSignals loop
409-- this.metaWindow.disconnect(this.clone._updateId);
410-+ this.realWindow.disconnect(this.clone._updateId);
411- this.realWindow.disconnect(this.clone._destroyId);
412- this.clone.destroy();
413-
414-@@ -156,7 +156,7 @@ var WindowClone = new Lang.Class({
415+ this.clone._destroyId = this.realWindow.connect('destroy', () => {
416+ // First destroy the clone and then destroy everything
417+@@ -153,7 +153,7 @@ var WindowClone = new Lang.Class({
418 let clone = new Clutter.Clone({ source: realDialog });
419 this._updateDialogPosition(realDialog, clone);
420
421 - clone._updateId = metaDialog.connect('position-changed', dialog => {
422-+ clone._updateId = metaDialog.connect('notify::position', dialog => {
423++ clone._updateId = realDialog.connect('notify::position', dialog => {
424 this._updateDialogPosition(dialog, clone);
425 });
426 clone._destroyId = realDialog.connect('destroy', () => {
427-@@ -174,7 +174,6 @@ var WindowClone = new Lang.Class({
428+@@ -171,7 +171,6 @@ var WindowClone = new Lang.Class({
429 },
430
431 _onPositionChanged() {
432@@ -137,7 +118,7 @@ index 0c72e74..4db8c19 100644
433 this.actor.set_position(this.realWindow.x, this.realWindow.y);
434 },
435
436-@@ -182,7 +181,7 @@ var WindowClone = new Lang.Class({
437+@@ -179,7 +178,7 @@ var WindowClone = new Lang.Class({
438 this.actor.get_children().forEach(child => {
439 let realWindow = child.source;
440

Subscribers

People subscribed via source and target branches