Issue 101 - Fix policy tests

chrome/content/tests/policy.js

 (function()
 {
   let server = null;
   let frame = null;
   let requestNotifier = null;
 
   module("Content policy", {
     setup: function()
     {
       prepareFilterComponents.call(this);
@@ skipping 184 matching lines @@
     [
       "HTML object (Java)",
       '<object type="application/x-java-applet" data="test.class"></object>',
       "http://127.0.0.1:1234/test.class", "object", false, false
     ]);
   }
 
   let policyHits = [];
   function onPolicyHit(wnd, node, item, scanComplete)
   {
-    if (!item) {

[Wladimir Palant, 2014/07/09 18:48:32]

Nit: opening { belongs onto the next line. Alternatively, you can simply omit
the brackets here.

+    if (!item)
       return;
-    }
     if (item.location == "http://127.0.0.1:1234/test" ||
         item.location == "http://127.0.0.1:1234/redirect.gif" ||
         item.location == "http://127.0.0.1:1234/redirect2.gif")
     {
       return;
     }
     if (item.filter instanceof WhitelistFilter)
       return;
 
     if (policyHits.length > 0)
@@ skipping 70 matching lines @@
 
           equal(actualStatus, expectedStatus, "Request blocked");
           equal(item.typeDescr.toLowerCase(), expectedType, "Request type");
           equal(item.thirdParty, expectedThirdParty, "Third-party flag");
           equal(item.docDomain, "127.0.0.1", "Document domain");
         }
       }
       server.registerPathHandler(expectedURL.replace(/http:\/\/[^\/]+/, ""), null);
       equal(serverHit, expectedStatus == "allowed", "Request received by server");
 
-      if (waitForMessage) {

[Wladimir Palant, 2014/07/09 18:48:32]

Nit: As above, the opening { belongs onto the next line.

-        window.removeEventListener("message", callback, true)
-      }

[Wladimir Palant, 2014/07/09 18:48:32]

No need to remove conditionally, we can simply always remove the listener rather
than duplicate the logic. We should also clear frame.onload - it is no longer
being overridden on each new test and will stay around otherwise.

+      window.removeEventListener("message", callback, true);
+      frame.removeEventListener("load", callback, true);
 
       start();
     };
     frame.contentWindow.location.href = "http://127.0.0.1:1234/test";
 
-    if (waitForMessage) {

[Wladimir Palant, 2014/07/09 18:48:32]

Nit: As above, the opening { belongs onto the next line.

+    if (waitForMessage)
       window.addEventListener("message", callback, true, true);
-    } else {
-      frame.onload = callback;

[Wladimir Palant, 2014/07/09 18:48:32]

Feel free to use frame.addEventListener("load", ...) for consistency here.

-    }
-
+    else
+      frame.addEventListener("load", callback, true, true);

[Wladimir Palant, 2014/07/14 13:33:54]

I don't think that we need a capturing listener, and we don't need untrusted
events either. So please use addEventListener("load", callback, false). Feel
free to check whether we really need capturing for the "message" event above.

   }
 
   let stageDescriptions = {
     1: "running without filters",
     2: "running with filter %S",
     3: "running with filter %S and site exception",
     4: "running with filter %S and exception not applicable to sites",
   };
 
   for (let test = 0; test < tests.length; test++)
   {
     let [name, body, expectedURL, expectedType, expectedDomain, expectedThirdParty] = tests[test];
     for (let stage = 1; stage in stageDescriptions; stage++)
     {
       let stageDescription = stageDescriptions[stage];
       if (stageDescription.indexOf("%S") >= 0)
         stageDescription = stageDescription.replace("%S", expectedURL);
 
       asyncTest(name + " (" + stageDescription + ")", runTest.bind(null, tests[test], stage));
     }
   }
 })();