From 07b38c4d27435e9f2699661ff18b5e02849c183c Mon Sep 17 00:00:00 2001 From: Margaret Leibovic Date: Wed, 22 Oct 2014 09:12:08 -0700 Subject: [PATCH] Bug 1086991 - Reduce JS/Java message passing for reader mode (Reader:Add*). r=rnewman --- mobile/android/base/ReadingListHelper.java | 16 ++- mobile/android/base/Tab.java | 17 --- mobile/android/base/Tabs.java | 7 +- mobile/android/chrome/content/Reader.js | 123 ++++++++----------- mobile/android/chrome/content/aboutReader.js | 19 +-- mobile/android/chrome/content/browser.js | 2 +- 6 files changed, 67 insertions(+), 117 deletions(-) diff --git a/mobile/android/base/ReadingListHelper.java b/mobile/android/base/ReadingListHelper.java index 36dbc0319df7..4c77dfce6ee3 100644 --- a/mobile/android/base/ReadingListHelper.java +++ b/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": { - handleReadingListAdded(message); + case "Reader:AddToList": { + 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)); } /** diff --git a/mobile/android/base/Tab.java b/mobile/android/base/Tab.java index 1eeac241c502..fd862122e07c 100644 --- a/mobile/android/base/Tab.java +++ b/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)); diff --git a/mobile/android/base/Tabs.java b/mobile/android/base/Tabs.java index 1b059c5a973c..fbac0b21200e 100644 --- a/mobile/android/base/Tabs.java +++ b/mobile/android/base/Tabs.java @@ -111,8 +111,7 @@ public class Tabs implements GeckoEventListener { "Tab:ViewportMetadata", "Tab:StreamStart", "Tab:StreamStop", - "Reader:Click", - "Reader:LongClick"); + "Reader:Toggle"); } @@ -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) { diff --git a/mobile/android/chrome/content/Reader.js b/mobile/android/chrome/content/Reader.js index 3ae5a4d4feba..d70e21455a6b 100644 --- a/mobile/android/chrome/content/Reader.js +++ b/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({ - type: "Reader:LongClick", - tabID: tabID - }); - + Reader.addTabToReadingList(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; diff --git a/mobile/android/chrome/content/aboutReader.js b/mobile/android/chrome/content/aboutReader.js index c675f7d5442e..722dae682c67 100644 --- a/mobile/android/chrome/content/aboutReader.js +++ b/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 (args.url == this._article.url) { + if (aData == 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; - this._updateToggleButton(); - - if (this._isReadingListItem == 1) { + if (this._isReadingListItem == 0) { 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, diff --git a/mobile/android/chrome/content/browser.js b/mobile/android/chrome/content/browser.js index 21056a94a7c8..0ddda250e9a1 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: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() {