[geary/wip/jsc-migration: 2/5] Port JavaScript code from JS to JSC APIs



commit 6f1834d696ddb37679f3206a84a8ebdacb918009
Author: Michael Gratton <mike vee net>
Date:   Sun Jul 21 09:01:58 2019 +1000

    Port JavaScript code from JS to JSC APIs
    
    JSC is WebKitGTK's supported API going forward and is more
    straight-forward than the old JS API, so this simplifies the code and
    removes a number of deprecated calls.

 src/client/components/client-web-view.vala         |   2 +-
 .../conversation-viewer/conversation-web-view.vala |  48 ++++---
 src/client/util/util-webkit.vala                   |  59 ++++-----
 src/client/web-process/web-process-extension.vala  |  62 ++++-----
 src/engine/util/util-js.vala                       | 143 +++++++--------------
 test/js/composer-page-state-test.vala              |   2 +-
 test/js/conversation-page-state-test.vala          |   2 +-
 7 files changed, 129 insertions(+), 189 deletions(-)
---
diff --git a/src/client/components/client-web-view.vala b/src/client/components/client-web-view.vala
index b47ead01..b6931596 100644
--- a/src/client/components/client-web-view.vala
+++ b/src/client/components/client-web-view.vala
@@ -617,7 +617,7 @@ public abstract class ClientWebView : WebKit.WebView, Geary.BaseInterface {
     private void on_preferred_height_changed(WebKit.JavascriptResult result) {
         double height = this.webkit_reported_height;
         try {
-            height = Util.WebKit.to_number(result);
+            height = Util.WebKit.to_double(result);
         } catch (Geary.JS.Error err) {
             debug("Could not get preferred height: %s", err.message);
         }
diff --git a/src/client/conversation-viewer/conversation-web-view.vala 
b/src/client/conversation-viewer/conversation-web-view.vala
index 6f456278..d0ad7549 100644
--- a/src/client/conversation-viewer/conversation-web-view.vala
+++ b/src/client/conversation-viewer/conversation-web-view.vala
@@ -97,7 +97,7 @@ public class ConversationWebView : ClientWebView {
             Geary.JS.callable("geary.getAnchorTargetY")
             .string(anchor_body), null
         );
-        return (int) Util.WebKit.to_number(result);
+        return (int) Util.WebKit.to_int32(result);
     }
 
     /**
@@ -198,38 +198,34 @@ public class ConversationWebView : ClientWebView {
 
     private void on_deceptive_link_clicked(WebKit.JavascriptResult result) {
         try {
-            unowned JS.GlobalContext context = result.get_global_context();
-            JS.Object details = Util.WebKit.to_object(result);
-
-            uint reason = (uint) Geary.JS.to_number(
-                context,
-                Geary.JS.get_property(context, details, "reason"));
+            JSC.Value object = result.get_js_value();
+            uint reason = (uint) Geary.JS.to_int32(
+                Geary.JS.get_property(object, "reason")
+            );
 
             string href = Geary.JS.to_string(
-                context,
-                Geary.JS.get_property(context, details, "href"));
+                Geary.JS.get_property(object, "href")
+            );
 
             string text = Geary.JS.to_string(
-                context,
-                Geary.JS.get_property(context, details, "text"));
+                Geary.JS.get_property(object, "text")
+            );
 
-            JS.Object js_location = Geary.JS.to_object(
-                context,
-                Geary.JS.get_property(context, details, "location"));
+            JSC.Value js_location = Geary.JS.get_property(object, "location");
 
             Gdk.Rectangle location = Gdk.Rectangle();
-            location.x = (int) Geary.JS.to_number(
-                context,
-                Geary.JS.get_property(context, js_location, "x"));
-            location.y = (int) Geary.JS.to_number(
-                context,
-                Geary.JS.get_property(context, js_location, "y"));
-            location.width = (int) Geary.JS.to_number(
-                context,
-                Geary.JS.get_property(context, js_location, "width"));
-            location.height = (int) Geary.JS.to_number(
-                context,
-                Geary.JS.get_property(context, js_location, "height"));
+            location.x = Geary.JS.to_int32(
+                Geary.JS.get_property(js_location, "x")
+            );
+            location.y = Geary.JS.to_int32(
+                Geary.JS.get_property(js_location, "y")
+            );
+            location.width = Geary.JS.to_int32(
+                Geary.JS.get_property(js_location, "width")
+            );
+            location.height = Geary.JS.to_int32(
+                Geary.JS.get_property(js_location, "height")
+            );
 
             deceptive_link_clicked((DeceptiveText) reason, text, href, location);
         } catch (Geary.JS.Error err) {
diff --git a/src/client/util/util-webkit.vala b/src/client/util/util-webkit.vala
index 718048b5..89074a61 100644
--- a/src/client/util/util-webkit.vala
+++ b/src/client/util/util-webkit.vala
@@ -11,75 +11,62 @@
 namespace Util.WebKit {
 
     /**
-     * Returns a WebKit {@link WebKit.JavascriptResult} as a `bool`.
+     * Returns a WebKit JavascriptResult as a `bool`.
      *
      * This will raise a {@link Geary.JS.Error.TYPE} error if the
      * result is not a JavaScript `Boolean`.
      */
     public bool to_bool(global::WebKit.JavascriptResult result)
         throws Geary.JS.Error {
-        unowned JS.GlobalContext context = result.get_global_context();
-        unowned JS.Value value = result.get_value();
-        if (!value.is_boolean(context)) {
-            throw new Geary.JS.Error.TYPE("Result is not a JS Boolean object");
-        }
-        return value.to_boolean(context);
+        return Geary.JS.to_bool(result.get_js_value());
     }
 
     /**
-     * Returns a WebKit {@link WebKit.JavascriptResult} as a `double`.
+     * Returns a WebKit JavascriptResult as a `double`.
      *
      * This will raise a {@link Geary.JS.Error.TYPE} error if the
      * result is not a JavaScript `Number`.
      */
-    public inline double to_number(global::WebKit.JavascriptResult result)
+    public inline double to_int32(global::WebKit.JavascriptResult result)
         throws Geary.JS.Error {
-        return Geary.JS.to_number(result.get_global_context(),
-                                  result.get_value());
+        return Geary.JS.to_double(result.get_js_value());
     }
 
     /**
-     * Returns a WebKit {@link WebKit.JavascriptResult} as a Vala {@link string}.
+     * Returns a WebKit JavascriptResult as a `int32`.
      *
      * This will raise a {@link Geary.JS.Error.TYPE} error if the
-     * result is not a JavaScript `String`.
+     * result is not a JavaScript `Number`.
      */
-    public inline string to_string(global::WebKit.JavascriptResult result)
+    public inline int32 to_double(global::WebKit.JavascriptResult result)
         throws Geary.JS.Error {
-        return Geary.JS.to_string(result.get_global_context(),
-                                  result.get_value());
+        return Geary.JS.to_int32(result.get_js_value());
     }
 
     /**
-     * Converts a WebKit {@link WebKit.JavascriptResult} to a {@link string}.
+     * Returns a WebKit JavascriptResult as a GLib string.
      *
-     * Unlike the other `get_foo_result` methods, this will coax the
-     * result to a string, effectively by calling the JavaScript
-     * `toString()` method on it, and returning that value.
+     * This will raise a {@link Geary.JS.Error.TYPE} error if the
+     * result is not a JavaScript `String`.
      */
-    public string as_string(global::WebKit.JavascriptResult result)
+    public inline string to_string(global::WebKit.JavascriptResult result)
         throws Geary.JS.Error {
-        unowned JS.GlobalContext context = result.get_global_context();
-        unowned JS.Value js_str_value = result.get_value();
-        JS.Value? err = null;
-        JS.String js_str = js_str_value.to_string_copy(context, out err);
-        Geary.JS.check_exception(context, err);
-        return Geary.JS.to_native_string(js_str);
+        return Geary.JS.to_string(result.get_js_value());
     }
 
     /**
-     * Returns a WebKit {@link WebKit.JavascriptResult} as an Object.
-     *
-     * This will raise a {@link Geary.JS.Error.TYPE} error if the
-     * result is not a JavaScript `Object`.
+     * Converts a WebKit JavascriptResult to a GLib string.
      *
-     * Return type is nullable as a workaround for Bug 778046, it will
-     * never actually be null.
+     * Unlike the other `get_foo_result` methods, this will coax the
+     * result to a string, effectively by calling the JavaScript
+     * `toString()` method on it, and returning that value.
      */
-    public JS.Object? to_object(global::WebKit.JavascriptResult result)
+    public string as_string(global::WebKit.JavascriptResult result)
         throws Geary.JS.Error {
-        return Geary.JS.to_object(result.get_global_context(),
-                                  result.get_value());
+        JSC.Value value = result.get_js_value();
+        string str = value.to_string();
+        Geary.JS.check_exception(value.context);
+        return str;
     }
 
 }
diff --git a/src/client/web-process/web-process-extension.vala 
b/src/client/web-process/web-process-extension.vala
index ac7723d7..024967be 100644
--- a/src/client/web-process/web-process-extension.vala
+++ b/src/client/web-process/web-process-extension.vala
@@ -86,15 +86,17 @@ public class GearyWebExtension : Object {
     private bool should_load_remote_images(WebKit.WebPage page) {
         bool should_load = false;
         WebKit.Frame frame = page.get_main_frame();
-        // Explicit cast fixes build on s390x/ppc64. Bug 783882
-        unowned JS.GlobalContext context = (JS.GlobalContext)
-            frame.get_javascript_global_context();
+        JSC.Context context = frame.get_js_context();
         try {
-            unowned JS.Value ret = execute_script(
-                context, "geary.allowRemoteImages", int.parse("__LINE__")
+            JSC.Value ret = execute_script(
+                context,
+                "geary.allowRemoteImages",
+                GLib.Log.FILE,
+                GLib.Log.METHOD,
+                GLib.Log.LINE
             );
-            should_load = ret.to_boolean(context);
-        } catch (Error err) {
+            should_load = Geary.JS.to_bool(ret);
+        } catch (GLib.Error err) {
             debug(
                 "Error checking PageState::allowRemoteImages: %s",
                 err.message
@@ -105,12 +107,14 @@ public class GearyWebExtension : Object {
 
     private void remote_image_load_blocked(WebKit.WebPage page) {
         WebKit.Frame frame = page.get_main_frame();
-        // Explicit cast fixes build on s390x/ppc64. Bug 783882
-        unowned JS.GlobalContext context = (JS.GlobalContext)
-            frame.get_javascript_global_context();
+        JSC.Context context = frame.get_js_context();
         try {
             execute_script(
-                context, "geary.remoteImageLoadBlocked();", int.parse("__LINE__")
+                context,
+                "geary.remoteImageLoadBlocked();",
+                GLib.Log.FILE,
+                GLib.Log.METHOD,
+                GLib.Log.LINE
             );
         } catch (Error err) {
             debug(
@@ -122,33 +126,31 @@ public class GearyWebExtension : Object {
 
     private void selection_changed(WebKit.WebPage page) {
         WebKit.Frame frame = page.get_main_frame();
-        // Explicit cast fixes build on s390x/ppc64. Bug 783882
-        unowned JS.GlobalContext context = (JS.GlobalContext)
-            frame.get_javascript_global_context();
+        JSC.Context context = frame.get_js_context();
         try {
             execute_script(
-                context, "geary.selectionChanged();", int.parse("__LINE__")
+                context,
+                "geary.selectionChanged();",
+                GLib.Log.FILE,
+                GLib.Log.METHOD,
+                GLib.Log.LINE
             );
         } catch (Error err) {
             debug("Error calling PageStates::selectionChanged: %s", err.message);
         }
     }
 
-    // Return type is nullable as a workaround for Bug 778046, it will
-    // never actually be null.
-    private unowned JS.Value? execute_script(JS.Context context, string script, int line)
-    throws Geary.JS.Error {
-        JS.String js_script = new JS.String.create_with_utf8_cstring(script);
-        JS.String js_source = new JS.String.create_with_utf8_cstring("__FILE__");
-        JS.Value? err = null;
-        try {
-            unowned JS.Value ret = context.evaluate_script(
-                js_script, null, js_source, line, out err
-            );
-            Geary.JS.check_exception(context, err);
-            return ret;
-        } finally {
-        }
+    private JSC.Value execute_script(JSC.Context context,
+                                     string script,
+                                     string file_name,
+                                     string method_name,
+                                     int line_number)
+        throws Geary.JS.Error {
+        JSC.Value ret = context.evaluate_with_source_uri(
+            script, -1, "geary:%s/%s".printf(file_name, method_name), line_number
+        );
+        Geary.JS.check_exception(context);
+        return ret;
     }
 
 }
diff --git a/src/engine/util/util-js.vala b/src/engine/util/util-js.vala
index 21b847ae..7011431b 100644
--- a/src/engine/util/util-js.vala
+++ b/src/engine/util/util-js.vala
@@ -10,16 +10,6 @@
  */
 namespace Geary.JS {
 
-#if !VALA_0_42
-    // Workaround broken version of this in the vala bindings. See Bug
-    // 788113.
-    [CCode (cname = "JSStringGetUTF8CString")]
-    private extern size_t js_string_get_utf8_cstring(
-        global::JS.String js,
-        [CCode (array_length_type = "gsize")] char[] buffer
-    );
-#endif
-
     /**
      * Errors produced by functions in {@link Geary.JS}.
      */
@@ -36,138 +26,103 @@ namespace Geary.JS {
     }
 
     /**
-     * Determines if a {@link JS.Value} object is {{{null}}}.
+     * Returns a JSC Value as a bool.
      *
-     * @return `true` if `js` is `null` or has a {@link JS.Type} of
-     * `NULL` according to `context`.
+     * This will raise a {@link Geary.JS.Error.TYPE} error if the
+     * value is not a JavaScript `Boolean`.
      */
-    public inline bool is_null(global::JS.Context context,
-                               global::JS.Value? js) {
-        return (js == null || js.get_type(context) == global::JS.Type.NULL);
+    public bool to_bool(JSC.Value value)
+        throws Geary.JS.Error {
+        if (!value.is_boolean()) {
+            throw new Geary.JS.Error.TYPE("Value is not a JS Boolean object");
+        }
+        bool boolean = value.to_boolean();
+        Geary.JS.check_exception(value.context);
+        return boolean;
     }
 
     /**
-     * Returns a JSC Value as a number.
+     * Returns a JSC Value as a double.
      *
      * This will raise a {@link Geary.JS.Error.TYPE} error if the
      * value is not a JavaScript `Number`.
      */
-    public double to_number(global::JS.Context context,
-                            global::JS.Value value)
+    public double to_double(JSC.Value value)
         throws Geary.JS.Error {
-        if (!value.is_number(context)) {
+        if (!value.is_number()) {
             throw new Geary.JS.Error.TYPE("Value is not a JS Number object");
         }
 
-        global::JS.Value? err = null;
-        double number = value.to_number(context, out err);
-        Geary.JS.check_exception(context, err);
+        double number = value.to_double();
+        Geary.JS.check_exception(value.context);
         return number;
     }
 
     /**
-     * Returns a JSC Value as a string.
+     * Returns a JSC Value as an int32.
      *
      * This will raise a {@link Geary.JS.Error.TYPE} error if the
-     * value is not a JavaScript `String`.
+     * value is not a JavaScript `Number`.
      */
-    public string to_string(global::JS.Context context,
-                            global::JS.Value value)
+    public int32 to_int32(JSC.Value value)
         throws Geary.JS.Error {
-        if (!value.is_string(context)) {
-            throw new Geary.JS.Error.TYPE("Value is not a JS String object");
+        if (!value.is_number()) {
+            throw new Geary.JS.Error.TYPE("Value is not a JS Number object");
         }
 
-        global::JS.Value? err = null;
-        global::JS.String js_str = value.to_string_copy(context, out err);
-        Geary.JS.check_exception(context, err);
-
-        return to_native_string(js_str);
+        int32 number = value.to_int32();
+        Geary.JS.check_exception(value.context);
+        return number;
     }
 
     /**
-     * Returns a JSC Value as an object.
+     * Returns a JSC Value as a string.
      *
      * This will raise a {@link Geary.JS.Error.TYPE} error if the
-     * value is not a JavaScript `Object`.
-     *
-     * Return type is nullable as a workaround for Bug 778046, it will
-     * never actually be null.
+     * value is not a JavaScript `String`.
      */
-    public global::JS.Object? to_object(global::JS.Context context,
-                                       global::JS.Value value)
+    public string to_string(JSC.Value value)
         throws Geary.JS.Error {
-        if (!value.is_object(context)) {
-            throw new Geary.JS.Error.TYPE("Value is not a JS Object");
+        if (!value.is_string()) {
+            throw new Geary.JS.Error.TYPE("Value is not a JS String object");
         }
 
-        global::JS.Value? err = null;
-        global::JS.Object js_obj = value.to_object(context, out err);
-        Geary.JS.check_exception(context, err);
-
-        return js_obj;
-    }
-
-    /**
-     * Returns a JSC {@link JS.String} as a Vala {@link string}.
-     */
-    public inline string to_native_string(global::JS.String js) {
-        size_t len = js.get_maximum_utf8_cstring_size();
-        uint8[] str = new uint8[len];
-#if VALA_0_42
-        js.get_utf8_cstring(str);
-#else
-        js_string_get_utf8_cstring(js, (char[]) str);
-#endif
-        return (string) str;
+        string str = value.to_string();
+        Geary.JS.check_exception(value.context);
+        return str;
     }
 
     /**
-     * Returns the value of an object's property.
+     * Returns the value of an object property.
      *
      * This will raise a {@link Geary.JS.Error.TYPE} error if the
-     * object does not contain the named property.
-     *
-     * Return type is nullable as a workaround for Bug 778046, it will
-     * never actually be null.
+     * value is not an object or does not contain the named property.
      */
-    public inline global::JS.Value? get_property(global::JS.Context context,
-                                                global::JS.Object object,
-                                                string name)
+    public inline JSC.Value get_property(JSC.Value value,
+                                         string name)
         throws Geary.JS.Error {
-        global::JS.String js_name = new global::JS.String.create_with_utf8_cstring(name);
-        global::JS.Value? err = null;
-        global::JS.Value prop = object.get_property(context, js_name, out err);
-        Geary.JS.check_exception(context, err);
+        if (!value.is_object()) {
+            throw new Geary.JS.Error.TYPE("Value is not a JS Object");
+        }
 
-        return prop;
+        JSC.Value property = value.object_get_property(name);
+        Geary.JS.check_exception(value.context);
+        return property;
     }
 
     /**
      * Checks an JS exception returned from a JSC call.
      *
-     * This method will raise a {@link Geary.JS.Error} if the given
-     * `err_value` is not null (in a Vala or JS sense).
+     * If the given context has a current exception, it will cleared
+     * and a {@link Geary.JS.Error} will be thrown.
      */
-    public inline void check_exception(global::JS.Context context,
-                                       global::JS.Value? err_value)
+    public inline void check_exception(JSC.Context context)
         throws Error {
-        if (!is_null(context, err_value)) {
-            global::JS.Value? nested_err = null;
-            global::JS.Type err_type = err_value.get_type(context);
-            global::JS.String err_str =
-                err_value.to_string_copy(context, out nested_err);
-
-            if (!is_null(context, nested_err)) {
-                throw new Error.EXCEPTION(
-                    "Nested exception getting exception %s as a string",
-                    err_type.to_string()
-                );
-            }
-
+        JSC.Exception? exception = context.get_exception();
+        if (exception != null) {
+            context.clear_exception();
             throw new Error.EXCEPTION(
-                "JS exception thrown [%s]: %s"
-                .printf(err_type.to_string(), to_native_string(err_str))
+                "JS exception thrown: %s", exception.to_string()
             );
         }
     }
diff --git a/test/js/composer-page-state-test.vala b/test/js/composer-page-state-test.vala
index ec88f8ff..bdcbcb1b 100644
--- a/test/js/composer-page-state-test.vala
+++ b/test/js/composer-page-state-test.vala
@@ -70,7 +70,7 @@ class ComposerPageStateTest : ClientWebViewTestCase<ComposerWebView> {
         try {
             run_javascript(@"SelectionUtil.selectNode(document.getElementById('test'))");
             run_javascript(@"geary.indentLine()");
-            
assert(Util.WebKit.to_number(run_javascript(@"document.querySelectorAll('blockquote[type=cite]').length")) == 
1);
+            
assert(Util.WebKit.to_int32(run_javascript(@"document.querySelectorAll('blockquote[type=cite]').length")) == 
1);
             
assert(Util.WebKit.to_string(run_javascript(@"document.querySelectorAll('blockquote[type=cite]').item(0).innerText"))
 ==
                 "some text");
         } catch (Geary.JS.Error err) {
diff --git a/test/js/conversation-page-state-test.vala b/test/js/conversation-page-state-test.vala
index 84131535..b56dc8fe 100644
--- a/test/js/conversation-page-state-test.vala
+++ b/test/js/conversation-page-state-test.vala
@@ -159,7 +159,7 @@ class ConversationPageStateTest : ClientWebViewTestCase<ConversationWebView> {
 
     private uint exec_is_deceptive_text(string text, string href) {
         try {
-            return (uint) Util.WebKit.to_number(
+            return (uint) Util.WebKit.to_int32(
                 run_javascript(@"ConversationPageState.isDeceptiveText(\"$text\", \"$href\")")
             );
         } catch (Geary.JS.Error err) {


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]