[geary/bug/728002-webkit2: 85/140] Cleanup code, registration and memory management for JS message handlers.

[Michael Gratton <mjog src gnome org>]

commit 840a76dac3f7974154c4a929700cea55cb4a3464
Author: Michael James Gratton <mike vee net>
Date:   Sat Jan 14 23:00:52 2017 +1100

    Cleanup code, registration and memory management for JS message handlers.
    
    * src/client/components/client-web-view.vala (ClientWebView): Introduce
      new JavaScriptMessageHandler delegate for JS message handler callbacks.
      (ClientWebView::register_message_handler): Add delegate as second param
      and register it as the callback for the handler. Update call sites to
      use this form, do not have them explicitly unref the JavascriptResult
      since WK2 is doing the right thing, and clean them up in general.

 src/client/components/client-web-view.vala |  100 +++++++++++++++-------------
 src/client/composer/composer-web-view.vala |   28 ++------
 2 files changed, 59 insertions(+), 69 deletions(-)
---
diff --git a/src/client/components/client-web-view.vala b/src/client/components/client-web-view.vala
index 583ccfe..2a9c265 100644
--- a/src/client/components/client-web-view.vala
+++ b/src/client/components/client-web-view.vala
@@ -25,10 +25,9 @@ public class ClientWebView : WebKit.WebView {
 
     /** URI Scheme and delimiter for images loaded by Content-ID. */
     public const string CID_URL_PREFIX = "cid:";
-
-    private const string PREFERRED_HEIGHT_MESSAGE = "preferredHeightChanged";
-    private const string REMOTE_IMAGE_LOAD_BLOCKED_MESSAGE = "remoteImageLoadBlocked";
-    private const string SELECTION_CHANGED_MESSAGE = "selectionChanged";
+    private const string PREFERRED_HEIGHT_CHANGED = "preferredHeightChanged";
+    private const string REMOTE_IMAGE_LOAD_BLOCKED = "remoteImageLoadBlocked";
+    private const string SELECTION_CHANGED = "selectionChanged";
 
     private const double ZOOM_DEFAULT = 1.0;
     private const double ZOOM_FACTOR = 0.1;
@@ -146,6 +145,8 @@ public class ClientWebView : WebKit.WebView {
     }
 
 
+    /** Delegate for UserContentManager message callbacks. */
+    public delegate void JavaScriptMessageHandler(WebKit.JavascriptResult js_result);
 
     /** Determines if the view has started rendering the HTML */
     public bool has_valid_height { get; set; default = false; }
@@ -188,10 +189,10 @@ public class ClientWebView : WebKit.WebView {
     private int preferred_height = 0;
 
 
-    /** Emitted when the web view's selection has changed. */
+    /** Emitted when the view's selection has changed. */
     public signal void selection_changed(bool has_selection);
 
-    /** Emitted when a user clicks a link in this web view. */
+    /** Emitted when a user clicks a link in the view. */
     public signal void link_activated(string uri);
 
     /** Emitted when the view has loaded a resource added to it. */
@@ -232,46 +233,15 @@ public class ClientWebView : WebKit.WebView {
                 return Gdk.EVENT_PROPAGATE;
             });
 
-        content_manager.script_message_received[PREFERRED_HEIGHT_MESSAGE].connect(
-            (result) => {
-                try {
-                    this.preferred_height = (int) WebKitUtil.to_number(result);
-                    if (this.preferred_height >= 1) {
-                        if (!this.has_valid_height) {
-                            // Only update the property value if not
-                            // already true
-                            this.has_valid_height = true;
-                        }
-                        queue_resize();
-                    }
-                } catch (Geary.JS.Error err) {
-                    debug("Could not get preferred height: %s", err.message);
-                } finally {
-                    result.unref();
-                }
-            });
-        content_manager.script_message_received[REMOTE_IMAGE_LOAD_BLOCKED_MESSAGE].connect(
-            (result) => {
-                try {
-                    remote_image_load_blocked();
-                } finally {
-                    result.unref();
-                }
-            });
-        content_manager.script_message_received[SELECTION_CHANGED_MESSAGE].connect(
-            (result) => {
-                try {
-                    selection_changed(WebKitUtil.to_bool(result));
-                } catch (Geary.JS.Error err) {
-                    debug("Could not get selection content: %s", err.message);
-                } finally {
-                    result.unref();
-                }
-            });
-
-        register_message_handler(PREFERRED_HEIGHT_MESSAGE);
-        register_message_handler(REMOTE_IMAGE_LOAD_BLOCKED_MESSAGE);
-        register_message_handler(SELECTION_CHANGED_MESSAGE);
+        register_message_handler(
+            PREFERRED_HEIGHT_CHANGED, on_preferred_height_changed
+        );
+        register_message_handler(
+            REMOTE_IMAGE_LOAD_BLOCKED, on_remote_image_load_blocked
+        );
+        register_message_handler(
+            SELECTION_CHANGED, on_selection_changed
+         );
 
         // Manage zoom level
         config.bind(Configuration.CONVERSATION_VIEWER_ZOOM_KEY, this, "zoom_level");
@@ -379,7 +349,15 @@ public class ClientWebView : WebKit.WebView {
         minimum_height = natural_height = 0;
     }
 
-    protected inline void register_message_handler(string name) {
+    /**
+     * Convenience function for registering and connecting JS messages.
+     */
+    protected inline void register_message_handler(string name,
+                                                   JavaScriptMessageHandler handler) {
+        // XXX cant use the delegate directly: b.g.o Bug 604781
+        this.user_content_manager.script_message_received[name].connect(
+            (result) => { handler(result); }
+        );
         if (!get_user_content_manager().register_script_message_handler(name)) {
             debug("Failed to register script message handler: %s", name);
         }
@@ -465,6 +443,34 @@ public class ClientWebView : WebKit.WebView {
         return false;
     }
 
+    private void on_preferred_height_changed(WebKit.JavascriptResult result) {
+        try {
+            this.preferred_height = (int) WebKitUtil.to_number(result);
+            if (this.preferred_height >= 1) {
+                // Avoid firing multiple notifies if the value hasn't
+                // changed
+                if (!this.has_valid_height) {
+                    this.has_valid_height = true;
+                }
+                queue_resize();
+            }
+        } catch (Geary.JS.Error err) {
+            debug("Could not get preferred height: %s", err.message);
+        }
+    }
+
+    private void on_remote_image_load_blocked(WebKit.JavascriptResult result) {
+        remote_image_load_blocked();
+    }
+
+    private void on_selection_changed(WebKit.JavascriptResult result) {
+        try {
+            selection_changed(WebKitUtil.to_bool(result));
+        } catch (Geary.JS.Error err) {
+            debug("Could not get selection content: %s", err.message);
+        }
+    }
+
 }
 
 // XXX this needs to be moved into the libsoup bindings
diff --git a/src/client/composer/composer-web-view.vala b/src/client/composer/composer-web-view.vala
index 75fa47c..4d1bb08 100644
--- a/src/client/composer/composer-web-view.vala
+++ b/src/client/composer/composer-web-view.vala
@@ -129,19 +129,9 @@ public class ComposerWebView : ClientWebView {
         this.user_content_manager.add_script(ComposerWebView.app_script);
         // this.should_insert_text.connect(on_should_insert_text);
 
-        this.user_content_manager.script_message_received[COMMAND_STACK_CHANGED].connect(
-            on_command_stack_changed_message
-        );
-        this.user_content_manager.script_message_received[CURSOR_STYLE_CHANGED].connect(
-            on_cursor_style_changed_message
-        );
-        this.user_content_manager.script_message_received[DOCUMENT_MODIFIED].connect(
-            on_document_modified_message
-        );
-
-        register_message_handler(COMMAND_STACK_CHANGED);
-        register_message_handler(CURSOR_STYLE_CHANGED);
-        register_message_handler(DOCUMENT_MODIFIED);
+        register_message_handler(COMMAND_STACK_CHANGED, on_command_stack_changed);
+        register_message_handler(CURSOR_STYLE_CHANGED, on_cursor_style_changed);
+        register_message_handler(DOCUMENT_MODIFIED, on_document_modified);
     }
 
     /**
@@ -399,18 +389,16 @@ public class ComposerWebView : ClientWebView {
         return ""; // XXX
     }
 
-    private void on_command_stack_changed_message(WebKit.JavascriptResult result) {
+    private void on_command_stack_changed(WebKit.JavascriptResult result) {
         try {
             string[] values = WebKitUtil.to_string(result).split(",");
             command_stack_changed(values[0] == "true", values[1] == "true");
         } catch (Geary.JS.Error err) {
             debug("Could not get command stack state: %s", err.message);
-        } finally {
-            result.unref();
         }
     }
 
-    private void on_cursor_style_changed_message(WebKit.JavascriptResult result) {
+    private void on_cursor_style_changed(WebKit.JavascriptResult result) {
         try {
             string[] values = WebKitUtil.to_string(result).split(",");
             string view_name = values[0].down();
@@ -428,14 +416,10 @@ public class ComposerWebView : ClientWebView {
             cursor_style_changed(font_family, font_size);
         } catch (Geary.JS.Error err) {
             debug("Could not get cursor style: %s", err.message);
-        } finally {
-            result.unref();
         }
     }
 
-    private void on_document_modified_message(WebKit.JavascriptResult result) {
-        result.unref();
-
+    private void on_document_modified(WebKit.JavascriptResult result) {
         // Only modify actually changed to avoid excessive notify
         // signals being fired.
         if (this.is_empty) {