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

mobile/android/base/ReadingListHelper.java

@@ -38,14 +38,14 @@ public final class ReadingListHelper implements GeckoEventListener, NativeEventL
         this.context = context;
 
         EventDispatcher.getInstance().registerGeckoThreadListener((GeckoEventListener) this,
-            "Reader:Added", "Reader:FaviconRequest");
+            "Reader:AddToList", "Reader:FaviconRequest");
         EventDispatcher.getInstance().registerGeckoThreadListener((NativeEventListener) this,
             "Reader:ListStatusRequest", "Reader:Removed");
     }
 
     public void uninit() {
         EventDispatcher.getInstance().unregisterGeckoThreadListener((GeckoEventListener) this,
-            "Reader:Added", "Reader:FaviconRequest");
+            "Reader:AddToList", "Reader:FaviconRequest");
         EventDispatcher.getInstance().unregisterGeckoThreadListener((NativeEventListener) this,
             "Reader:ListStatusRequest", "Reader:Removed");
     }
@@ -53,8 +53,8 @@ public final class ReadingListHelper implements GeckoEventListener, NativeEventL
     @Override
     public void handleMessage(String event, JSONObject message) {
         switch(event) {
-            case "Reader:Added": {
+            case "Reader:AddToList": {
-                handleReadingListAdded(message);
+                handleAddToList(message);
                 break;
             }
 
@@ -85,7 +85,7 @@ public final class ReadingListHelper implements GeckoEventListener, NativeEventL
      * A page can be added to the ReadingList by long-tap of the page-action
      * icon, or by tapping the readinglist-add icon in the ReaderMode banner.
      */
-    private void handleReadingListAdded(JSONObject message) {
+    private void handleAddToList(JSONObject message) {
         final int result = message.optInt("result", READER_ADD_FAILED);
         if (result != READER_ADD_SUCCESS) {
             if (result == READER_ADD_FAILED) {
@@ -97,7 +97,9 @@ public final class ReadingListHelper implements GeckoEventListener, NativeEventL
         }
 
         final ContentValues values = new ContentValues();
-        values.put(ReadingListItems.URL, message.optString("url"));
+        final String url = message.optString("url");
+
+        values.put(ReadingListItems.URL, url);
         values.put(ReadingListItems.TITLE, message.optString("title"));
         values.put(ReadingListItems.LENGTH, message.optInt("length"));
         values.put(ReadingListItems.EXCERPT, message.optString("excerpt"));
@@ -109,6 +111,8 @@ public final class ReadingListHelper implements GeckoEventListener, NativeEventL
                 showToast(R.string.reading_list_added, Toast.LENGTH_SHORT);
             }
         });
+
+        GeckoAppShell.sendEventToGecko(GeckoEvent.createBroadcastEvent("Reader:Added", url));
     }
 
     /**

mobile/android/base/Tab.java

@@ -513,23 +513,6 @@ public class Tab {
         });
     }
 
-    public void addToReadingList() {
-        if (!mReaderEnabled)
-            return;
-
-        JSONObject json = new JSONObject();
-        try {
-            json.put("tabID", String.valueOf(getId()));
-            json.put("url", getURL());
-        } catch (JSONException e) {
-            Log.e(LOGTAG, "JSON error - failing to add to reading list", e);
-            return;
-        }
-
-        GeckoEvent e = GeckoEvent.createBroadcastEvent("Reader:Add", json.toString());
-        GeckoAppShell.sendEventToGecko(e);
-    }
-
     public void toggleReaderMode() {
         if (AboutPages.isAboutReader(mUrl)) {
             Tabs.getInstance().loadUrl(ReaderModeUtils.getUrlFromAboutReader(mUrl));

mobile/android/base/Tabs.java

@@ -111,8 +111,7 @@ public class Tabs implements GeckoEventListener {
             "Tab:ViewportMetadata",
             "Tab:StreamStart",
             "Tab:StreamStop",
-            "Reader:Click",
+            "Reader:Toggle");
-            "Reader:LongClick");
 
     }
 
@@ -539,10 +538,8 @@ public class Tabs implements GeckoEventListener {
             } else if (event.equals("Tab:StreamStop")) {
                 tab.setRecording(false);
                 notifyListeners(tab, TabEvents.RECORDING_CHANGE);
-            } else if (event.equals("Reader:Click")) {
+            } else if (event.equals("Reader:Toggle")) {
                 tab.toggleReaderMode();
-            } else if (event.equals("Reader:LongClick")) {
-                tab.addToReadingList();
             }
 
         } catch (Exception e) {

mobile/android/chrome/content/Reader.js

@@ -32,17 +32,13 @@ let Reader = {
   pageAction: {
     readerModeCallback: function(tabID) {
       Messaging.sendRequest({
-        type: "Reader:Click",
+        type: "Reader:Toggle",
         tabID: tabID
       });
     },
 
     readerModeActiveCallback: function(tabID) {
-      Messaging.sendRequest({
+      Reader.addTabToReadingList(tabID);
-        type: "Reader:LongClick",
-        tabID: tabID
-      });
-
       UITelemetry.addEvent("save.1", "pageaction", null, "reader");
     },
   },
@@ -83,74 +79,6 @@ let Reader = {
 
   observe: function(aMessage, aTopic, aData) {
     switch(aTopic) {
-      case "Reader:Add": {
-        let args = JSON.parse(aData);
-        if ('fromAboutReader' in args) {
-          // Ignore adds initiated from aboutReader menu banner
-          break;
-        }
-
-        let tabID = null;
-        let url, urlWithoutRef;
-
-        if ('tabID' in args) {
-          tabID = args.tabID;
-
-          let tab = BrowserApp.getTabForId(tabID);
-          let currentURI = tab.browser.currentURI;
-
-          url = currentURI.spec;
-          urlWithoutRef = currentURI.specIgnoringRef;
-        } else if ('url' in args) {
-          let uri = Services.io.newURI(args.url, null, null);
-          url = uri.spec;
-          urlWithoutRef = uri.specIgnoringRef;
-        } else {
-          throw new Error("Reader:Add requires a tabID or an URL as argument");
-        }
-
-        let sendResult = function(result, article) {
-          article = article || {};
-          this.log("Reader:Add success=" + result + ", url=" + url + ", title=" + article.title + ", excerpt=" + article.excerpt);
-
-          Messaging.sendRequest({
-            type: "Reader:Added",
-            result: result,
-            title: truncate(article.title, MAX_TITLE_LENGTH),
-            url: truncate(url, MAX_URI_LENGTH),
-            length: article.length,
-            excerpt: article.excerpt
-          });
-        }.bind(this);
-
-        let handleArticle = function(article) {
-          if (!article) {
-            sendResult(this.READER_ADD_FAILED, null);
-            return;
-          }
-
-          this.storeArticleInCache(article, function(success) {
-            let result = (success ? this.READER_ADD_SUCCESS : this.READER_ADD_FAILED);
-            sendResult(result, article);
-          }.bind(this));
-        }.bind(this);
-
-        this.getArticleFromCache(urlWithoutRef, function (article) {
-          // If the article is already in reading list, bail
-          if (article) {
-            sendResult(this.READER_ADD_DUPLICATE, null);
-            return;
-          }
-
-          if (tabID != null) {
-            this.getArticleForTab(tabID, urlWithoutRef, handleArticle);
-          } else {
-            this.parseDocumentFromURL(urlWithoutRef, handleArticle);
-          }
-        }.bind(this));
-        break;
-      }
-
       case "Reader:Remove": {
         let args = JSON.parse(aData);
 
@@ -179,6 +107,52 @@ let Reader = {
     }
   },
 
+  addTabToReadingList: function(tabID) {
+    let tab = BrowserApp.getTabForId(tabID);
+    let currentURI = tab.browser.currentURI;
+    let url = currentURI.spec;
+    let urlWithoutRef = currentURI.specIgnoringRef;
+
+    let sendResult = function(result, article) {
+      article = article || {};
+
+      Messaging.sendRequest({
+        type: "Reader:AddToList",
+        result: result,
+        title: truncate(article.title, MAX_TITLE_LENGTH),
+        url: truncate(url, MAX_URI_LENGTH),
+        length: article.length,
+        excerpt: article.excerpt
+      });
+    }.bind(this);
+
+    let handleArticle = function(article) {
+      if (!article) {
+        sendResult(this.READER_ADD_FAILED, null);
+        return;
+      }
+
+      this.storeArticleInCache(article, function(success) {
+        let result = (success ? this.READER_ADD_SUCCESS : this.READER_ADD_FAILED);
+        sendResult(result, article);
+      }.bind(this));
+    }.bind(this);
+
+    this.getArticleFromCache(urlWithoutRef, function (article) {
+      // If the article is already in reading list, bail
+      if (article) {
+        sendResult(this.READER_ADD_DUPLICATE, null);
+        return;
+      }
+
+      if (tabID != null) {
+        this.getArticleForTab(tabID, urlWithoutRef, handleArticle);
+      } else {
+        this.parseDocumentFromURL(urlWithoutRef, handleArticle);
+      }
+    }.bind(this));
+  },
+
   getStateForParseOnLoad: function Reader_getStateForParseOnLoad() {
     let isEnabled = Services.prefs.getBoolPref("reader.parse-on-load.enabled");
     let isForceEnabled = Services.prefs.getBoolPref("reader.parse-on-load.force-enabled");
@@ -352,7 +326,6 @@ let Reader = {
   uninit: function Reader_uninit() {
     Services.prefs.removeObserver("reader.parse-on-load.", this);
 
-    Services.obs.removeObserver(this, "Reader:Add");
     Services.obs.removeObserver(this, "Reader:Remove");
 
     let requests = this._requests;

mobile/android/chrome/content/aboutReader.js

@@ -33,7 +33,7 @@ let AboutReader = function(doc, win) {
   this._winRef = Cu.getWeakReference(win);
 
   Services.obs.addObserver(this, "Reader:FaviconReturn", false);
-  Services.obs.addObserver(this, "Reader:Add", false);
+  Services.obs.addObserver(this, "Reader:Added", false);
   Services.obs.addObserver(this, "Reader:Remove", false);
   Services.obs.addObserver(this, "Reader:ListStatusReturn", false);
   Services.obs.addObserver(this, "Gesture:DoubleTap", false);
@@ -190,10 +190,9 @@ AboutReader.prototype = {
         break;
       }
 
-      case "Reader:Add": {
+      case "Reader:Added": {
         // Page can be added by long-press pageAction, or by tap on banner icon.
-        let args = JSON.parse(aData);
+        if (aData == this._article.url) {
-        if (args.url == this._article.url) {
           if (this._isReadingListItem != 1) {
             this._isReadingListItem = 1;
             this._updateToggleButton();
@@ -280,7 +279,7 @@ AboutReader.prototype = {
         break;
 
       case "unload":
-        Services.obs.removeObserver(this, "Reader:Add");
+        Services.obs.removeObserver(this, "Reader:Added");
         Services.obs.removeObserver(this, "Reader:Remove");
         Services.obs.removeObserver(this, "Reader:ListStatusReturn");
         Services.obs.removeObserver(this, "Gesture:DoubleTap");
@@ -320,14 +319,9 @@ AboutReader.prototype = {
     if (!this._article)
       return;
 
-    this._isReadingListItem = (this._isReadingListItem == 1) ? 0 : 1;
+    if (this._isReadingListItem == 0) {
-    this._updateToggleButton();
-
-    if (this._isReadingListItem == 1) {
       let uptime = UITelemetry.uptimeMillis();
       gChromeWin.Reader.storeArticleInCache(this._article, function(success) {
-        dump("Reader:Add (in reader) success=" + success);
-
         let result = gChromeWin.Reader.READER_ADD_FAILED;
         if (success) {
           result = gChromeWin.Reader.READER_ADD_SUCCESS;
@@ -335,10 +329,9 @@ AboutReader.prototype = {
         }
 
         let json = JSON.stringify({ fromAboutReader: true, url: this._article.url });
-        Services.obs.notifyObservers(null, "Reader:Add", json);
 
         Messaging.sendRequest({
-          type: "Reader:Added",
+          type: "Reader:AddToList",
           result: result,
           title: this._article.title,
           url: this._article.url,

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:Add", "Reader:Remove"], "chrome://browser/content/Reader.js"],
+  ["Reader", ["Reader:Remove"], "chrome://browser/content/Reader.js"],
 ].forEach(function (aScript) {
   let [name, notifications, script] = aScript;
   XPCOMUtils.defineLazyGetter(window, name, function() {