[geary/mjog/558-webkit-shared-process-redux: 89/102] Components.WebView: Check for pass up exceptions when calling JS code

[Michael Gratton <mjog src gnome org>]

commit c813aa5707acc5226a57dca82449dc709969d05a
Author: Michael Gratton <mike vee net>
Date:   Thu Aug 27 16:18:45 2020 +1000

    Components.WebView: Check for pass up exceptions when calling JS code
    
    Update web extension to check for errors when invoking page state
    methods and pass a message back if found. Check for this, decode and
    throw a vala error in the WebView if found.

 src/client/components/components-web-view.vala    | 56 ++++++++++++++++++---
 src/client/web-process/web-process-extension.vala | 39 ++++++++++++---
 test/js/components-page-state-test.vala           | 60 +++++++++++++++++++++++
 ui/components-web-view.js                         |  4 ++
 4 files changed, 146 insertions(+), 13 deletions(-)
---
diff --git a/src/client/components/components-web-view.vala b/src/client/components/components-web-view.vala
index 368b6a8d7..2b3731702 100644
--- a/src/client/components/components-web-view.vala
+++ b/src/client/components/components-web-view.vala
@@ -26,6 +26,10 @@ public abstract class Components.WebView : WebKit.WebView, Geary.BaseInterface {
     /** URI Scheme and delimiter for images loaded by Content-ID. */
     public const string CID_URL_PREFIX = "cid:";
 
+    // Keep these in sync with GearyWebExtension
+    private const string MESSAGE_RETURN_VALUE_NAME = "__return__";
+    private const string MESSAGE_EXCEPTION_NAME = "__exception__";
+
     // WebKit message handler names
     private const string COMMAND_STACK_CHANGED = "commandStackChanged";
     private const string CONTENT_LOADED = "contentLoaded";
@@ -467,9 +471,7 @@ public abstract class Components.WebView : WebKit.WebView, Geary.BaseInterface {
     protected async void call_void(Util.JS.Callable target,
                                    GLib.Cancellable? cancellable)
         throws GLib.Error {
-        yield send_message_to_page(
-            target.to_message(), cancellable
-        );
+        yield call_impl(target, cancellable);
     }
 
     /**
@@ -488,12 +490,10 @@ public abstract class Components.WebView : WebKit.WebView, Geary.BaseInterface {
     protected async T call_returning<T>(Util.JS.Callable target,
                                         GLib.Cancellable? cancellable)
         throws GLib.Error {
-        WebKit.UserMessage? response = yield send_message_to_page(
-            target.to_message(), cancellable
-        );
+        WebKit.UserMessage? response = yield call_impl(target, cancellable);
         if (response == null) {
             throw new Util.JS.Error.TYPE(
-                "Method call did not return a value: %s", target.to_string()
+                "Method call %s did not return a value", target.to_string()
             );
         }
         GLib.Variant? param = response.parameters;
@@ -612,6 +612,48 @@ public abstract class Components.WebView : WebKit.WebView, Geary.BaseInterface {
                              "monospace-font", SettingsBindFlags.DEFAULT);
     }
 
+    private async WebKit.UserMessage? call_impl(Util.JS.Callable target,
+                                                GLib.Cancellable? cancellable)
+        throws GLib.Error {
+        WebKit.UserMessage? response = yield send_message_to_page(
+            target.to_message(), cancellable
+        );
+        if (response != null) {
+            var response_name = response.name;
+            if (response_name == MESSAGE_EXCEPTION_NAME) {
+                var exception = new GLib.VariantDict(response.parameters);
+                var name = exception.lookup_value("name", GLib.VariantType.STRING) as string;
+                var message = exception.lookup_value("message", GLib.VariantType.STRING) as string;
+                var backtrace = exception.lookup_value("backtrace_string", GLib.VariantType.STRING) as 
string;
+                var source = exception.lookup_value("source_uri", GLib.VariantType.STRING) as string;
+                var line = exception.lookup_value("line_number", GLib.VariantType.UINT32);
+                var column = exception.lookup_value("column_number", GLib.VariantType.UINT32);
+
+                var log_message = "Method call %s raised %s exception at %s:%d:%d: %s".printf(
+                    target.to_string(),
+                    name ?? "unknown",
+                    source ?? "unknown",
+                    (line != null ? (int) line.get_uint32() : -1),
+                    (column != null ? (int) column.get_uint32() : -1),
+                    message ?? "unknown"
+                );
+                debug(log_message);
+                if (backtrace != null) {
+                    debug(backtrace);
+                }
+
+                throw new Util.JS.Error.EXCEPTION(log_message);
+            } else if (response_name != MESSAGE_RETURN_VALUE_NAME) {
+                throw new Util.JS.Error.TYPE(
+                    "Method call %s returned unknown name: %s",
+                    target.to_string(),
+                    response_name
+                );
+            }
+        }
+        return response;
+    }
+
     private void handle_cid_request(WebKit.URISchemeRequest request) {
         if (!handle_internal_response(request)) {
             request.finish_error(new FileError.NOENT("Unknown CID"));
diff --git a/src/client/web-process/web-process-extension.vala 
b/src/client/web-process/web-process-extension.vala
index 86f7f44c3..7aa6dd3ca 100644
--- a/src/client/web-process/web-process-extension.vala
+++ b/src/client/web-process/web-process-extension.vala
@@ -31,6 +31,8 @@ public void webkit_web_extension_initialize_with_user_data(WebKit.WebExtension e
 public class GearyWebExtension : Object {
 
     private const string PAGE_STATE_OBJECT_NAME = "geary";
+
+    // Keep these in sync with Components.WebView
     private const string MESSAGE_RETURN_VALUE_NAME = "__return__";
     private const string MESSAGE_EXCEPTION_NAME = "__exception__";
 
@@ -199,12 +201,37 @@ public class GearyWebExtension : Object {
             // rain hail or shine.
             // https://bugs.webkit.org/show_bug.cgi?id=215880
 
-            message.send_reply(
-                new WebKit.UserMessage(
-                    MESSAGE_RETURN_VALUE_NAME,
-                    Util.JS.value_to_variant(ret)
-                )
-            );
+            JSC.Exception? thrown = context.get_exception();
+            if (thrown != null) {
+                var detail = new GLib.VariantDict();
+                if (thrown.get_message() != null) {
+                    detail.insert_value("name", new GLib.Variant.string(thrown.get_name()));
+                }
+                if (thrown.get_message() != null) {
+                    detail.insert_value("message", new GLib.Variant.string(thrown.get_message()));
+                }
+                if (thrown.get_backtrace_string() != null) {
+                    detail.insert_value("backtrace_string", new 
GLib.Variant.string(thrown.get_backtrace_string()));
+                }
+                if (thrown.get_source_uri() != null) {
+                    detail.insert_value("source_uri", new GLib.Variant.string(thrown.get_source_uri()));
+                }
+                detail.insert_value("line_number", new GLib.Variant.uint32(thrown.get_line_number()));
+                detail.insert_value("column_number", new GLib.Variant.uint32(thrown.get_column_number()));
+                message.send_reply(
+                    new WebKit.UserMessage(
+                        MESSAGE_EXCEPTION_NAME,
+                        detail.end()
+                    )
+                );
+            } else {
+                message.send_reply(
+                    new WebKit.UserMessage(
+                        MESSAGE_RETURN_VALUE_NAME,
+                        Util.JS.value_to_variant(ret)
+                    )
+                );
+            }
         } catch (GLib.Error err) {
             debug("Failed to handle message: %s", err.message);
         }
diff --git a/test/js/components-page-state-test.vala b/test/js/components-page-state-test.vala
index 562c6cda6..bf9524166 100644
--- a/test/js/components-page-state-test.vala
+++ b/test/js/components-page-state-test.vala
@@ -31,7 +31,9 @@ class Components.PageStateTest : WebViewTestCase<WebView> {
         base("Components.PageStateTest");
         add_test("content_loaded", content_loaded);
         add_test("call_void", call_void);
+        add_test("call_void_throws", call_void_throws);
         add_test("call_returning", call_returning);
+        add_test("call_returning_throws", call_returning_throws);
 
         try {
             WebView.load_resources(GLib.File.new_for_path("/tmp"));
@@ -68,6 +70,35 @@ class Components.PageStateTest : WebViewTestCase<WebView> {
         assert_test_result("void");
     }
 
+    public void call_void_throws() throws GLib.Error {
+        load_body_fixture("OHHAI");
+        var test_article = this.test_view as TestWebView;
+
+        try {
+            test_article.call_void.begin(
+                new Util.JS.Callable("testThrow").string("void message"),
+                this.async_completion
+            );
+            test_article.call_void.end(this.async_result());
+            assert_not_reached();
+        } catch (Util.JS.Error.EXCEPTION err) {
+            assert_string(
+                err.message
+            ).contains(
+                "testThrow"
+            // WebKitGTK doesn't actually pass any details through:
+            // https://bugs.webkit.org/show_bug.cgi?id=215877
+            // ).contains(
+            //     "Error"
+            // ).contains(
+            //     "void message"
+            // ).contains(
+            //     "components-web-view.js"
+            );
+            assert_test_result("void message");
+        }
+    }
+
     public void call_returning() throws GLib.Error {
         load_body_fixture("OHHAI");
         var test_article = this.test_view as TestWebView;
@@ -81,6 +112,35 @@ class Components.PageStateTest : WebViewTestCase<WebView> {
         assert_test_result("check 1-2");
     }
 
+    public void call_returning_throws() throws GLib.Error {
+        load_body_fixture("OHHAI");
+        var test_article = this.test_view as TestWebView;
+
+        try {
+            test_article.call_returning.begin(
+                new Util.JS.Callable("testThrow").string("return message"),
+                this.async_completion
+            );
+            test_article.call_returning.end(this.async_result());
+            assert_not_reached();
+        } catch (Util.JS.Error.EXCEPTION err) {
+            assert_string(
+                err.message
+            ).contains(
+                "testThrow"
+            // WebKitGTK doesn't actually pass any details through:
+            // https://bugs.webkit.org/show_bug.cgi?id=215877
+            // ).contains(
+            //     "Error"
+            // ).contains(
+            //     "return message"
+            // ).contains(
+            //     "components-web-view.js"
+            );
+            assert_test_result("return message");
+        }
+    }
+
     protected override WebView set_up_test_view() {
         WebKit.UserScript test_script;
         test_script = new WebKit.UserScript(
diff --git a/ui/components-web-view.js b/ui/components-web-view.js
index 289abca06..0f932a19c 100644
--- a/ui/components-web-view.js
+++ b/ui/components-web-view.js
@@ -194,5 +194,9 @@ PageState.prototype = {
     testReturn: function(value) {
         this.testResult = value;
         return value;
+    },
+    testThrow: function(value) {
+        this.testResult = value;
+        throw this.testResult;
     }
 };