Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code
Issues
All Issues

Unified Diff: chrome/content/tests/popupBlocker.js

Issue 29331996: Issue 3244 - Pop-up blocker unit tests broken in E10S mode (Closed)
Patch Set: Created Dec. 4, 2015, 8:56 p.m.
Use n/p to move between diff chunks; N/P to move between comments.
Jump to:
View side-by-side diff with in-line comments
Download patch
« chrome/content/common.js ('K') | « chrome/content/common.js ('k') | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: chrome/content/tests/popupBlocker.js
===================================================================
--- a/chrome/content/tests/popupBlocker.js
+++ b/chrome/content/tests/popupBlocker.js
@@ -1,11 +1,12 @@
(function()
{
+ let tabs = SDK.require("sdk/tabs");
+ let modelFor = SDK.require("sdk/model/core").modelFor;
Erik 2015/12/04 20:58:33 oh I should remove this.
let server = null;
- let wnd = null;
let tab = null;
module("Pop-up blocker", {
- setup: function()
+ beforeEach: function()
{
prepareFilterComponents.call(this, true);
preparePrefs.call(this);
@@ -13,22 +14,27 @@
server = new nsHttpServer();
server.start(1234);
+ // '/test' serves an html page with a single link
server.registerPathHandler("/test", function(metadata, response)
{
response.setStatusLine("1.1", "200", "OK");
response.setHeader("Content-Type", "text/html; charset=utf-8");
let body =
- '<body onload="document.dispatchEvent(new CustomEvent(\'abp:frameready\', {bubbles: true}));">' +
+ '<body>' +
'<a id="link" href="/redirect" target="_blank">link</a>' +
'</body>';
response.bodyOutputStream.write(body, body.length);
});
+
+ // redirects '/redirect' to '/target'
server.registerPathHandler("/redirect", function(metadata, response)
{
response.setStatusLine("1.1", "302", "Moved Temporarily");
response.setHeader("Location", "http://127.0.0.1:1234/target");
});
+
+ // '/target' serves an html page with 'OK' message
server.registerPathHandler("/target", function(metadata, response)
{
response.setHeader("Content-Type", "text/html; charset=utf-8");
@@ -37,16 +43,30 @@
response.bodyOutputStream.write(body, body.length);
});
- wnd = UI.currentWindow;
- tab = wnd.gBrowser.loadOneTab("http://127.0.0.1:1234/test", {inBackground: false});
- wnd.gBrowser.getBrowserForTab(tab).addEventListener("abp:frameready", function(event)
- {
- start();
- }, false, true);
+ tabs.open({
+ url: "http://127.0.0.1:1234/test",
+ inBackground: false,
+ onReady: function(aTab)
+ {
+ tab = aTab;
+ var worker = tab.attach({
+ contentScript: "(" + function()
+ {
+ if (document.getElementById("link"))
+ self.port.emit("done");
+ } + ")()"
+ });
Wladimir Palant 2015/12/14 11:28:53 I don't think that clicking the link should be par
+
+ worker.port.once("done", function()
+ {
+ start();
+ })
Wladimir Palant 2015/12/14 11:28:53 Nit: No need to wrap the callback here, this will
+ }
+ });
stop();
},
- teardown: function()
+ afterEach: function()
{
restoreFilterComponents.call(this);
restorePrefs.call(this);
@@ -54,71 +74,109 @@
stop();
server.stop(function()
{
- wnd.gBrowser.removeTab(tab);
-
- server = null;
- frame = null;
-
- start();
+ tab.close(function()
+ {
+ server = null;
+ start();
+ });
});
}
});
let tests = [
- ["||127.0.0.1:1234/target$popup", false],
+ // filter says '/target' as a popup should be blocked
+ // expect no opened tab
Wladimir Palant 2015/12/14 11:28:52 Frankly, I think that this is overdocumenting. I w
Erik 2015/12/29 22:49:40 We can leave this for another issue, I'll take it
+ //["||127.0.0.1:1234/target$popup", false],
Wladimir Palant 2015/12/14 11:28:53 Why is this test commented out, isn't it working c
Erik 2015/12/29 22:49:40 oh I didn't notice that I did that, iirc I comment
+ // filter says '/target' as a subdoc should be blocked
+ // expect to find OK message in opened tab (b/c popup is not blocked)
["||127.0.0.1:1234/target$~subdocument", true],
+ // filter says '/target' as a popup should be blocked from domain 127.0.0.1
+ // expect no opened tab
["||127.0.0.1:1234/target$popup,domain=127.0.0.1", false],
+ // filter says '/target' as a popup should be blocked from domain 128.0.0.1
+ // expect to find OK message in opened tab (b/c popup is not blocked)
["||127.0.0.1:1234/target$popup,domain=128.0.0.1", true],
+ // filter says '/redirect' as a popup should be blocked
+ // expect no opened tab
["||127.0.0.1:1234/redirect$popup", false],
+ // filter says '/redirect' as a subdocument should be blocked
+ // expect to find OK message in opened tab (b/c popup is not blocked)
["||127.0.0.1:1234/redirect$~subdocument", true],
+ // filter says '/redirect' as a popup should be blocked from domain 127.0.0.1
+ // expect no opened tab
["||127.0.0.1:1234/redirect$popup,domain=127.0.0.1", false],
+ // filter says '/redirect' as a popup should be blocked from domain 128.0.0.1
+ // expect to find OK message in opened tab (b/c popup is not blocked)
["||127.0.0.1:1234/redirect$popup,domain=128.0.0.1", true],
];
+ var testCount = 0;
Wladimir Palant 2015/12/14 11:28:52 This variable seems unused.
function runTest(filter, result)
{
+ tabs.off("ready", onTabOpen);
Wladimir Palant 2015/12/14 11:28:53 This seems pointless, why would there be an existi
Erik 2015/12/29 22:49:40 This should be in `onTabOpen`.
FilterStorage.addFilter(filter);
let successful = false;
- function onTabOpen(event)
- {
+ function onTabOpen(tab) {
Wladimir Palant 2015/12/14 11:28:52 Style nit: Bracket on the next line please.
+ // link in '/test' was clicked
+ tab.on("close", onTabClose);
window.clearTimeout(timeout);
- wnd.gBrowser.tabContainer.removeEventListener("TabOpen", onTabOpen, false);
- let tab = event.target;
- let browser = wnd.gBrowser.getBrowserForTab(tab);
- Utils.runAsync(function()
+ var worker = tab.attach({
+ contentScriptWhen: "ready",
+ contentScript: "self.port.emit('done', document.body.innerHTML.toString());"
Wladimir Palant 2015/12/14 11:28:52 No point calling toString() here - innerHTML is al
Erik 2015/12/29 22:49:40 good point.
+ });
+
+ worker.port.once("done", function(bodyText)
{
- browser.contentWindow.addEventListener("load", function(event)
- {
- if (browser.contentDocument.body.textContent.indexOf("OK") >= 0)
- successful = true;
+ if (bodyText.indexOf("OK") >= 0)
+ successful = true;
- browser.contentWindow.close();
- }, false);
+ // pop-up was not blocked so close it
+ tab.close();
});
}
+ tabs.on("ready", onTabOpen);
- function onTabClose(event)
+ function onTabClose(tab)
{
- wnd.gBrowser.tabContainer.removeEventListener("TabClose", onTabClose, false);
+ tabs.off("ready", onTabOpen);
+ if (tab)
+ tab.off("close", onTabClose);
+
ok(result == successful, "Opening tab with filter " + filter.text);
+
var keys = [];
for (let key in defaultMatcher.blacklist.keywordByFilter)
keys.push(key);
Wladimir Palant 2015/12/14 11:28:52 I have no idea why this code is here, it isn't bei
Erik 2015/12/29 22:49:40 alright it doesn't seem to be used to me either.
FilterStorage.removeFilter(filter);
+
start();
}
- wnd.gBrowser.tabContainer.addEventListener("TabOpen", onTabOpen, false);
- wnd.gBrowser.tabContainer.addEventListener("TabClose", onTabClose, false);
- let timeout = window.setTimeout(onTabClose, 1000); // In case the tab isn't opened
+ // In case the tab isn't opened
+ var timeout = window.setTimeout(function()
+ {
+ onTabClose();
+ }, 1000);
Wladimir Palant 2015/12/14 11:28:52 Nit: no point wrapping the callback here either. T
- wnd.gBrowser.getBrowserForTab(tab).contentDocument.getElementById("link").click();
+ // click the link in the '/test' tab opened before the test
+ var worker = tab.attach({
+ contentScriptWhen: "ready",
+ contentScriptOptions: {
+ filter: filter.toString()
+ },
Wladimir Palant 2015/12/14 11:28:52 The content script doesn't need any options...
+ contentScript: "(" + function()
+ {
+ document.getElementById('link').click();
+ } + ")()"
+ //contentScript: "document.getElementById('link').click();"
Wladimir Palant 2015/12/14 11:28:52 Please remove this line.
+ });
}
+ // create async qunit tests
Wladimir Palant 2015/12/14 11:28:52 This is stating the obvious again.
for (let [filter, result] of tests)
asyncTest(filter, runTest.bind(null, Filter.fromText(filter), result));
})();
« chrome/content/common.js ('K') | « chrome/content/common.js ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld