Bug 1007520 - Part 2: Avoid leaking Activities.callers in ActivitiesService.jsm. r=fabrice
authorTing-Yu Chou <janus926@gmail.com>
Tue, 10 Jun 2014 14:05:38 +0800
changeset 187832 0522e007a3aaf1059fe0b5307e341a8711e5c50a
parent 187831 066ed94a586898531e9e867fdc26c882f5fd58d8
child 187833 b1d4ab072fc255cd9df2aa713bd69f7a9df1740a
push id44666
push userkhuey@mozilla.com
push dateTue, 10 Jun 2014 06:09:15 +0000
treeherdermozilla-inbound@b1d4ab072fc2 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersfabrice
bugs1007520
milestone33.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1007520 - Part 2: Avoid leaking Activities.callers in ActivitiesService.jsm. r=fabrice
dom/activities/src/ActivitiesService.jsm
--- a/dom/activities/src/ActivitiesService.jsm
+++ b/dom/activities/src/ActivitiesService.jsm
@@ -202,83 +202,81 @@ let Activities = {
     * - finds a list of matching activities.
     * - calls the UI glue to get the user choice.
     * - fire an system message of type "activity" to this app, sending the
     *   activity data as a payload.
     */
   startActivity: function activities_startActivity(aMsg) {
     debug("StartActivity: " + JSON.stringify(aMsg));
 
+    let self = this;
     let successCb = function successCb(aResults) {
       debug(JSON.stringify(aResults));
 
       function getActivityChoice(aResultType, aResult) {
         switch(aResultType) {
           case Ci.nsIActivityUIGlueCallback.NATIVE_ACTIVITY: {
-            Activities.callers[aMsg.id].mm.sendAsyncMessage("Activity:FireSuccess", {
+            self.callers[aMsg.id].mm.sendAsyncMessage("Activity:FireSuccess", {
               "id": aMsg.id,
               "result": aResult
             });
             break;
           }
           case Ci.nsIActivityUIGlueCallback.WEBAPPS_ACTIVITY: {
             debug("Activity choice: " + aResult);
 
             // We have no matching activity registered, let's fire an error.
             // Don't do this check until we have passed to UIGlue so the glue can choose to launch
             // its own activity if needed.
             if (aResults.options.length === 0) {
-              Activities.callers[aMsg.id].mm.sendAsyncMessage("Activity:FireError", {
+              self.trySendAndCleanup(aMsg.id, "Activity:FireError", {
                 "id": aMsg.id,
                 "error": "NO_PROVIDER"
               });
-              delete Activities.callers[aMsg.id];
               return;
             }
 
             // The user has cancelled the choice, fire an error.
             if (aResult === -1) {
-              Activities.callers[aMsg.id].mm.sendAsyncMessage("Activity:FireError", {
+              self.trySendAndCleanup(aMsg.id, "Activity:FireError", {
                 "id": aMsg.id,
                 "error": "ActivityCanceled"
               });
-              delete Activities.callers[aMsg.id];
               return;
             }
 
             let sysmm = Cc["@mozilla.org/system-message-internal;1"]
                           .getService(Ci.nsISystemMessagesInternal);
             if (!sysmm) {
               // System message is not present, what should we do?
-              delete Activities.callers[aMsg.id];
+              delete self.callers[aMsg.id];
               return;
             }
 
             debug("Sending system message...");
             let result = aResults.options[aResult];
             sysmm.sendMessage("activity", {
                 "id": aMsg.id,
                 "payload": aMsg.options,
                 "target": result.description
               },
               Services.io.newURI(result.description.href, null, null),
               Services.io.newURI(result.manifest, null, null),
               {
-                "manifestURL": Activities.callers[aMsg.id].manifestURL,
-                "pageURL": Activities.callers[aMsg.id].pageURL
+                "manifestURL": self.callers[aMsg.id].manifestURL,
+                "pageURL": self.callers[aMsg.id].pageURL
               });
 
             if (!result.description.returnValue) {
-              Activities.callers[aMsg.id].mm.sendAsyncMessage("Activity:FireSuccess", {
+              // No need to notify observers, since we don't want the caller
+              // to be raised on the foreground that quick.
+              self.trySendAndCleanup(aMsg.id, "Activity:FireSuccess", {
                 "id": aMsg.id,
                 "result": null
               });
-              // No need to notify observers, since we don't want the caller
-              // to be raised on the foreground that quick.
-              delete Activities.callers[aMsg.id];
             }
             break;
           }
         }
       };
 
       let caller = Activities.callers[aMsg.id];
       if (aMsg.getFilterResults === true &&
@@ -327,16 +325,24 @@ let Activities = {
     let matchFunc = function matchFunc(aResult) {
       return ActivitiesServiceFilter.match(aMsg.options.data,
                                            aResult.description.filters);
     };
 
     this.db.find(aMsg, successCb, errorCb, matchFunc);
   },
 
+  trySendAndCleanup: function activities_trySendAndCleanup(aId, aName, aPayload) {
+    try {
+      this.callers[aId].mm.sendAsyncMessage(aName, aPayload);
+    } finally {
+      delete this.callers[aId];
+    }
+  },
+
   receiveMessage: function activities_receiveMessage(aMessage) {
     let mm = aMessage.target;
     let msg = aMessage.json;
 
     let caller;
     let obsData;
 
     if (aMessage.name == "Activity:PostResult" ||
@@ -360,22 +366,20 @@ let Activities = {
         this.startActivity(msg);
         break;
 
       case "Activity:Ready":
         caller.childMM = mm;
         break;
 
       case "Activity:PostResult":
-        caller.mm.sendAsyncMessage("Activity:FireSuccess", msg);
-        delete this.callers[msg.id];
+        this.trySendAndCleanup(msg.id, "Activity:FireSuccess", msg);
         break;
       case "Activity:PostError":
-        caller.mm.sendAsyncMessage("Activity:FireError", msg);
-        delete this.callers[msg.id];
+        this.trySendAndCleanup(msg.id, "Activity:FireError", msg);
         break;
 
       case "Activities:Register":
         let self = this;
         this.db.add(msg,
           function onSuccess(aEvent) {
             mm.sendAsyncMessage("Activities:Register:OK", null);
             let res = [];
@@ -393,21 +397,20 @@ let Activities = {
         let res = [];
         msg.forEach(function(aActivity) {
           this.updateContentTypeList(aActivity, res);
         }, this);
         break;
       case "child-process-shutdown":
         for (let id in this.callers) {
           if (this.callers[id].childMM == mm) {
-            this.callers[id].mm.sendAsyncMessage("Activity:FireError", {
+            this.trySendAndCleanup(id, "Activity:FireError", {
               "id": id,
               "error": "ActivityCanceled"
             });
-            delete this.callers[id];
             break;
           }
         }
         break;
     }
   },
 
   updateContentTypeList: function updateContentTypeList(aActivity, aResult) {