Bug 1026737 - Implement listener counting on DOMRequestHelper r=fabrice
authorAntonio M. Amaya <amac@tid.es>
Thu, 19 Jun 2014 19:51:05 -0700
changeset 189653 acf7a09d3f458a3711429795e39537d338bdc14d
parent 189652 169296bb408c594d4dd2149d9c899bfa00c4ac12
child 189654 f6eaee3256bdd547320d17e8a3072e48faef87b8
push id1
push userroot
push dateMon, 20 Oct 2014 17:29:22 +0000
reviewersfabrice
bugs1026737
milestone33.0a1
Bug 1026737 - Implement listener counting on DOMRequestHelper r=fabrice
dom/base/DOMRequestHelper.jsm
dom/base/test/test_domrequesthelper.xul
--- a/dom/base/DOMRequestHelper.jsm
+++ b/dom/base/DOMRequestHelper.jsm
@@ -78,28 +78,33 @@ DOMRequestIpcHelper.prototype = {
 
     if (!Array.isArray(aMessages)) {
       aMessages = [aMessages];
     }
 
     aMessages.forEach((aMsg) => {
       let name = aMsg.name || aMsg;
       // If the listener is already set and it is of the same type we just
-      // bail out. If it is not of the same type, we throw an exception.
+      // increase the count and bail out. If it is not of the same type,
+      // we throw an exception.
       if (this._listeners[name] != undefined) {
-        if (!!aMsg.weakRef == this._listeners[name]) {
+        if (!!aMsg.weakRef == this._listeners[name].weakRef) {
+          this._listeners[name].count++;
           return;
         } else {
           throw Cr.NS_ERROR_FAILURE;
         }
       }
 
       aMsg.weakRef ? cpmm.addWeakMessageListener(name, this)
                    : cpmm.addMessageListener(name, this);
-      this._listeners[name] = !!aMsg.weakRef;
+      this._listeners[name] = {
+        weakRef: !!aMsg.weakRef,
+        count: 1
+      };
     });
   },
 
   /**
    * 'aMessages' is expected to be a string or an array of strings containing
    * the message names of the listeners to be removed.
    */
   removeMessageListeners: function(aMessages) {
@@ -111,19 +116,24 @@ DOMRequestIpcHelper.prototype = {
       aMessages = [aMessages];
     }
 
     aMessages.forEach((aName) => {
       if (this._listeners[aName] == undefined) {
         return;
       }
 
-      this._listeners[aName] ? cpmm.removeWeakMessageListener(aName, this)
-                             : cpmm.removeMessageListener(aName, this);
-      delete this._listeners[aName];
+      // Only remove the listener really when we don't have anybody that could
+      // be waiting on a message.
+      if (!--this._listeners[aName].count) {
+        this._listeners[aName].weakRef ?
+            cpmm.removeWeakMessageListener(aName, this)
+          : cpmm.removeMessageListener(aName, this);
+        delete this._listeners[aName];
+      }
     });
   },
 
   /**
    * Initialize the helper adding the corresponding listeners to the messages
    * provided as the second parameter.
    *
    * 'aMessages' is expected to be an array of either:
--- a/dom/base/test/test_domrequesthelper.xul
+++ b/dom/base/test/test_domrequesthelper.xul
@@ -61,30 +61,32 @@
       is(dummy._messages, undefined, "Messages is undefined");
       is(dummy._window, undefined, "Window is undefined");
     }
 
     /**
      * Message listeners.
      */
     function checkMessageListeners(aExpectedListeners, aCount) {
-      ok(true, "Checking message listeners\n" + "Expected listeners " +
-         JSON.stringify(aExpectedListeners) + " \nExpected count " + aCount);
+      info("Checking message listeners\n" + "Expected listeners " +
+           JSON.stringify(aExpectedListeners) + " \nExpected count " + aCount);
       let count = 0;
       Object.keys(dummy._listeners).forEach(function(name) {
         count++;
-        is(aExpectedListeners[name], dummy._listeners[name],
-           "Message found " + name + " - Same listeners");
+        is(aExpectedListeners[name].weakRef, dummy._listeners[name].weakRef,
+           "Message found " + name + " - Same weakRef");
+        is(aExpectedListeners[name].count, dummy._listeners[name].count,
+           "Message found " + name + " - Same count");
       });
       is(aCount, count, "Correct number of listeners");
     }
 
     function addMessageListenersTest(aMessages, aExpectedListeners, aCount) {
       dummy.addMessageListeners(aMessages);
-      ok(true, JSON.stringify(dummy._listeners));
+      info(JSON.stringify(dummy._listeners));
       checkMessageListeners(aExpectedListeners, aCount);
     }
 
     function removeMessageListenersTest(aMessages, aExpectedListeners, aCount) {
       dummy.removeMessageListeners(aMessages);
       checkMessageListeners(aExpectedListeners, aCount);
     }
 
@@ -160,248 +162,271 @@
       document.documentElement.appendChild(frame);
     }
 
     /**
      * Test steps.
      */
     var tests = [
       function() {
-        ok(true, "== InitDOMRequestHelper no messages");
+        info("== InitDOMRequestHelper no messages");
         initDOMRequestHelperTest(null);
         next();
       },
       function() {
-        ok(true, "== DestroyDOMRequestHelper");
+        info("== DestroyDOMRequestHelper");
         destroyDOMRequestHelperTest();
         next();
       },
       function() {
-        ok(true, "== InitDOMRequestHelper empty array");
+        info("== InitDOMRequestHelper empty array");
         initDOMRequestHelperTest([]);
         checkMessageListeners({}, 0);
         next();
       },
       function() {
-        ok(true, "== DestroyDOMRequestHelper");
+        info("== DestroyDOMRequestHelper");
         destroyDOMRequestHelperTest();
         next();
       },
       function() {
-        ok(true, "== InitDOMRequestHelper with strings array");
+        info("== InitDOMRequestHelper with strings array");
         initDOMRequestHelperTest(["name1", "nameN"]);
-        checkMessageListeners({"name1": false, "nameN": false}, 2);
+        checkMessageListeners({"name1": {weakRef: false, count: 1},
+                               "nameN": {weakRef: false, count: 1}}, 2);
         next();
       },
       function() {
-        ok(true, "== DestroyDOMRequestHelper");
+        info("== DestroyDOMRequestHelper");
         destroyDOMRequestHelperTest();
         next();
       },
       function() {
-        ok(true, "== InitDOMRequestHelper with objects array");
+        info("== InitDOMRequestHelper with objects array");
         initDOMRequestHelperTest([{
           name: "name1",
           weakRef: false
         }, {
           name: "nameN",
           weakRef: true
         }]);
-        checkMessageListeners({"name1": false, "nameN": true}, 2);
-        next();
-      },
-      function() {
-        ok(true, "== AddMessageListeners empty array");
-        addMessageListenersTest([], {"name1": false, "nameN": true}, 2);
+        checkMessageListeners({
+          "name1": {weakRef: false, count: 1},
+          "nameN": {weakRef: true,  count: 1}
+        }, 2);
         next();
       },
       function() {
-        ok(true, "== AddMessageListeners null");
-        addMessageListenersTest(null, {"name1": false, "nameN": true}, 2);
+        info("== AddMessageListeners empty array");
+        addMessageListenersTest([], {
+        "name1": {weakRef: false, count: 1},
+        "nameN": {weakRef: true,  count: 1}
+        }, 2);
         next();
       },
       function() {
-        ok(true, "== AddMessageListeners new listener, string only");
+        info("== AddMessageListeners null");
+        addMessageListenersTest(null, {
+          "name1": {weakRef: false, count: 1},
+          "nameN": {weakRef: true,  count: 1}
+          }, 2);
+        next();
+      },
+      function() {
+        info("== AddMessageListeners new listener, string only");
         addMessageListenersTest("name2", {
-          "name1": false,
-          "name2": false,
-          "nameN": true
+          "name1": {weakRef: false, count: 1},
+          "name2": {weakRef: false, count: 1},
+          "nameN": {weakRef: true,  count: 1}
         }, 3);
         next();
       },
       function() {
-        ok(true, "== AddMessageListeners new listeners, strings array");
+        info("== AddMessageListeners new listeners, strings array");
         addMessageListenersTest(["name3", "name4"], {
-          "name1": false,
-          "name2": false,
-          "name3": false,
-          "name4": false,
-          "nameN": true
+          "name1": {weakRef: false, count: 1},
+          "name2": {weakRef: false, count: 1},
+          "name3": {weakRef: false, count: 1},
+          "name4": {weakRef: false, count: 1},
+          "nameN": {weakRef: true,  count: 1}
         }, 5);
         next();
       },
       function() {
-        ok(true, "== AddMessageListeners new listeners, objects array");
+        info("== AddMessageListeners new listeners, objects array");
         addMessageListenersTest([{
           name: "name5",
           weakRef: true
         }, {
           name: "name6",
           weakRef: false
         }], {
-          "name1": false,
-          "name2": false,
-          "name3": false,
-          "name4": false,
-          "name5": true,
-          "name6": false,
-          "nameN": true
+          "name1": {weakRef: false, count: 1},
+          "name2": {weakRef: false, count: 1},
+          "name3": {weakRef: false, count: 1},
+          "name4": {weakRef: false, count: 1},
+          "name5": {weakRef: true,  count: 1},
+          "name6": {weakRef: false, count: 1},
+          "nameN": {weakRef: true,  count: 1}
         }, 7);
         next();
       },
       function() {
-        ok(true, "== RemoveMessageListeners, null");
+        info("== RemoveMessageListeners, null");
         removeMessageListenersTest(null, {
-          "name1": false,
-          "name2": false,
-          "name3": false,
-          "name4": false,
-          "name5": true,
-          "name6": false,
-          "nameN": true
+          "name1": {weakRef: false, count: 1},
+          "name2": {weakRef: false, count: 1},
+          "name3": {weakRef: false, count: 1},
+          "name4": {weakRef: false, count: 1},
+          "name5": {weakRef: true,  count: 1},
+          "name6": {weakRef: false, count: 1},
+          "nameN": {weakRef: true,  count: 1}
         }, 7);
         next();
       },
       function() {
-        ok(true, "== RemoveMessageListeners, one message");
+        info("== RemoveMessageListeners, one message");
         removeMessageListenersTest("name1", {
-          "name2": false,
-          "name3": false,
-          "name4": false,
-          "name5": true,
-          "name6": false,
-          "nameN": true
+          "name2": {weakRef: false, count: 1},
+          "name3": {weakRef: false, count: 1},
+          "name4": {weakRef: false, count: 1},
+          "name5": {weakRef: true,  count: 1},
+          "name6": {weakRef: false, count: 1},
+          "nameN": {weakRef: true,  count: 1}
         }, 6);
         next();
       },
       function() {
-        ok(true, "== RemoveMessageListeners, array of messages");
+        info("== RemoveMessageListeners, array of messages");
         removeMessageListenersTest(["name2", "name3"], {
-          "name4": false,
-          "name5": true,
-          "name6": false,
-          "nameN": true
+          "name4": {weakRef: false, count: 1},
+          "name5": {weakRef: true,  count: 1},
+          "name6": {weakRef: false, count: 1},
+          "nameN": {weakRef: true,  count: 1}
         }, 4);
         next();
       },
       function() {
-        ok(true, "== RemoveMessageListeners, unknown message");
+        info("== RemoveMessageListeners, unknown message");
         removeMessageListenersTest("unknown", {
-          "name4": false,
-          "name5": true,
-          "name6": false,
-          "nameN": true
+          "name4": {weakRef: false, count: 1},
+          "name5": {weakRef: true,  count: 1},
+          "name6": {weakRef: false, count: 1},
+          "nameN": {weakRef: true,  count: 1}
         }, 4);
         next();
       },
       function() {
         try {
-          ok(true, "== AddMessageListeners, same message, same kind");
+          info("== AddMessageListeners, same message, same kind");
           addMessageListenersTest("name4", {
-            "name4": false,
-            "name5": true,
-            "name6": false,
-            "nameN": true
+            "name4": {weakRef: false, count: 2},
+            "name5": {weakRef: true,  count: 1},
+            "name6": {weakRef: false, count: 1},
+            "nameN": {weakRef: true,  count: 1}
           }, 4);
           next();
         } catch (ex) {
           ok(false, "Unexpected exception " + ex);
         }
       },
       function() {
-        ok(true, "== AddMessageListeners, same message, different kind");
+        info("== AddMessageListeners, same message, different kind");
         try {
           addMessageListenersTest({name: "name4", weakRef: true}, {
-            "name4": true,
-            "name5": true,
-            "name6": false,
-            "nameN": true
+            "name4": {weakRef: false, count: 2},
+            "name5": {weakRef: true,  count: 1},
+            "name6": {weakRef: false, count: 1},
+            "nameN": {weakRef: true,  count: 1}
           }, 4);
           ok(false, "Should have thrown an exception");
         } catch (ex) {
           ok(true, "Expected exception");
           next();
         }
       },
       function() {
-        ok(true, "== Test createRequest()");
+        info("== RemoveMessageListeners, message with two listeners");
+        try {
+          removeMessageListenersTest(["name4", "name5"], {
+            "name4": {weakRef: false, count: 1},
+            "name6": {weakRef: false, count: 1},
+            "nameN": {weakRef: true,  count: 1}
+          }, 3);
+          next();
+        } catch (ex) {
+          ok(false, "Unexpected exception " + ex);
+        }
+      },
+      function() {
+        info("== Test createRequest()");
         ok(DOMRequest, "DOMRequest object exists");
         var req = dummy.createRequest();
         ok(req instanceof DOMRequest, "Returned a DOMRequest");
         next();
       },
       function() {
-        ok(true, "== Test getRequestId(), removeRequest() and getRequest()");
+        info("== Test getRequestId(), removeRequest() and getRequest()");
         var req = dummy.createRequest();
         var id = dummy.getRequestId(req);
         is(typeof id, "string", "id is a string");
         var req_ = dummy.getRequest(id);
         is(req, req_, "Got correct request");
         dummy.removeRequest(id);
         req = dummy.getRequest(id);
         is(req, null, "No request");
         next();
       },
       function() {
-        ok(true, "== Test createPromise()");
+        info("== Test createPromise()");
         ok(Promise, "Promise object exists");
         var promise = dummy.createPromise(function(resolve, reject) {
           resolve(true);
         });
         ok(promise instanceof Promise, "Returned a Promise");
         promise.then(next);
       },
       function() {
-        ok(true, "== Test getResolver()");
+        info("== Test getResolver()");
         var id;
         var resolver;
         var promise = dummy.createPromise(function(resolve, reject) {
           var r = { resolve: resolve, reject: reject };
           id = dummy.getPromiseResolverId(r);
           resolver = r;
           ok(typeof id === "string", "id is a string");
           r.resolve(true);
         }).then(function(unused) {
           var r = dummy.getPromiseResolver(id);
           ok(resolver === r, "Get succeeded");
           next();
         });
       },
       function() {
-        ok(true, "== Test removeResolver");
+        info("== Test removeResolver");
         var id;
         var promise = dummy.createPromise(function(resolve, reject) {
           var r = { resolve: resolve, reject: reject };
           id = dummy.getPromiseResolverId(r);
           ok(typeof id === "string", "id is a string");
 
           var resolver = dummy.getPromiseResolver(id);
-          ok(true, "Got resolver " + JSON.stringify(resolver));
+          info("Got resolver " + JSON.stringify(resolver));
           ok(resolver === r, "Resolver get succeeded");
 
           r.resolve(true);
         }).then(function(unused) {
           dummy.removePromiseResolver(id);
           var resolver = dummy.getPromiseResolver(id);
           ok(resolver === undefined, "removeResolver: get failed");
           next();
         });
       },
       function() {
-        ok(true, "== Test takeResolver");
+        info("== Test takeResolver");
         var id;
         var resolver;
         var promise = dummy.createPromise(function(resolve, reject) {
           var r = { resolve: resolve, reject: reject };
           id = dummy.getPromiseResolverId(r);
           resolver = r;
           ok(typeof id === "string", "id is a string");
 
@@ -414,55 +439,55 @@
           ok(resolver === r, "take should succeed");
 
           r = dummy.getPromiseResolver(id);
           ok(r === undefined, "takeResolver: get failed");
           next();
         });
       },
       function() {
-        ok(true, "== Test window destroyed without messages and without GC");
+        info("== Test window destroyed without messages and without GC");
         checkWindowDestruction({ gc: false }, function(uninitCalled) {
           ok(uninitCalled, "uninit() should have been called");
           next();
         });
       },
       function() {
-        ok(true, "== Test window destroyed without messages and with GC");
+        info("== Test window destroyed without messages and with GC");
         checkWindowDestruction({ gc: true }, function(uninitCalled) {
           ok(!uninitCalled, "uninit() should NOT have been called");
           next();
         });
       },
       function() {
-        ok(true, "== Test window destroyed with weak messages and without GC");
+        info("== Test window destroyed with weak messages and without GC");
         checkWindowDestruction({ messages: [{ name: "foo", weakRef: true }],
                                  gc: false }, function(uninitCalled) {
           ok(uninitCalled, "uninit() should have been called");
           next();
         });
       },
       function() {
-        ok(true, "== Test window destroyed with weak messages and with GC");
+        info("== Test window destroyed with weak messages and with GC");
         checkWindowDestruction({ messages: [{ name: "foo", weakRef: true }],
                                  gc: true }, function(uninitCalled) {
           ok(!uninitCalled, "uninit() should NOT have been called");
           next();
         });
       },
       function() {
-        ok(true, "== Test window destroyed with strong messages and without GC");
+        info("== Test window destroyed with strong messages and without GC");
         checkWindowDestruction({ messages: [{ name: "foo", weakRef: false }],
                                  gc: false }, function(uninitCalled) {
           ok(uninitCalled, "uninit() should have been called");
           next();
         });
       },
       function() {
-        ok(true, "== Test window destroyed with strong messages and with GC");
+        info("== Test window destroyed with strong messages and with GC");
         checkWindowDestruction({ messages: [{ name: "foo", weakRef: false }],
                                  gc: true }, function(uninitCalled) {
           ok(uninitCalled, "uninit() should have been called");
           next();
         });
       }
     ];