Bug 876614 - Queue up the incomming message if we're currently dispatching a message to avoid nested event loop (e.g. via alert()). r=jlebar a=leo+
authorGene Lian <clian@mozilla.com>
Fri, 21 Jun 2013 16:17:22 +0800
changeset 147752 bd0fdb3585c66e4c47b3deb6166f57c539e16ec1
parent 147751 07ce3bc9b4dd3fd253b5b472987bbe8066a7e9bc
child 147753 1529545e7704385e0b5f4324d170f3b73c6b0c0c
push id2697
push userbbajaj@mozilla.com
push dateMon, 05 Aug 2013 18:49:53 +0000
treeherdermozilla-beta@dfec938c7b63 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjlebar, leo
bugs876614
milestone24.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 876614 - Queue up the incomming message if we're currently dispatching a message to avoid nested event loop (e.g. via alert()). r=jlebar a=leo+
dom/messages/SystemMessageManager.js
--- a/dom/messages/SystemMessageManager.js
+++ b/dom/messages/SystemMessageManager.js
@@ -22,19 +22,25 @@ XPCOMUtils.defineLazyServiceGetter(this,
 
 function debug(aMsg) {
    // dump("-- SystemMessageManager " + Date.now() + " : " + aMsg + "\n");
 }
 
 // Implementation of the DOM API for system messages
 
 function SystemMessageManager() {
-  // Message handlers for this page.
-  // We can have only one handler per message type.
-  this._handlers = {};
+  // If we have a system message handler registered for messages of type
+  // |type|, this._dispatchers[type] equals {handler, messages, isHandling},
+  // where
+  //  - |handler| is the message handler that the page registered,
+  //  - |messages| is a list of messages which we've received while
+  //    dispatching messages to the handler, but haven't yet sent, and
+  //  - |isHandling| indicates whether we're currently dispatching messages
+  //    to this handler.
+  this._dispatchers = {};
 
   // Pending messages for this page, keyed by message type.
   this._pendings = {};
 
   // Flag to specify if this process has already registered manifest.
   this._registerManifestReady = false;
 
   // Flag to determine this process is a parent or child process.
@@ -47,17 +53,31 @@ function SystemMessageManager() {
   if (this._isParentProcess) {
     Services.obs.addObserver(this, kSystemMessageInternalReady, false);
   }
 }
 
 SystemMessageManager.prototype = {
   __proto__: DOMRequestIpcHelper.prototype,
 
-  _dispatchMessage: function sysMessMgr_dispatchMessage(aType, aHandler, aMessage) {
+  _dispatchMessage: function sysMessMgr_dispatchMessage(aType, aDispatcher, aMessage) {
+    if (aDispatcher.isHandling) {
+      // Queue up the incomming message if we're currently dispatching a
+      // message; we'll send the message once we finish with the current one.
+      //
+      // _dispatchMethod is reentrant because a page can spin up a nested
+      // event loop from within a system message handler (e.g. via alert()),
+      // and we can then try to send the page another message while it's
+      // inside this nested event loop.
+      aDispatcher.messages.push(aMessage);
+      return;
+    }
+
+    aDispatcher.isHandling = true;
+
     // We get a json blob, but in some cases we want another kind of object
     // to be dispatched.
     // To do so, we check if we have a with a contract ID of
     // "@mozilla.org/dom/system-messages/wrapper/TYPE;1" component implementing
     // nsISystemMessageWrapper.
     debug("Dispatching " + JSON.stringify(aMessage) + "\n");
     let contractID = "@mozilla.org/dom/system-messages/wrapper/" + aType + ";1";
     let wrapped = false;
@@ -67,44 +87,58 @@ SystemMessageManager.prototype = {
       let wrapper = Cc[contractID].createInstance(Ci.nsISystemMessagesWrapper);
       if (wrapper) {
         aMessage = wrapper.wrapMessage(aMessage, this._window);
         wrapped = true;
         debug("wrapped = " + aMessage);
       }
     }
 
-    aHandler.handleMessage(wrapped ? aMessage
-                                   : ObjectWrapper.wrap(aMessage, this._window));
+    aDispatcher.handler
+      .handleMessage(wrapped ? aMessage
+                             : ObjectWrapper.wrap(aMessage, this._window));
+
+    // We need to notify the parent one of the system messages has been handled,
+    // so the parent can release the CPU wake lock it took on our behalf.
+    cpmm.sendAsyncMessage("SystemMessageManager:HandleMessagesDone",
+                          { type: aType,
+                            manifest: this._manifest,
+                            uri: this._uri,
+                            handledCount: 1 });
+
+    aDispatcher.isHandling = false;
+    if (aDispatcher.messages.length > 0) {
+      this._dispatchMessage(aType, aDispatcher, aDispatcher.messages.shift());
+    }
   },
 
   mozSetMessageHandler: function sysMessMgr_setMessageHandler(aType, aHandler) {
     debug("set message handler for [" + aType + "] " + aHandler);
 
     if (this._isInBrowserElement) {
       debug("the app loaded in the browser cannot set message handler");
       // Don't throw there, but ignore the registration.
       return;
     }
 
     if (!aType) {
       // Just bail out if we have no type.
       return;
     }
 
-    let handlers = this._handlers;
+    let dispatchers = this._dispatchers;
     if (!aHandler) {
-      // Setting the handler to null means we don't want to receive messages
-      // of this type anymore.
-      delete handlers[aType];
+      // Setting the dispatcher to null means we don't want to handle messages
+      // for this type anymore.
+      delete dispatchers[aType];
       return;
     }
 
     // Last registered handler wins.
-    handlers[aType] = aHandler;
+    dispatchers[aType] = { handler: aHandler, messages: [], isHandling: false };
 
     // Ask for the list of currently pending messages.
     cpmm.sendAsyncMessage("SystemMessageManager:GetPendingMessages",
                           { type: aType,
                             uri: this._uri,
                             manifest: this._manifest });
   },
 
@@ -113,28 +147,28 @@ SystemMessageManager.prototype = {
 
     if (this._isInBrowserElement) {
       debug("the app loaded in the browser cannot ask pending message");
       // Don't throw there, but pretend to have no messages available.
       return false;
     }
 
     // If we have a handler for this type, we can't have any pending message.
-    if (aType in this._handlers) {
+    if (aType in this._dispatchers) {
       return false;
     }
 
     return cpmm.sendSyncMessage("SystemMessageManager:HasPendingMessages",
                                 { type: aType,
                                   uri: this._uri,
                                   manifest: this._manifest })[0];
   },
 
   uninit: function sysMessMgr_uninit()  {
-    this._handlers = null;
+    this._dispatchers = null;
     this._pendings = null;
 
     if (this._isParentProcess) {
       Services.obs.removeObserver(this, kSystemMessageInternalReady);
     }
 
     if (this._isInBrowserElement) {
       debug("the app loaded in the browser doesn't need to unregister " +
@@ -182,35 +216,38 @@ SystemMessageManager.prototype = {
                               msgID: msg.msgID });
     }
 
     let messages = (aMessage.name == "SystemMessageManager:Message")
                    ? [msg.msg]
                    : msg.msgQueue;
 
     // We only dispatch messages when a handler is registered.
-    let handler = this._handlers[msg.type];
-    if (handler) {
+    let dispatcher = this._dispatchers[msg.type];
+    if (dispatcher) {
       messages.forEach(function(aMsg) {
-        this._dispatchMessage(msg.type, handler, aMsg);
+        this._dispatchMessage(msg.type, dispatcher, aMsg);
       }, this);
+    } else {
+      // We need to notify the parent that all the queued system messages have
+      // been handled (notice |handledCount: messages.length|), so the parent
+      // can release the CPU wake lock it took on our behalf.
+      cpmm.sendAsyncMessage("SystemMessageManager:HandleMessagesDone",
+                            { type: msg.type,
+                              manifest: this._manifest,
+                              uri: this._uri,
+                              handledCount: messages.length });
     }
 
-    // We need to notify the parent the system messages have been handled,
-    // even if there are no handlers registered for them, so the parent can
-    // release the CPU wake lock it took on our behalf.
-    cpmm.sendAsyncMessage("SystemMessageManager:HandleMessagesDone",
-                          { type: msg.type,
-                            manifest: this._manifest,
-                            uri: this._uri,
-                            handledCount: messages.length });
-
-    Services.obs.notifyObservers(/* aSubject */ null,
-                                 "handle-system-messages-done",
-                                 /* aData */ null);
+    if (!dispatcher || !dispatcher.isHandling) {
+      // TODO: Bug 874353 - Remove SystemMessageHandledListener in ContentParent
+      Services.obs.notifyObservers(/* aSubject */ null,
+                                   "handle-system-messages-done",
+                                   /* aData */ null);
+    }
   },
 
   // nsIDOMGlobalPropertyInitializer implementation.
   init: function sysMessMgr_init(aWindow) {
     debug("init");
     this.initHelper(aWindow, ["SystemMessageManager:Message",
                               "SystemMessageManager:GetPendingMessages:Return"]);
 
@@ -267,16 +304,17 @@ SystemMessageManager.prototype = {
   },
 
   classID: Components.ID("{bc076ea0-609b-4d8f-83d7-5af7cbdc3bb2}"),
 
   QueryInterface: XPCOMUtils.generateQI([Ci.nsIDOMNavigatorSystemMessages,
                                          Ci.nsIDOMGlobalPropertyInitializer,
                                          Ci.nsIObserver]),
 
-  classInfo: XPCOMUtils.generateCI({classID: Components.ID("{bc076ea0-609b-4d8f-83d7-5af7cbdc3bb2}"),
-                                    contractID: "@mozilla.org/system-message-manager;1",
-                                    interfaces: [Ci.nsIDOMNavigatorSystemMessages],
-                                    flags: Ci.nsIClassInfo.DOM_OBJECT,
-                                    classDescription: "System Messages"})
+  classInfo: XPCOMUtils.generateCI({
+    classID: Components.ID("{bc076ea0-609b-4d8f-83d7-5af7cbdc3bb2}"),
+    contractID: "@mozilla.org/system-message-manager;1",
+    interfaces: [Ci.nsIDOMNavigatorSystemMessages],
+    flags: Ci.nsIClassInfo.DOM_OBJECT,
+    classDescription: "System Messages"})
 }
 
 this.NSGetFactory = XPCOMUtils.generateNSGetFactory([SystemMessageManager]);