Bug 1086991 - Reduce JS/Java message passing for reader mode (Reader:Remove*). r=rnewman

mobile/android/base/ReadingListHelper.java

@@ -40,14 +40,14 @@ public final class ReadingListHelper implements GeckoEventListener, NativeEventL
         EventDispatcher.getInstance().registerGeckoThreadListener((GeckoEventListener) this,
             "Reader:AddToList", "Reader:FaviconRequest");
         EventDispatcher.getInstance().registerGeckoThreadListener((NativeEventListener) this,
-            "Reader:ListStatusRequest", "Reader:Removed");
+            "Reader:ListStatusRequest", "Reader:RemoveFromList");
     }
 
     public void uninit() {
         EventDispatcher.getInstance().unregisterGeckoThreadListener((GeckoEventListener) this,
             "Reader:AddToList", "Reader:FaviconRequest");
         EventDispatcher.getInstance().unregisterGeckoThreadListener((NativeEventListener) this,
-            "Reader:ListStatusRequest", "Reader:Removed");
+            "Reader:ListStatusRequest", "Reader:RemoveFromList");
     }
 
     @Override
@@ -69,8 +69,8 @@ public final class ReadingListHelper implements GeckoEventListener, NativeEventL
     public void handleMessage(final String event, final NativeJSObject message,
                               final EventCallback callback) {
         switch(event) {
-            case "Reader:Removed": {
-                handleReadingListRemoved(message.getString("url"));
+            case "Reader:RemoveFromList": {
+                handleRemoveFromList(message.getString("url"));
                 break;
             }
 
@@ -149,11 +149,12 @@ public final class ReadingListHelper implements GeckoEventListener, NativeEventL
      * A page can be removed from the ReadingList by panel context menu,
      * or by tapping the readinglist-remove icon in the ReaderMode banner.
      */
-    private void handleReadingListRemoved(final String url) {
+    private void handleRemoveFromList(final String url) {
         ThreadUtils.postToBackgroundThread(new Runnable() {
             @Override
             public void run() {
                 BrowserDB.removeReadingListItemWithURL(context.getContentResolver(), url);
+                GeckoAppShell.sendEventToGecko(GeckoEvent.createBroadcastEvent("Reader:Removed", url));
                 showToast(R.string.page_removed, Toast.LENGTH_SHORT);
             }
         });

mobile/android/base/home/HomeFragment.java

@@ -362,17 +362,7 @@ public abstract class HomeFragment extends Fragment {
 
                 case READING_LIST:
                     BrowserDB.removeReadingListItemWithURL(cr, mUrl);
-
-                    final JSONObject json = new JSONObject();
-                    try {
-                        json.put("url", mUrl);
-                        json.put("notify", false);
-                    } catch (JSONException e) {
-                        Log.e(LOGTAG, "error building JSON arguments");
-                    }
-
-                    GeckoEvent e = GeckoEvent.createBroadcastEvent("Reader:Remove", json.toString());
-                    GeckoAppShell.sendEventToGecko(e);
+                    GeckoAppShell.sendEventToGecko(GeckoEvent.createBroadcastEvent("Reader:Removed", mUrl));
                     break;
 
                 default:

mobile/android/chrome/content/Reader.js

@@ -79,22 +79,8 @@ let Reader = {
 
   observe: function(aMessage, aTopic, aData) {
     switch(aTopic) {
-      case "Reader:Remove": {
-        let args = JSON.parse(aData);
-
-        if (!("url" in args)) {
-          throw new Error("Reader:Remove requires URL as an argument");
-        }
-
-        this.removeArticleFromCache(args.url, function(success) {
-          this.log("Reader:Remove success=" + success + ", url=" + args.url);
-          if (success && args.notify) {
-            Messaging.sendRequest({
-              type: "Reader:Removed",
-              url: args.url
-            });
-          }
-        }.bind(this));
+      case "Reader:Removed": {
+        this.removeArticleFromCache(aData);
         break;
       }
 
@@ -299,10 +285,9 @@ let Reader = {
     }.bind(this));
   },
 
-  removeArticleFromCache: function Reader_removeArticleFromCache(url, callback) {
+  removeArticleFromCache: function Reader_removeArticleFromCache(url) {
     this._getCacheDB(function(cacheDB) {
       if (!cacheDB) {
-        callback(false);
         return;
       }
 
@@ -313,12 +298,10 @@ let Reader = {
 
       request.onerror = function(event) {
         this.log("Error removing article from the cache DB: " + url);
-        callback(false);
       }.bind(this);
 
       request.onsuccess = function(event) {
         this.log("Removed article from the cache DB: " + url);
-        callback(true);
       }.bind(this);
     }.bind(this));
   },
@@ -326,7 +309,7 @@ let Reader = {
   uninit: function Reader_uninit() {
     Services.prefs.removeObserver("reader.parse-on-load.", this);
 
-    Services.obs.removeObserver(this, "Reader:Remove");
+    Services.obs.removeObserver(this, "Reader:Removed");
 
     let requests = this._requests;
     for (let url in requests) {

mobile/android/chrome/content/aboutReader.js

@@ -34,7 +34,7 @@ let AboutReader = function(doc, win) {
 
   Services.obs.addObserver(this, "Reader:FaviconReturn", false);
   Services.obs.addObserver(this, "Reader:Added", false);
-  Services.obs.addObserver(this, "Reader:Remove", false);
+  Services.obs.addObserver(this, "Reader:Removed", false);
   Services.obs.addObserver(this, "Reader:ListStatusReturn", false);
   Services.obs.addObserver(this, "Gesture:DoubleTap", false);
 
@@ -200,9 +200,9 @@ AboutReader.prototype = {
         }
         break;
       }
-      case "Reader:Remove": {
-        let args = JSON.parse(aData);
-        if (args.url == this._article.url) {
+
+      case "Reader:Removed": {
+        if (aData == this._article.url) {
           if (this._isReadingListItem != 0) {
             this._isReadingListItem = 0;
             this._updateToggleButton();
@@ -280,7 +280,7 @@ AboutReader.prototype = {
 
       case "unload":
         Services.obs.removeObserver(this, "Reader:Added");
-        Services.obs.removeObserver(this, "Reader:Remove");
+        Services.obs.removeObserver(this, "Reader:Removed");
         Services.obs.removeObserver(this, "Reader:ListStatusReturn");
         Services.obs.removeObserver(this, "Gesture:DoubleTap");
         break;
@@ -340,11 +340,10 @@ AboutReader.prototype = {
         });
       }.bind(this));
     } else {
-      // In addition to removing the article from the cache (handled in
-      // browser.js), sending this message will cause the toggle button to be
-      // updated (handled in this file).
-      let json = JSON.stringify({ url: this._article.url, notify: true });
-      Services.obs.notifyObservers(null, "Reader:Remove", json);
+      Messaging.sendRequest({
+        type: "Reader:RemoveFromList",
+        url: this._article.url
+      });
 
       UITelemetry.addEvent("unsave.1", "button", null, "reader");
     }

mobile/android/chrome/content/browser.js

@@ -137,7 +137,7 @@ XPCOMUtils.defineLazyModuleGetter(this, "SharedPreferences",
   ["SelectionHandler", ["TextSelection:Get"], "chrome://browser/content/SelectionHandler.js"],
   ["Notifications", ["Notification:Event"], "chrome://browser/content/Notifications.jsm"],
   ["EmbedRT", ["GeckoView:ImportScript"], "chrome://browser/content/EmbedRT.js"],
-  ["Reader", ["Reader:Remove"], "chrome://browser/content/Reader.js"],
+  ["Reader", ["Reader:Removed"], "chrome://browser/content/Reader.js"],
 ].forEach(function (aScript) {
   let [name, notifications, script] = aScript;
   XPCOMUtils.defineLazyGetter(window, name, function() {