Bug 1386445 - Early about:newtab pages are missing message APIs because RemotePages registers too late. r?Mossop
This moves AboutNewTab.init from nsBrowserGlue.js handling of "browser-delayed-startup-finished" into aboutNewTabService.js so that when the service is loaded once from the main thread probably by browser.js towards the beginning of _delayedStartup just before potentially calling gBrowser.loadTabs, the service triggers the attaching of RemotePages(about:newtab) before any about:newtab pages load.
Additionally even when RemotePages starts early enough, Activity Stream might not borrow the RemotePages instance early enough to catch the RemotePage:Load message, so to simulate that, RemotePages now remembers when a port has been loaded for consumers to check. Adds tests to confirm the expected properties on the port and value of loaded at the various RemotePage:* messages.
MozReview-Commit-ID: IXJLvFCgbEH
--- a/browser/base/content/browser.js
+++ b/browser/base/content/browser.js
@@ -1450,16 +1450,21 @@ var gBrowserInit = {
gBrowser.addEventListener("InsecureLoginFormsStateChange", function() {
gIdentityHandler.refreshForInsecureLoginForms();
});
gBrowser.addEventListener("PermissionStateChange", function() {
gIdentityHandler.refreshIdentityBlock();
});
+ // Get the service so that it initializes and registers listeners for new
+ // tab pages in order to be ready for any early-loading about:newtab pages,
+ // e.g., start/home page, command line / startup uris to load, sessionstore
+ gAboutNewTabService.QueryInterface(Ci.nsISupports);
+
let uriToLoad = this._getUriToLoad();
if (uriToLoad && uriToLoad != "about:blank") {
if (uriToLoad instanceof Ci.nsIArray) {
let count = uriToLoad.length;
let specs = [];
for (let i = 0; i < count; i++) {
let urisstring = uriToLoad.queryElementAt(i, Ci.nsISupportsString);
specs.push(urisstring.data);
--- a/browser/components/newtab/aboutNewTabService.js
+++ b/browser/components/newtab/aboutNewTabService.js
@@ -5,31 +5,37 @@
*/
"use strict";
const {utils: Cu, interfaces: Ci} = Components;
Cu.import("resource://gre/modules/XPCOMUtils.jsm");
Cu.import("resource://gre/modules/Services.jsm");
-XPCOMUtils.defineLazyModuleGetter(this, "Preferences",
- "resource://gre/modules/Preferences.jsm");
+XPCOMUtils.defineLazyModuleGetter(this, "AboutNewTab",
+ "resource:///modules/AboutNewTab.jsm");
const LOCAL_NEWTAB_URL = "chrome://browser/content/newtab/newTab.xhtml";
const ACTIVITY_STREAM_URL = "resource://activity-stream/data/content/activity-stream.html";
const ABOUT_URL = "about:newtab";
+const IS_MAIN_PROCESS = Services.appinfo.processType === Services.appinfo.PROCESS_TYPE_DEFAULT;
+
// Pref that tells if activity stream is enabled
const PREF_ACTIVITY_STREAM_ENABLED = "browser.newtabpage.activity-stream.enabled";
function AboutNewTabService() {
- Preferences.observe(PREF_ACTIVITY_STREAM_ENABLED, this._handleToggleEvent.bind(this));
- this.toggleActivityStream(Services.prefs.getBoolPref(PREF_ACTIVITY_STREAM_ENABLED));
+ Services.obs.addObserver(this, "quit-application-granted");
+ Services.prefs.addObserver(PREF_ACTIVITY_STREAM_ENABLED, this);
+ this.toggleActivityStream();
+ if (IS_MAIN_PROCESS) {
+ AboutNewTab.init();
+ }
}
/*
* A service that allows for the overriding, at runtime, of the newtab page's url.
* Additionally, the service manages pref state between a activity stream, or the regular
* about:newtab page.
*
* There is tight coupling with browser/about/AboutRedirector.cpp.
@@ -63,24 +69,38 @@ function AboutNewTabService() {
*/
AboutNewTabService.prototype = {
_newTabURL: ABOUT_URL,
_activityStreamEnabled: false,
_overridden: false,
classID: Components.ID("{dfcd2adc-7867-4d3a-ba70-17501f208142}"),
- QueryInterface: XPCOMUtils.generateQI([Ci.nsIAboutNewTabService]),
+ QueryInterface: XPCOMUtils.generateQI([
+ Ci.nsIAboutNewTabService,
+ Ci.nsIObserver
+ ]),
_xpcom_categories: [{
service: true
}],
- _handleToggleEvent(stateEnabled) {
- if (this.toggleActivityStream(stateEnabled)) {
- Services.obs.notifyObservers(null, "newtab-url-changed", ABOUT_URL);
+ observe(subject, topic, data) {
+ switch (topic) {
+ case "nsPref:changed":
+ if (this.toggleActivityStream()) {
+ Services.obs.notifyObservers(null, "newtab-url-changed", ABOUT_URL);
+ }
+ break;
+ case "quit-application-granted":
+ Services.obs.removeObserver(this, "quit-application-granted");
+ Services.prefs.removeObserver(PREF_ACTIVITY_STREAM_ENABLED, this);
+ if (IS_MAIN_PROCESS) {
+ AboutNewTab.uninit();
+ }
+ break;
}
},
/**
* React to changes to the activity stream pref.
*
* If browser.newtabpage.activity-stream.enabled is true, this will change the default URL to the
* activity stream page URL. If browser.newtabpage.activity-stream.enabled is false, the default URL
@@ -88,17 +108,18 @@ AboutNewTabService.prototype = {
*
* This will only act if there is a change of state and if not overridden.
*
* @returns {Boolean} Returns if there has been a state change
*
* @param {Boolean} stateEnabled activity stream enabled state to set to
* @param {Boolean} forceState force state change
*/
- toggleActivityStream(stateEnabled, forceState = false) {
+ toggleActivityStream(stateEnabled = Services.prefs.getBoolPref(PREF_ACTIVITY_STREAM_ENABLED),
+ forceState = false) {
if (!forceState && (this.overridden || stateEnabled === this.activityStreamEnabled)) {
// exit there is no change of state
return false;
}
if (stateEnabled) {
this._activityStreamEnabled = true;
} else {
@@ -153,14 +174,14 @@ AboutNewTabService.prototype = {
get activityStreamURL() {
return ACTIVITY_STREAM_URL;
},
resetNewTabURL() {
this._overridden = false;
this._newTabURL = ABOUT_URL;
- this.toggleActivityStream(Services.prefs.getBoolPref(PREF_ACTIVITY_STREAM_ENABLED), true);
+ this.toggleActivityStream(undefined, true);
Services.obs.notifyObservers(null, "newtab-url-changed", this._newTabURL);
}
};
this.NSGetFactory = XPCOMUtils.generateNSGetFactory([AboutNewTabService]);
--- a/browser/components/nsBrowserGlue.js
+++ b/browser/components/nsBrowserGlue.js
@@ -20,17 +20,17 @@ XPCOMUtils.defineLazyGetter(this, "Weave
XPCOMUtils.defineLazyModuleGetter(this, "ContextualIdentityService",
"resource://gre/modules/ContextualIdentityService.jsm");
XPCOMUtils.defineLazyModuleGetter(this, "SafeBrowsing",
"resource://gre/modules/SafeBrowsing.jsm");
// lazy module getters
-/* global AboutHome:false, AboutNewTab:false, AddonManager:false, AppMenuNotifications:false,
+/* global AboutHome:false, AddonManager:false, AppMenuNotifications:false,
AsyncPrefs: false, AsyncShutdown:false, AutoCompletePopup:false, BookmarkHTMLUtils:false,
BookmarkJSONUtils:false, BrowserUITelemetry:false, BrowserUsageTelemetry:false,
ContentClick:false, ContentPrefServiceParent:false, ContentSearch:false,
DateTimePickerHelper:false, DirectoryLinksProvider:false,
ExtensionsUI:false, Feeds:false,
FileUtils:false, FormValidationHandler:false, Integration:false,
LightweightThemeManager:false, LoginHelper:false, LoginManagerParent:false,
NetUtil:false, NewTabUtils:false, OS:false,
@@ -49,17 +49,16 @@ XPCOMUtils.defineLazyModuleGetter(this,
* IF YOU ADD OR REMOVE FROM THIS LIST, PLEASE UPDATE THE LIST ABOVE AS WELL.
* XXX Bug 1325373 is for making eslint detect these automatically.
*/
let initializedModules = {};
[
["AboutHome", "resource:///modules/AboutHome.jsm", "init"],
- ["AboutNewTab", "resource:///modules/AboutNewTab.jsm"],
["AddonManager", "resource://gre/modules/AddonManager.jsm"],
["AppMenuNotifications", "resource://gre/modules/AppMenuNotifications.jsm"],
["AsyncPrefs", "resource://gre/modules/AsyncPrefs.jsm"],
["AsyncShutdown", "resource://gre/modules/AsyncShutdown.jsm"],
["AutoCompletePopup", "resource://gre/modules/AutoCompletePopup.jsm"],
["BookmarkHTMLUtils", "resource://gre/modules/BookmarkHTMLUtils.jsm"],
["BookmarkJSONUtils", "resource://gre/modules/BookmarkJSONUtils.jsm"],
["BrowserUITelemetry", "resource:///modules/BrowserUITelemetry.jsm"],
@@ -964,17 +963,16 @@ BrowserGlue.prototype = {
WeaveService.init();
}
PageThumbs.init();
DirectoryLinksProvider.init();
NewTabUtils.init();
NewTabUtils.links.addProvider(DirectoryLinksProvider);
- AboutNewTab.init();
PageActions.init();
this._firstWindowTelemetry(aWindow);
this._firstWindowLoaded();
this._mediaTelemetryIdleObserver = {
browserGlue: this,
@@ -1025,17 +1023,16 @@ BrowserGlue.prototype = {
if (mod.uninit) {
mod.uninit();
}
}
BrowserUsageTelemetry.uninit();
PageThumbs.uninit();
- AboutNewTab.uninit();
NewTabUtils.uninit();
AutoCompletePopup.uninit();
DateTimePickerHelper.uninit();
},
_initServiceDiscovery() {
if (!Services.prefs.getBoolPref("browser.casting.enabled")) {
return;
--- a/browser/extensions/activity-stream/lib/ActivityStreamMessageChannel.jsm
+++ b/browser/extensions/activity-stream/lib/ActivityStreamMessageChannel.jsm
@@ -128,16 +128,23 @@ this.ActivityStreamMessageChannel = clas
createChannel() {
// Receive AboutNewTab's Remote Pages instance, if it exists, on override
const channel = this.pageURL === ABOUT_NEW_TAB_URL && AboutNewTab.override(true);
this.channel = channel || new RemotePages(this.pageURL);
this.channel.addMessageListener("RemotePage:Init", this.onNewTabInit);
this.channel.addMessageListener("RemotePage:Load", this.onNewTabLoad);
this.channel.addMessageListener("RemotePage:Unload", this.onNewTabUnload);
this.channel.addMessageListener(this.incomingMessageName, this.onMessage);
+
+ // Some pages might have already loaded, so we won't get the usual message
+ for (const {loaded, portID} of this.channel.messagePorts) {
+ if (loaded) {
+ this.onNewTabLoad({target: {portID}});
+ }
+ }
}
/**
* destroyChannel - Destroys the RemotePages channel
*/
destroyChannel() {
this.channel.removeMessageListener("RemotePage:Init", this.onNewTabInit);
this.channel.removeMessageListener("RemotePage:Load", this.onNewTabLoad);
--- a/toolkit/modules/RemotePageManager.jsm
+++ b/toolkit/modules/RemotePageManager.jsm
@@ -88,39 +88,49 @@ RemotePages.prototype = {
this.listener = null;
this.destroyed = true;
},
// Called when a page matching the url has loaded in a frame.
portCreated(port) {
this.messagePorts.add(port);
+ port.loaded = false;
+ port.addMessageListener("RemotePage:Load", this.portMessageReceived);
port.addMessageListener("RemotePage:Unload", this.portMessageReceived);
for (let name of this.listener.keys()) {
this.registerPortListener(port, name);
}
this.listener.callListeners({ target: port, name: "RemotePage:Init" });
},
// A message has been received from one of the pages
portMessageReceived(message) {
- if (message.name == "RemotePage:Unload")
- this.removeMessagePort(message.target);
+ switch (message.name) {
+ case "RemotePage:Load":
+ message.target.loaded = true;
+ break;
+ case "RemotePage:Unload":
+ message.target.loaded = false;
+ this.removeMessagePort(message.target);
+ break;
+ }
this.listener.callListeners(message);
},
// A page has closed
removeMessagePort(port) {
for (let name of this.listener.keys()) {
port.removeMessageListener(name, this.portMessageReceived);
}
+ port.removeMessageListener("RemotePage:Load", this.portMessageReceived);
port.removeMessageListener("RemotePage:Unload", this.portMessageReceived);
this.messagePorts.delete(port);
},
registerPortListener(port, name) {
port.addMessageListener(name, this.portMessageReceived);
},
@@ -172,23 +182,25 @@ function publicMessagePort(port) {
"sendAsyncMessage", "destroy"];
let clean = {};
for (let property of properties) {
clean[property] = port[property].bind(port);
}
Object.defineProperty(clean, "portID", {
+ enumerable: true,
get() {
return port.portID;
}
});
if (port instanceof ChromeMessagePort) {
Object.defineProperty(clean, "browser", {
+ enumerable: true,
get() {
return port.browser;
}
});
}
return clean;
}
--- a/toolkit/modules/tests/browser/browser_RemotePageManager.js
+++ b/toolkit/modules/tests/browser/browser_RemotePageManager.js
@@ -300,16 +300,69 @@ add_task(async function remote_pages_bas
try {
pages.sendAsyncMessage("Foo");
ok(false, "Should have seen exception");
} catch (e) {
ok(true, "Should have seen exception");
}
});
+// Test that properties exist on the target port provided to listeners
+add_task(async function check_port_properties() {
+ let pages = new RemotePages(TEST_URL);
+
+ const expectedProperties = [
+ "addMessageListener",
+ "browser",
+ "destroy",
+ "loaded",
+ "portID",
+ "removeMessageListener",
+ "sendAsyncMessage"
+ ];
+ function checkProperties(port, description) {
+ const expected = [];
+ const unexpected = [];
+ for (const key in port) {
+ (expectedProperties.includes(key) ? expected : unexpected).push(key);
+ }
+ is(`${expected.sort()}`, expectedProperties, `${description} has expected keys`);
+ is(`${unexpected.sort()}`, "", `${description} should not have unexpected keys`);
+ }
+
+ function portFrom(message, extraFn = () => {}) {
+ return new Promise(resolve => {
+ function onMessage({target}) {
+ pages.removeMessageListener(message, onMessage);
+ resolve(target);
+ }
+ pages.addMessageListener(message, onMessage);
+ extraFn();
+ });
+ }
+
+ let portFromInit = await portFrom("RemotePage:Init", () =>
+ (gBrowser.selectedTab = BrowserTestUtils.addTab(gBrowser, TEST_URL)));
+ checkProperties(portFromInit, "inited port");
+ is(portFromInit.loaded, false, "inited port has not been loaded yet");
+
+ let portFromLoad = await portFrom("RemotePage:Load");
+ is(portFromLoad, portFromInit, "got the same port from init and load");
+ checkProperties(portFromLoad, "loaded port");
+ is(portFromInit.loaded, true, "loaded port is now loaded");
+
+ let portFromUnload = await portFrom("RemotePage:Unload", () =>
+ BrowserTestUtils.removeTab(gBrowser.selectedTab));
+ is(portFromUnload, portFromInit, "got the same port from init and unload");
+ checkProperties(portFromUnload, "unloaded port");
+ is(portFromInit.loaded, false, "unloaded port is now not loaded");
+
+ pages.destroy();
+});
+
// Test sending messages to all remote pages works
add_task(async function remote_pages_multiple() {
let pages = new RemotePages(TEST_URL);
let port1 = await waitForPage(pages);
let port2 = await waitForPage(pages);
let pongPorts = [];
await new Promise((resolve) => {
@@ -384,10 +437,12 @@ add_task(async function get_ports_for_br
let pages = new RemotePages(TEST_URL);
let port = await waitForPage(pages);
// waitForPage creates a new tab and selects it by default, so
// the selected tab should be the one hosting this port.
let browser = gBrowser.selectedBrowser;
let foundPorts = pages.portsForBrowser(browser);
is(foundPorts.length, 1, "There should only be one port for this simple page");
is(foundPorts[0], port, "Should find the port");
+
+ pages.destroy();
gBrowser.removeCurrentTab();
});