Bug 1007520 - Part 2: Avoid leaking Activities.callers in ActivitiesService.jsm. r=fabrice a=khuey
authorTing-Yu Chou <janus926@gmail.com>
Tue, 10 Jun 2014 14:05:38 +0800
changeset 200130 987cdc05ff389c88e6a29160325da1c5d84efb21
parent 200129 948df010c6a0bd892d22e0d952905d8f303ab6a5
child 200131 7b60deda620e5726b7d6af0ac9b69f40b28795f0
push id6053
push userkhuey@mozilla.com
push dateWed, 11 Jun 2014 04:41:46 +0000
treeherdermozilla-aurora@7b60deda620e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersfabrice, khuey
bugs1007520
milestone32.0a2
Bug 1007520 - Part 2: Avoid leaking Activities.callers in ActivitiesService.jsm. r=fabrice a=khuey
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) {