Bug 1254003: Don't call tabs.create callback before the tab is ready. r?gabor
MozReview-Commit-ID: 3qK6GpTmIFQ
--- a/browser/components/extensions/ext-tabs.js
+++ b/browser/components/extensions/ext-tabs.js
@@ -406,64 +406,84 @@ extensions.registerSchemaAPI("tabs", nul
AllWindowEvents.removeListener("TabAttrModified", listener);
AllWindowEvents.removeListener("TabPinned", listener);
AllWindowEvents.removeListener("TabUnpinned", listener);
};
}).api(),
create: function(createProperties) {
return new Promise((resolve, reject) => {
- function createInWindow(window) {
- let url;
-
- if (createProperties.url !== null) {
- url = context.uri.resolve(createProperties.url);
-
- if (!context.checkLoadURL(url, {dontReportErrors: true})) {
- reject({message: `URL not allowed: ${url}`});
- return;
- }
- }
-
- let tab = window.gBrowser.addTab(url || window.BROWSER_NEW_TAB_URL);
-
- let active = true;
- if (createProperties.active !== null) {
- active = createProperties.active;
- }
- if (active) {
- window.gBrowser.selectedTab = tab;
- }
-
- if (createProperties.index !== null) {
- window.gBrowser.moveTabTo(tab, createProperties.index);
- }
-
- if (createProperties.pinned) {
- window.gBrowser.pinTab(tab);
- }
-
- resolve(TabManager.convert(extension, tab));
- }
-
let window = createProperties.windowId !== null ?
WindowManager.getWindow(createProperties.windowId, context) :
WindowManager.topWindow;
if (!window.gBrowser) {
let obs = (finishedWindow, topic, data) => {
if (finishedWindow != window) {
return;
}
Services.obs.removeObserver(obs, "browser-delayed-startup-finished");
- createInWindow(window);
+ resolve(window);
};
Services.obs.addObserver(obs, "browser-delayed-startup-finished", false);
} else {
- createInWindow(window);
+ resolve(window);
+ }
+ }).then(window => {
+ let url;
+
+ if (createProperties.url !== null) {
+ url = context.uri.resolve(createProperties.url);
+
+ if (!context.checkLoadURL(url, {dontReportErrors: true})) {
+ return Promise.reject({message: `Illegal URL: ${url}`});
+ }
+ }
+
+ let tab = window.gBrowser.addTab(url || window.BROWSER_NEW_TAB_URL);
+
+ let active = true;
+ if (createProperties.active !== null) {
+ active = createProperties.active;
+ }
+ if (active) {
+ window.gBrowser.selectedTab = tab;
+ }
+
+ if (createProperties.index !== null) {
+ window.gBrowser.moveTabTo(tab, createProperties.index);
}
+
+ if (createProperties.pinned) {
+ window.gBrowser.pinTab(tab);
+ }
+
+ if (!createProperties.url) {
+ // We can't wait for a location change event for about:newtab,
+ // since it may be pre-rendered, in which case its initial
+ // location change event has already fired.
+ return tab;
+ }
+
+ // Wait for the first location change event, so that operations
+ // like `executeScript` are dispatched to the inner window that
+ // contains the URL we're attempting to load.
+ return new Promise(resolve => {
+ let webProgress = tab.linkedBrowser.webProgress;
+ let listener = {
+ QueryInterface: XPCOMUtils.generateQI([Ci.nsIWebProgressListener, Ci.nsISupportsWeakReference]),
+
+ onLocationChange() {
+ webProgress.removeProgressListener(listener);
+ resolve(tab);
+ },
+ };
+ webProgress.addProgressListener(listener, Ci.nsIWebProgress.NOTIFY_LOCATION);
+ });
+ }).then(tab => {
+ return TabManager.convert(extension, tab);
});
},
remove: function(tabs) {
if (!Array.isArray(tabs)) {
tabs = [tabs];
}
@@ -483,17 +503,17 @@ extensions.registerSchemaAPI("tabs", nul
}
let tabbrowser = tab.ownerDocument.defaultView.gBrowser;
if (updateProperties.url !== null) {
let url = context.uri.resolve(updateProperties.url);
if (!context.checkLoadURL(url, {dontReportErrors: true})) {
- return Promise.reject({message: `URL not allowed: ${url}`});
+ return Promise.reject({message: `Illegal URL: ${url}`});
}
tab.linkedBrowser.loadURI(url);
}
if (updateProperties.active !== null) {
if (updateProperties.active) {
tabbrowser.selectedTab = tab;
--- a/browser/components/extensions/test/browser/browser_ext_pageAction_context.js
+++ b/browser/components/extensions/test/browser/browser_ext_pageAction_context.js
@@ -200,18 +200,16 @@ add_task(function* testTabSwitchContext(
browser.tabs.onUpdated.addListener(function listener(tabId, changed) {
if (tabId == details.id && changed.url == details.url) {
browser.tabs.onUpdated.removeListener(listener);
resolve();
}
});
});
};
- let tabLoadPromise;
-
return [
expect => {
browser.test.log("Initial state. No icon visible.");
expect(null);
},
expect => {
browser.test.log("Show the icon on the first tab, expect default properties.");
browser.pageAction.show(tabs[0]);
@@ -220,26 +218,23 @@ add_task(function* testTabSwitchContext(
expect => {
browser.test.log("Change the icon. Expect default properties excluding the icon.");
browser.pageAction.setIcon({tabId: tabs[0], path: "1.png"});
expect(details[1]);
},
expect => {
browser.test.log("Create a new tab. No icon visible.");
browser.tabs.create({active: true, url: "about:blank?0"}, tab => {
- tabLoadPromise = promiseTabLoad({url: "about:blank?0", id: tab.id});
tabs.push(tab.id);
expect(null);
});
},
expect => {
browser.test.log("Await tab load. No icon visible.");
- tabLoadPromise.then(() => {
- expect(null);
- });
+ expect(null);
},
expect => {
browser.test.log("Change properties. Expect new properties.");
let tabId = tabs[1];
browser.pageAction.show(tabId);
browser.pageAction.setIcon({tabId, path: "2.png"});
browser.pageAction.setPopup({tabId, popup: "2.html"});
browser.pageAction.setTitle({tabId, title: "Title 2"});
--- a/browser/components/extensions/test/browser/browser_ext_tabs_create.js
+++ b/browser/components/extensions/test/browser/browser_ext_tabs_create.js
@@ -96,53 +96,54 @@ add_task(function* () {
return;
}
let test = tests.shift();
let expected = Object.assign({}, DEFAULTS, test.result);
browser.test.log(`Testing tabs.create(${JSON.stringify(test.create)}), expecting ${JSON.stringify(test.result)}`);
- let tabId;
let updatedPromise = new Promise(resolve => {
let onUpdated = (changedTabId, changed) => {
- if (changedTabId === tabId && changed.url) {
+ if (changed.url) {
browser.tabs.onUpdated.removeListener(onUpdated);
- resolve(changed.url);
+ resolve({tabId: changedTabId, url: changed.url});
}
};
browser.tabs.onUpdated.addListener(onUpdated);
});
let createdPromise = new Promise(resolve => {
let onCreated = tab => {
browser.test.assertTrue("id" in tab, `Expected tabs.onCreated callback to receive tab object`);
resolve();
};
browser.tabs.onCreated.addListener(onCreated);
});
+ let tabId;
Promise.all([
browser.tabs.create(test.create),
createdPromise,
]).then(([tab]) => {
tabId = tab.id;
for (let key of Object.keys(expected)) {
if (key === "url") {
// FIXME: This doesn't get updated until later in the load cycle.
continue;
}
browser.test.assertEq(expected[key], tab[key], `Expected value for tab.${key}`);
}
return updatedPromise;
- }).then(url => {
- browser.test.assertEq(expected.url, url, `Expected value for tab.url`);
+ }).then(updated => {
+ browser.test.assertEq(tabId, updated.tabId, `Expected value for tab.id`);
+ browser.test.assertEq(expected.url, updated.url, `Expected value for tab.url`);
return browser.tabs.remove(tabId);
}).then(() => {
return browser.tabs.update(activeTab, {active: true});
}).then(() => {
nextTest();
});
}
--- a/browser/components/extensions/test/browser/browser_ext_tabs_create_invalid_url.js
+++ b/browser/components/extensions/test/browser/browser_ext_tabs_create_invalid_url.js
@@ -8,17 +8,17 @@ function* testTabsCreateInvalidURL(tabsC
"permissions": ["tabs"],
},
background: function() {
browser.test.sendMessage("ready");
browser.test.onMessage.addListener((msg, tabsCreateURL) => {
browser.tabs.create({url: tabsCreateURL}, (tab) => {
browser.test.assertEq(undefined, tab, "on error tab should be undefined");
- browser.test.assertTrue(/URL not allowed/.test(browser.runtime.lastError.message),
+ browser.test.assertTrue(/Illegal URL/.test(browser.runtime.lastError.message),
"runtime.lastError should report the expected error message");
// Remove the opened tab is any.
if (tab) {
browser.tabs.remove(tab.id);
}
browser.test.sendMessage("done");
});
--- a/browser/components/extensions/test/browser/browser_ext_tabs_detectLanguage.js
+++ b/browser/components/extensions/test/browser/browser_ext_tabs_detectLanguage.js
@@ -7,30 +7,17 @@ add_task(function* testDetectLanguage()
manifest: {
"permissions": ["tabs"],
},
background() {
const BASE_PATH = "browser/browser/components/extensions/test/browser";
function loadTab(url) {
- let tabId;
- let awaitUpdated = new Promise(resolve => {
- browser.tabs.onUpdated.addListener(function onUpdated(changedTabId, changed, tab) {
- if (changedTabId === tabId && changed.url) {
- browser.tabs.onUpdated.removeListener(onUpdated);
- resolve(tab);
- }
- });
- });
-
- return browser.tabs.create({url}).then(tab => {
- tabId = tab.id;
- return awaitUpdated;
- });
+ return browser.tabs.create({url});
}
loadTab(`http://example.co.jp/${BASE_PATH}/file_language_ja.html`).then(tab => {
return browser.tabs.detectLanguage(tab.id).then(lang => {
browser.test.assertEq("ja", lang, "Japanese document should be detected as Japanese");
return browser.tabs.remove(tab.id);
});
}).then(() => {
--- a/browser/components/extensions/test/browser/browser_ext_tabs_executeScript.js
+++ b/browser/components/extensions/test/browser/browser_ext_tabs_executeScript.js
@@ -113,16 +113,24 @@ add_task(function* testExecuteScript() {
browser.tabs.executeScript({
code: "location.href;",
frameId: frames[1].frameId,
}).then(result => {
browser.test.assertEq("http://mochi.test:8888/", result, "Result for frameId[1] is correct");
}),
+ browser.tabs.create({url: "http://example.com/"}).then(tab => {
+ return browser.tabs.executeScript(tab.id, {code: "location.href"}).then(result => {
+ browser.test.assertEq("http://example.com/", result, "Script executed correctly in new tab");
+
+ return browser.tabs.remove(tab.id);
+ });
+ }),
+
new Promise(resolve => {
browser.runtime.onMessage.addListener(message => {
browser.test.assertEq("script ran", message, "Expected runtime message");
resolve();
});
}),
]);
}).then(() => {
@@ -130,17 +138,17 @@ add_task(function* testExecuteScript() {
}).catch(e => {
browser.test.fail(`Error: ${e} :: ${e.stack}`);
browser.test.notifyFail("executeScript");
});
}
let extension = ExtensionTestUtils.loadExtension({
manifest: {
- "permissions": ["http://mochi.test/", "webNavigation"],
+ "permissions": ["http://mochi.test/", "http://example.com/", "webNavigation"],
},
background,
files: {
"script.js": function() {
browser.runtime.sendMessage("script ran");
},
--- a/browser/components/extensions/test/browser/browser_ext_tabs_update_url.js
+++ b/browser/components/extensions/test/browser/browser_ext_tabs_update_url.js
@@ -32,17 +32,17 @@ function* testTabsUpdateURL(existentTabU
browser.test.assertTrue(tab, "on success the tab should be defined");
}
};
let onTabsUpdateError = (error) => {
if (!isErrorExpected) {
browser.test.fails(`tabs.update with URL ${tabsUpdateURL} should not be rejected`);
} else {
- browser.test.assertTrue(/^URL not allowed/.test(error.message),
+ browser.test.assertTrue(/^Illegal URL/.test(error.message),
"tabs.update should be rejected with the expected error message");
}
};
let onTabsUpdateDone = () => browser.test.sendMessage("done");
browser.tabs.query({lastFocusedWindow: true}, (tabs) => {
browser.tabs.update(tabs[1].id, {url: tabsUpdateURL})