From 162cb65a8860f0fa3cb14f020bb0ac811fddb9ec Mon Sep 17 00:00:00 2001 From: Margaret Leibovic Date: Wed, 22 Oct 2014 09:12:11 -0700 Subject: [PATCH] Bug 1086991 - Reduce JS/Java message passing for reader mode (Reader:Remove*). r=rnewman --- mobile/android/base/ReadingListHelper.java | 11 +++++---- mobile/android/base/home/HomeFragment.java | 12 +--------- mobile/android/chrome/content/Reader.js | 25 ++++---------------- mobile/android/chrome/content/aboutReader.js | 19 +++++++-------- mobile/android/chrome/content/browser.js | 2 +- 5 files changed, 21 insertions(+), 48 deletions(-) diff --git a/mobile/android/base/ReadingListHelper.java b/mobile/android/base/ReadingListHelper.java index 4c77dfce6ee3..dbdce915c025 100644 --- a/mobile/android/base/ReadingListHelper.java +++ b/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); } }); diff --git a/mobile/android/base/home/HomeFragment.java b/mobile/android/base/home/HomeFragment.java index 89f7d5b7d494..52e69c7df417 100644 --- a/mobile/android/base/home/HomeFragment.java +++ b/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: diff --git a/mobile/android/chrome/content/Reader.js b/mobile/android/chrome/content/Reader.js index d70e21455a6b..c2d9af12e714 100644 --- a/mobile/android/chrome/content/Reader.js +++ b/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) { diff --git a/mobile/android/chrome/content/aboutReader.js b/mobile/android/chrome/content/aboutReader.js index 722dae682c67..97c95e395e3e 100644 --- a/mobile/android/chrome/content/aboutReader.js +++ b/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"); } diff --git a/mobile/android/chrome/content/browser.js b/mobile/android/chrome/content/browser.js index 0ddda250e9a1..e5301c11649a 100644 --- a/mobile/android/chrome/content/browser.js +++ b/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() {