update to final revision of background bug fix from upstream (BGO #722149)

This commit is contained in:
Adam Williamson 2014-03-12 10:01:43 -07:00
parent 2e63a4b65a
commit efa69b10e9
6 changed files with 295 additions and 186 deletions

View File

@ -0,0 +1,72 @@
From ec6facb9e76c916b3faa78af568424694c8da133 Mon Sep 17 00:00:00 2001
From: Ray Strode <rstrode@redhat.com>
Date: Wed, 26 Feb 2014 14:35:38 -0500
Subject: [PATCH 1/4] background: always copy background content when loading
into cache
Copying is actually a lightweight operation, so trying to avoid it just adds
code complexity for little gain.
Based on work from Jasper St. Pierre <jstpierre@macheye.net>
https://bugzilla.gnome.org/show_bug.cgi?id=722149
---
js/ui/background.js | 15 ++++-----------
1 file changed, 4 insertions(+), 11 deletions(-)
diff --git a/js/ui/background.js b/js/ui/background.js
index 3a7541c..3fe279d 100644
--- a/js/ui/background.js
+++ b/js/ui/background.js
@@ -143,8 +143,7 @@ const BackgroundCache = new Lang.Class({
for (let i = 0; i < this._pendingFileLoads.length; i++) {
if (this._pendingFileLoads[i].filename == params.filename &&
this._pendingFileLoads[i].style == params.style) {
- this._pendingFileLoads[i].callers.push({ shouldCopy: true,
- monitorIndex: params.monitorIndex,
+ this._pendingFileLoads[i].callers.push({ monitorIndex: params.monitorIndex,
effects: params.effects,
onFinished: params.onFinished });
return;
@@ -153,14 +152,11 @@ const BackgroundCache = new Lang.Class({
this._pendingFileLoads.push({ filename: params.filename,
style: params.style,
- callers: [{ shouldCopy: false,
- monitorIndex: params.monitorIndex,
+ callers: [{ monitorIndex: params.monitorIndex,
effects: params.effects,
onFinished: params.onFinished }] });
- let content = new Meta.Background({ meta_screen: global.screen,
- monitor: params.monitorIndex,
- effects: params.effects });
+ let content = new Meta.Background({ meta_screen: global.screen });
content.load_file_async(params.filename,
params.style,
@@ -171,7 +167,6 @@ const BackgroundCache = new Lang.Class({
content.load_file_finish(result);
this._monitorFile(params.filename);
- this._images.push(content);
} catch(e) {
content = null;
}
@@ -186,12 +181,10 @@ const BackgroundCache = new Lang.Class({
if (pendingLoad.callers[j].onFinished) {
let newContent;
- if (content && pendingLoad.callers[j].shouldCopy) {
+ if (content) {
newContent = content.copy(pendingLoad.callers[j].monitorIndex,
pendingLoad.callers[j].effects);
this._images.push(newContent);
- } else {
- newContent = content;
}
pendingLoad.callers[j].onFinished(newContent);
--
1.9.0

View File

@ -0,0 +1,70 @@
From e917b7ce0f1dca31683534e1980477b0b88da0de Mon Sep 17 00:00:00 2001
From: Ray Strode <rstrode@redhat.com>
Date: Wed, 26 Feb 2014 15:13:21 -0500
Subject: [PATCH 2/4] background: refactor file loading
This commit moves the code around a bit such that the
caller gets allocated up front and then a file load is either
found or created to attach the caller to.
Functionally, the code is the same, it's just now factored in a way
that will make it easier to fix a bug with cancellation later.
https://bugzilla.gnome.org/show_bug.cgi?id=722149
---
js/ui/background.js | 27 +++++++++++++++++----------
1 file changed, 17 insertions(+), 10 deletions(-)
diff --git a/js/ui/background.js b/js/ui/background.js
index 3fe279d..396309a 100644
--- a/js/ui/background.js
+++ b/js/ui/background.js
@@ -132,6 +132,10 @@ const BackgroundCache = new Lang.Class({
this._removeContent(this._images, content);
},
+ _attachCallerToFileLoad: function(caller, fileLoad) {
+ fileLoad.callers.push(caller);
+ },
+
_loadImageContent: function(params) {
params = Params.parse(params, { monitorIndex: 0,
style: null,
@@ -140,21 +144,24 @@ const BackgroundCache = new Lang.Class({
cancellable: null,
onFinished: null });
+ let caller = { monitorIndex: params.monitorIndex,
+ effects: params.effects,
+ onFinished: params.onFinished };
+
for (let i = 0; i < this._pendingFileLoads.length; i++) {
- if (this._pendingFileLoads[i].filename == params.filename &&
- this._pendingFileLoads[i].style == params.style) {
- this._pendingFileLoads[i].callers.push({ monitorIndex: params.monitorIndex,
- effects: params.effects,
- onFinished: params.onFinished });
+ let fileLoad = this._pendingFileLoads[i];
+
+ if (fileLoad.filename == params.filename &&
+ fileLoad.style == params.style) {
+ this._attachCallerToFileLoad(caller, fileLoad);
return;
}
}
- this._pendingFileLoads.push({ filename: params.filename,
- style: params.style,
- callers: [{ monitorIndex: params.monitorIndex,
- effects: params.effects,
- onFinished: params.onFinished }] });
+ let fileLoad = { filename: params.filename,
+ style: params.style,
+ callers: [] };
+ this._attachCallerToFileLoad(caller, fileLoad);
let content = new Meta.Background({ meta_screen: global.screen });
--
1.9.0

View File

@ -0,0 +1,67 @@
From fdf264ff64cd0e67fc5310f6720ea4586b72890d Mon Sep 17 00:00:00 2001
From: Ray Strode <rstrode@redhat.com>
Date: Wed, 26 Feb 2014 16:09:26 -0500
Subject: [PATCH 3/4] background: get rid of nested loop when finishing file
loading
At the moment when a file is loaded, we iterate through the list of
pending file loads and ignore any unrelated to the file, then iterate
all the callers of the related file loads and finish them.
In fact, there can only ever be one pending file load related to the
file, and we already know it, so we can avoid the ugly nested loops.
https://bugzilla.gnome.org/show_bug.cgi?id=722149
---
js/ui/background.js | 32 +++++++++++++-------------------
1 file changed, 13 insertions(+), 19 deletions(-)
diff --git a/js/ui/background.js b/js/ui/background.js
index 396309a..506fe59 100644
--- a/js/ui/background.js
+++ b/js/ui/background.js
@@ -178,28 +178,22 @@ const BackgroundCache = new Lang.Class({
content = null;
}
- for (let i = 0; i < this._pendingFileLoads.length; i++) {
- let pendingLoad = this._pendingFileLoads[i];
- if (pendingLoad.filename != params.filename ||
- pendingLoad.style != params.style)
- continue;
-
- for (let j = 0; j < pendingLoad.callers.length; j++) {
- if (pendingLoad.callers[j].onFinished) {
- let newContent;
-
- if (content) {
- newContent = content.copy(pendingLoad.callers[j].monitorIndex,
- pendingLoad.callers[j].effects);
- this._images.push(newContent);
- }
-
- pendingLoad.callers[j].onFinished(newContent);
+ for (let i = 0; i < fileLoad.callers.length; i++) {
+ let caller = fileLoad.callers[i];
+ if (caller.onFinished) {
+ let newContent;
+
+ if (content) {
+ newContent = content.copy(caller.monitorIndex, caller.effects);
+ this._images.push(newContent);
}
- }
- this._pendingFileLoads.splice(i, 1);
+ caller.onFinished(newContent);
+ }
}
+
+ let idx = this._pendingFileLoads.indexOf(fileLoad);
+ this._pendingFileLoads.splice(idx, 1);
}));
},
--
1.9.0

View File

@ -0,0 +1,73 @@
From 55d1c7e2ab9fb675f8f5a6e4c03ff17093a01cff Mon Sep 17 00:00:00 2001
From: Ray Strode <rstrode@redhat.com>
Date: Wed, 26 Feb 2014 15:45:14 -0500
Subject: [PATCH 4/4] background: fix cancellable issue
If we have the following sequence:
cache.getImageContent({ filename: "foo", cancellable: cancellable1 });
cache.getImageContent({ filename: "foo", cancellable: cancellable2 });
cancellable1.cancel();
Then the second load will complete with "null" as its content, even though
it was never cancelled, and we'll see a blank image. Meanwhile, since the
second load simply appends to the list of callers for the second load,
cancellable2 does absolutely nothing: cancelling it won't stop the load,
and it will still receive onFinished handling.
To prevent this from happening, give the actual load operation its own
Gio.Cancellable, which is "ref-counted" -- only cancel it when all the other
possible callers cancel.
Based on work from Jasper St. Pierre <jstpierre@macheye.net>
https://bugzilla.gnome.org/show_bug.cgi?id=722149
---
js/ui/background.js | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/js/ui/background.js b/js/ui/background.js
index 506fe59..b1eb57a 100644
--- a/js/ui/background.js
+++ b/js/ui/background.js
@@ -134,6 +134,21 @@ const BackgroundCache = new Lang.Class({
_attachCallerToFileLoad: function(caller, fileLoad) {
fileLoad.callers.push(caller);
+
+ if (!caller.cancellable)
+ return;
+
+ caller.cancellable.connect(Lang.bind(this, function() {
+ let idx = fileLoad.callers.indexOf(caller);
+ fileLoad.callers.splice(idx, 1);
+
+ if (fileLoad.callers.length == 0) {
+ fileLoad.cancellable.cancel();
+
+ let idx = this._pendingFileLoads.indexOf(fileLoad);
+ this._pendingFileLoads.splice(idx, 1);
+ }
+ }));
},
_loadImageContent: function(params) {
@@ -146,6 +161,7 @@ const BackgroundCache = new Lang.Class({
let caller = { monitorIndex: params.monitorIndex,
effects: params.effects,
+ cancellable: params.cancellable,
onFinished: params.onFinished };
for (let i = 0; i < this._pendingFileLoads.length; i++) {
@@ -160,6 +176,7 @@ const BackgroundCache = new Lang.Class({
let fileLoad = { filename: params.filename,
style: params.style,
+ cancellable: new Gio.Cancellable(),
callers: [] };
this._attachCallerToFileLoad(caller, fileLoad);
--
1.9.0

View File

@ -1,183 +0,0 @@
From b5d7bd02b514b9862203ff30435b6c98766143ef Mon Sep 17 00:00:00 2001
From: "Jasper St. Pierre" <jstpierre@mecheye.net>
Date: Mon, 13 Jan 2014 18:42:32 -0500
Subject: [PATCH] background: Fix cancellable issues
If we have the following sequence:
cache.getImageContent({ filename: "foo", cancellable: cancellable1 });
cache.getImageContent({ filename: "foo", cancellable: cancellable2 });
cancellable1.cancel();
Then the second load will complete with "null" as its content, even though
it was never cancelled, and we'll see a blank image. Meanwhile, since the
second load simply appends to the list of callers for the second load,
cancellable2 does absolutely nothing: cancelling it won't stop the load,
and it will still receive onFinished handling.
To prevent this from happening, give the actual load operation its own
Gio.Cancellable, which is "ref-counted" -- only cancel it when all the other
possible callers cancel.
Additionally, clean up the large nested loops by splitting out duplicated
code and other stuff.
https://bugzilla.gnome.org/show_bug.cgi?id=722149
---
js/ui/background.js | 120 ++++++++++++++++++++++++++++------------------------
1 file changed, 65 insertions(+), 55 deletions(-)
diff --git a/js/ui/background.js b/js/ui/background.js
index 3a7541c..a3224f5 100644
--- a/js/ui/background.js
+++ b/js/ui/background.js
@@ -132,6 +132,46 @@ const BackgroundCache = new Lang.Class({
this._removeContent(this._images, content);
},
+ _loadImageContentInternal: function(filename, style) {
+ let cancellable = new Gio.Cancellable();
+ let content = new Meta.Background({ meta_screen: global.screen });
+
+ let info = { filename: filename,
+ style: style,
+ cancellable: cancellable,
+ callers: [] };
+
+ content.load_file_async(filename, style, cancellable, Lang.bind(this, function(object, result) {
+ if (cancellable.is_cancelled())
+ return;
+
+ try {
+ content.load_file_finish(result);
+ } catch(e) {
+ content = null;
+ }
+
+ if (content) {
+ this._monitorFile(filename);
+ info.callers.forEach(Lang.bind(this, function(caller) {
+ let newContent = content.copy(caller.monitorIndex, caller.effects);
+ this._images.push(newContent);
+ caller.onFinished(newContent);
+ }));
+ } else {
+ info.callers.forEach(Lang.bind(this, function(caller) {
+ caller.onFinished(null);
+ }));
+ }
+
+ let idx = this._pendingFileLoads.indexOf(info);
+ this._pendingFileLoads.splice(idx, 1);
+ }));
+
+ this._pendingFileLoads.push(info);
+ return info;
+ },
+
_loadImageContent: function(params) {
params = Params.parse(params, { monitorIndex: 0,
style: null,
@@ -140,67 +180,38 @@ const BackgroundCache = new Lang.Class({
cancellable: null,
onFinished: null });
+ let caller = { monitorIndex: params.monitorIndex,
+ effects: params.effects,
+ cancellable: params.cancellable,
+ onFinished: params.onFinished };
+
+ let info = null;
for (let i = 0; i < this._pendingFileLoads.length; i++) {
- if (this._pendingFileLoads[i].filename == params.filename &&
- this._pendingFileLoads[i].style == params.style) {
- this._pendingFileLoads[i].callers.push({ shouldCopy: true,
- monitorIndex: params.monitorIndex,
- effects: params.effects,
- onFinished: params.onFinished });
- return;
+ let pendingLoad = this._pendingFileLoads[i];
+ if (pendingLoad.filename == params.filename && pendingLoad.style == params.style) {
+ info = pendingLoad;
+ break;
}
}
- this._pendingFileLoads.push({ filename: params.filename,
- style: params.style,
- callers: [{ shouldCopy: false,
- monitorIndex: params.monitorIndex,
- effects: params.effects,
- onFinished: params.onFinished }] });
+ if (!info)
+ info = this._loadImageContentInternal(params.filename, params.style);
- let content = new Meta.Background({ meta_screen: global.screen,
- monitor: params.monitorIndex,
- effects: params.effects });
+ info.callers.push(caller);
- content.load_file_async(params.filename,
- params.style,
- params.cancellable,
- Lang.bind(this,
- function(object, result) {
- try {
- content.load_file_finish(result);
-
- this._monitorFile(params.filename);
- this._images.push(content);
- } catch(e) {
- content = null;
- }
+ if (caller.cancellable) {
+ caller.cancellable.connect(Lang.bind(this, function() {
+ let idx = info.callers.indexOf(caller);
+ info.callers.splice(idx, 1);
- for (let i = 0; i < this._pendingFileLoads.length; i++) {
- let pendingLoad = this._pendingFileLoads[i];
- if (pendingLoad.filename != params.filename ||
- pendingLoad.style != params.style)
- continue;
-
- for (let j = 0; j < pendingLoad.callers.length; j++) {
- if (pendingLoad.callers[j].onFinished) {
- let newContent;
-
- if (content && pendingLoad.callers[j].shouldCopy) {
- newContent = content.copy(pendingLoad.callers[j].monitorIndex,
- pendingLoad.callers[j].effects);
- this._images.push(newContent);
- } else {
- newContent = content;
- }
-
- pendingLoad.callers[j].onFinished(newContent);
- }
- }
-
- this._pendingFileLoads.splice(i, 1);
- }
- }));
+ if (info.callers.length == 0) {
+ info.cancellable.cancel();
+
+ let idx = this._pendingFileLoads.indexOf(info);
+ this._pendingFileLoads.splice(idx, 1);
+ }
+ }));
+ }
},
getImageContent: function(params) {
@@ -250,7 +261,6 @@ const BackgroundCache = new Lang.Class({
monitorIndex: params.monitorIndex,
cancellable: params.cancellable,
onFinished: params.onFinished });
-
}
},
--
1.8.5.3

View File

@ -1,6 +1,6 @@
Name: gnome-shell
Version: 3.11.91
Release: 1%{?dist}
Release: 2%{?dist}
Summary: Window management and application launching for GNOME
Group: User Interface/Desktops
@ -14,7 +14,11 @@ Source0: http://download.gnome.org/sources/gnome-shell/3.11/%{name}-%{ver
Patch1: gnome-shell-favourite-apps-firefox.patch
# https://bugzilla.gnome.org/show_bug.cgi?id=722149
Patch2: gnome-shell-3.11.90-background.patch
# Patch series all backported from upstream git master, will be in 3.11.92
Patch2: 0001-background-always-copy-background-content-when-loadi.patch
Patch3: 0002-background-refactor-file-loading.patch
Patch4: 0003-background-get-rid-of-nested-loop-when-finishing-fil.patch
Patch5: 0004-background-fix-cancellable-issue.patch
%define clutter_version 1.13.4
%define gnome_bluetooth_version 1:3.9.0
@ -119,7 +123,10 @@ easy to use experience.
%prep
%setup -q
%patch1 -p1 -b .firefox
%patch2 -p1 -b .background
%patch2 -p1 -b .background1
%patch3 -p1 -b .background2
%patch4 -p1 -b .background3
%patch5 -p1 -b .background4
%build
(if ! test -x configure; then NOCONFIGURE=1 ./autogen.sh; fi;
@ -179,6 +186,9 @@ glib-compile-schemas --allow-any-name %{_datadir}/glib-2.0/schemas &> /dev/null
%exclude %{_datadir}/gtk-doc
%changelog
* Wed Mar 12 2014 Adam Williamson <awilliam@redhat.com> - 3.11.91-2
- update to final revision of background bug fix from upstream (BGO #722149)
* Thu Mar 06 2014 Florian Müllner <fmuellner@redhat.com> - 3.11.91-1
- Update to 3.11.91