From 35484151cece2dbb3d6c6b81c4ca285b1897e84c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Florian=20M=C3=BCllner?= <fmuellner@gnome.org>
Date: Tue, 24 Mar 2020 07:15:26 +0100
Subject: [PATCH] shell/screenshot: Throw error on failure

Commit da537cda43 moved the Shell.Screenshot API to GIO's async pattern,
but we never set the GError passed to the *_finish() functions and only
indicate failure by returning FALSE.

The expected behavior is to throw an error in that situation, so make sure
we do that.

https://gitlab.gnome.org/GNOME/gnome-shell/-/merge_requests/1123
---
 js/ui/screenshot.js    | 39 ++++++++++++++++++++-------------------
 src/shell-screenshot.c | 18 +++++++++---------
 2 files changed, 29 insertions(+), 28 deletions(-)

diff --git a/js/ui/screenshot.js b/js/ui/screenshot.js
index 78afc4ae4..7aed8d01b 100644
--- a/js/ui/screenshot.js
+++ b/js/ui/screenshot.js
@@ -113,16 +113,14 @@ var ScreenshotService = class {
         return [null, null];
     }
 
-    _onScreenshotComplete(result, area, stream, file, flash, invocation) {
-        if (result) {
-            if (flash) {
-                let flashspot = new Flashspot(area);
-                flashspot.fire(() => {
-                    this._removeShooterForSender(invocation.get_sender());
-                });
-            } else {
+    _onScreenshotComplete(area, stream, file, flash, invocation) {
+        if (flash) {
+            let flashspot = new Flashspot(area);
+            flashspot.fire(() => {
                 this._removeShooterForSender(invocation.get_sender());
-            }
+            });
+        } else {
+            this._removeShooterForSender(invocation.get_sender());
         }
 
         stream.close(null);
@@ -136,7 +134,7 @@ var ScreenshotService = class {
             clipboard.set_content(St.ClipboardType.CLIPBOARD, 'image/png', bytes);
         }
 
-        let retval = GLib.Variant.new('(bs)', [result, filenameUsed]);
+        let retval = GLib.Variant.new('(bs)', [true, filenameUsed]);
         invocation.return_value(retval);
     }
 
@@ -178,12 +176,13 @@ var ScreenshotService = class {
         screenshot.screenshot_area(x, y, width, height, stream,
             (o, res) => {
                 try {
-                    let [result, area] =
+                    let [success_, area] =
                         screenshot.screenshot_area_finish(res);
                     this._onScreenshotComplete(
-                        result, area, stream, file, flash, invocation);
+                        area, stream, file, flash, invocation);
                 } catch (e) {
-                    invocation.return_gerror(e);
+                    this._removeShooterForSender(invocation.get_sender());
+                    invocation.return_value(new GLib.Variant('(bs)', [false, '']));
                 }
             });
     }
@@ -201,12 +200,13 @@ var ScreenshotService = class {
         screenshot.screenshot_window(includeFrame, includeCursor, stream,
             (o, res) => {
                 try {
-                    let [result, area] =
+                    let [success_, area] =
                         screenshot.screenshot_window_finish(res);
                     this._onScreenshotComplete(
-                        result, area, stream, file, flash, invocation);
+                        area, stream, file, flash, invocation);
                 } catch (e) {
-                    invocation.return_gerror(e);
+                    this._removeShooterForSender(invocation.get_sender());
+                    invocation.return_value(new GLib.Variant('(bs)', [false, '']));
                 }
             });
     }
@@ -224,12 +224,13 @@ var ScreenshotService = class {
         screenshot.screenshot(includeCursor, stream,
             (o, res) => {
                 try {
-                    let [result, area] =
+                    let [success_, area] =
                         screenshot.screenshot_finish(res);
                     this._onScreenshotComplete(
-                        result, area, stream, file, flash, invocation);
+                        area, stream, file, flash, invocation);
                 } catch (e) {
-                    invocation.return_gerror(e);
+                    this._removeShooterForSender(invocation.get_sender());
+                    invocation.return_value(new GLib.Variant('(bs)', [false, '']));
                 }
             });
     }
diff --git a/src/shell-screenshot.c b/src/shell-screenshot.c
index 4cc65bd77..e60d5891b 100644
--- a/src/shell-screenshot.c
+++ b/src/shell-screenshot.c
@@ -87,12 +87,12 @@ write_screenshot_thread (GTask        *result,
                          gpointer      task_data,
                          GCancellable *cancellable)
 {
-  cairo_status_t status;
   ShellScreenshot *screenshot = SHELL_SCREENSHOT (object);
   ShellScreenshotPrivate *priv;
   g_autoptr (GOutputStream) stream = NULL;
   g_autoptr(GdkPixbuf) pixbuf = NULL;
   g_autofree char *creation_time = NULL;
+  GError *error = NULL;
 
   g_assert (screenshot != NULL);
 
@@ -109,15 +109,15 @@ write_screenshot_thread (GTask        *result,
   if (!creation_time)
     creation_time = g_date_time_format (priv->datetime, "%FT%T%z");
 
-  if (gdk_pixbuf_save_to_stream (pixbuf, stream, "png", NULL, NULL,
-                                 "tEXt::Software", "gnome-screenshot",
-                                 "tEXt::Creation Time", creation_time,
-                                 NULL))
-    status = CAIRO_STATUS_SUCCESS;
-  else
-    status = CAIRO_STATUS_WRITE_ERROR;
+  gdk_pixbuf_save_to_stream (pixbuf, stream, "png", NULL, &error,
+                             "tEXt::Software", "gnome-screenshot",
+                             "tEXt::Creation Time", creation_time,
+                             NULL);
 
-  g_task_return_boolean (result, status == CAIRO_STATUS_SUCCESS);
+  if (error)
+    g_task_return_error (result, error);
+  else
+    g_task_return_boolean (result, TRUE);
 }
 
 static void