Bug 1260548: Part 8 - Cut down on unnecessary console warnings during test runs. r=aswan
authorKris Maglione <maglione.k@gmail.com>
Sun, 29 Jan 2017 00:58:25 -0800
changeset 332140 8e67a3e9a5bfd7eff6327963a9c66fa13f3d7e80
parent 332139 1c8f0e831975e3f24c0b2c518134a8bc4e93e1e4
child 332141 bc59eff957ebf8d522b91f275ec3838e50df3392
push id31297
push usercbook@mozilla.com
push dateThu, 02 Feb 2017 13:27:06 +0000
treeherdermozilla-central@823dc40ab5fe [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersaswan
bugs1260548
milestone54.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 1260548: Part 8 - Cut down on unnecessary console warnings during test runs. r=aswan This isn't strictly related to the rest of the changes, but was helpful in the course of debugging them. MozReview-Commit-ID: 2nvccYYVXfR
toolkit/components/extensions/ExtensionChild.jsm
toolkit/components/extensions/ExtensionParent.jsm
toolkit/components/extensions/ExtensionUtils.jsm
toolkit/components/extensions/MessageChannel.jsm
--- a/toolkit/components/extensions/ExtensionChild.jsm
+++ b/toolkit/components/extensions/ExtensionChild.jsm
@@ -36,16 +36,17 @@ XPCOMUtils.defineLazyModuleGetter(this, 
 const CATEGORY_EXTENSION_SCRIPTS_ADDON = "webextension-scripts-addon";
 const CATEGORY_EXTENSION_SCRIPTS_DEVTOOLS = "webextension-scripts-devtools";
 
 Cu.import("resource://gre/modules/ExtensionCommon.jsm");
 Cu.import("resource://gre/modules/ExtensionUtils.jsm");
 
 const {
   DefaultMap,
+  LimitedSet,
   SingletonEventManager,
   SpreadArgs,
   defineLazyGetter,
   getInnerWindowID,
   getMessageManager,
   getUniqueId,
   injectAPI,
   promiseEvent,
@@ -539,16 +540,17 @@ class ProxyAPIImplementation extends Sch
 
     if (!map.listeners.has(listener)) {
       return;
     }
 
     let id = map.listeners.get(listener);
     map.listeners.delete(listener);
     map.ids.delete(id);
+    map.removedIds.add(id);
 
     this.childApiManager.messageManager.sendAsyncMessage("API:RemoveListener", {
       childId: this.childApiManager.id,
       listenerId: id,
       path: this.path,
     });
   }
 
@@ -578,16 +580,17 @@ class ChildAPIManager {
     MessageChannel.addListener(messageManager, "API:RunListener", this);
     messageManager.addMessageListener("API:CallResult", this);
 
     this.messageFilterStrict = {childId: this.id};
 
     this.listeners = new DefaultMap(() => ({
       ids: new Map(),
       listeners: new Map(),
+      removedIds: new LimitedSet(10),
     }));
 
     // Map[callId -> Deferred]
     this.callPromises = new Map();
 
     let params = {
       childId: this.id,
       extensionId: context.extension.id,
@@ -606,18 +609,20 @@ class ChildAPIManager {
     switch (name || messageName) {
       case "API:RunListener":
         let map = this.listeners.get(data.path);
         let listener = map.ids.get(data.listenerId);
 
         if (listener) {
           return this.context.runSafe(listener, ...data.args);
         }
-
-        Cu.reportError(`Unknown listener at childId=${data.childId} path=${data.path} listenerId=${data.listenerId}\n`);
+        if (!map.removedIds.has(data.listenerId)) {
+          Services.console.logStringMessage(
+            `Unknown listener at childId=${data.childId} path=${data.path} listenerId=${data.listenerId}\n`);
+        }
         break;
 
       case "API:CallResult":
         let deferred = this.callPromises.get(data.callId);
         if ("error" in data) {
           deferred.reject(data.error);
         } else {
           deferred.resolve(new SpreadArgs(data.result));
--- a/toolkit/components/extensions/ExtensionParent.jsm
+++ b/toolkit/components/extensions/ExtensionParent.jsm
@@ -566,17 +566,18 @@ ParentAPIManager = {
   call(data, target) {
     let context = this.getContextById(data.childId);
     if (context.parentMessageManager !== target.messageManager) {
       throw new Error("Got message on unexpected message manager");
     }
 
     let reply = result => {
       if (!context.parentMessageManager) {
-        Cu.reportError("Cannot send function call result: other side closed connection");
+        Services.console.logStringMessage("Cannot send function call result: other side closed connection " +
+                                          `(call data: ${uneval({path: data.path, args: data.args})})`);
         return;
       }
 
       context.parentMessageManager.sendAsyncMessage(
         "API:CallResult",
         Object.assign({
           childId: data.childId,
           callId: data.callId,
--- a/toolkit/components/extensions/ExtensionUtils.jsm
+++ b/toolkit/components/extensions/ExtensionUtils.jsm
@@ -726,16 +726,43 @@ function injectAPI(source, dest) {
       injectAPI(desc.value, obj);
     } else {
       Object.defineProperty(dest, prop, desc);
     }
   }
 }
 
 /**
+ * A set with a limited number of slots, which flushes older entries as
+ * newer ones are added.
+ */
+class LimitedSet extends Set {
+  constructor(limit, iterable = undefined) {
+    super(iterable);
+    this.limit = limit;
+  }
+
+  truncate(limit) {
+    for (let item of this) {
+      if (this.size <= limit) {
+        break;
+      }
+      this.delete(item);
+    }
+  }
+
+  add(item) {
+    if (!this.has(item) && this.size >= this.limit) {
+      this.truncate(this.limit - 1);
+    }
+    super.add(item);
+  }
+}
+
+/**
  * Returns a Promise which resolves when the given document's DOM has
  * fully loaded.
  *
  * @param {Document} doc The document to await the load of.
  * @returns {Promise<Document>}
  */
 function promiseDocumentReady(doc) {
   if (doc.readyState == "interactive" || doc.readyState == "complete") {
@@ -1032,16 +1059,20 @@ class MessageManagerProxy {
   sendAsyncMessage(...args) {
     if (this.messageManager) {
       return this.messageManager.sendAsyncMessage(...args);
     }
     /* globals uneval */
     Cu.reportError(`Cannot send message: Other side disconnected: ${uneval(args)}`);
   }
 
+  get isDisconnected() {
+    return !this.messageManager;
+  }
+
   /**
    * Adds a message listener to the current message manager, and
    * transfers it to the new message manager after a docShell swap.
    *
    * @param {string} message
    *        The name of the message to listen for.
    * @param {nsIMessageListener} listener
    *        The listener to add.
@@ -1147,14 +1178,15 @@ this.ExtensionUtils = {
   runSafeSyncWithoutClone,
   runSafeWithoutClone,
   stylesheetMap,
   DefaultMap,
   DefaultWeakMap,
   EventEmitter,
   ExtensionError,
   IconDetails,
+  LimitedSet,
   LocaleData,
   MessageManagerProxy,
   PlatformInfo,
   SingletonEventManager,
   SpreadArgs,
 };
--- a/toolkit/components/extensions/MessageChannel.jsm
+++ b/toolkit/components/extensions/MessageChannel.jsm
@@ -289,27 +289,33 @@ this.MessageChannel = {
     this.responseManagers = new FilteringMessageManagerMap(
       MESSAGE_RESPONSE, this._handleResponse.bind(this));
 
     /**
      * Contains a list of pending responses, either waiting to be
      * received or waiting to be sent. @see _addPendingResponse
      */
     this.pendingResponses = new Set();
+
+    /**
+     * Contains the message name of a limited number of aborted response
+     * handlers, the responses for which will be ignored.
+     */
+    this.abortedResponses = new ExtensionUtils.LimitedSet(30);
   },
 
   RESULT_SUCCESS: 0,
   RESULT_DISCONNECTED: 1,
   RESULT_NO_HANDLER: 2,
   RESULT_MULTIPLE_HANDLERS: 3,
   RESULT_ERROR: 4,
   RESULT_NO_RESPONSE: 5,
 
   REASON_DISCONNECTED: {
-    result: this.RESULT_DISCONNECTED,
+    result: 1, // this.RESULT_DISCONNECTED
     message: "Message manager disconnected",
   },
 
   /**
    * Specifies that only a single listener matching the specified
    * recipient tag may be listening for the given message, at the other
    * end of the target message manager.
    *
@@ -642,16 +648,26 @@ this.MessageChannel = {
           messageName: data.channelId,
           recipient: {},
           value,
         };
 
         target.sendAsyncMessage(MESSAGE_RESPONSE, response);
       },
       error => {
+        if (target.isDisconnected) {
+          // Target is disconnected. We can't send an error response, so
+          // don't even try.
+          if (error.result !== this.RESULT_DISCONNECTED &&
+              error.result !== this.RESULT_NO_RESPONSE) {
+            Cu.reportError(Cu.getClassName(error, false) === "Object" ? error.message : error);
+          }
+          return;
+        }
+
         let response = {
           result: this.RESULT_ERROR,
           messageName: data.channelId,
           recipient: {},
           error: {},
         };
 
         if (error && typeof(error) == "object") {
@@ -687,17 +703,22 @@ this.MessageChannel = {
    * @param {Array<MessageHandler>} handlers
    * @param {object} data
    * @param {nsIMessageSender|{messageManager:nsIMessageSender}} data.target
    */
   _handleResponse(handlers, data) {
     // If we have an error at this point, we have handler to report it to,
     // so just log it.
     if (handlers.length == 0) {
-      Cu.reportError(`No matching message response handler for ${data.messageName}`);
+      if (this.abortedResponses.has(data.messageName)) {
+        this.abortedResponses.delete(data.messageName);
+        Services.console.logStringMessage(`Ignoring response to aborted listener for ${data.messageName}`);
+      } else {
+        Cu.reportError(`No matching message response handler for ${data.messageName}`);
+      }
     } else if (handlers.length > 1) {
       Cu.reportError(`Multiple matching response handlers for ${data.messageName}`);
     } else if (data.result === this.RESULT_SUCCESS) {
       handlers[0].resolve(data.value);
     } else {
       handlers[0].reject(data.error);
     }
   },
@@ -748,16 +769,17 @@ this.MessageChannel = {
    * @param {object} [reason]
    *    An optional object describing the reason the response was aborted.
    *    Will be passed to the promise rejection handler of all aborted
    *    responses.
    */
   abortResponses(sender, reason = this.REASON_DISCONNECTED) {
     for (let response of this.pendingResponses) {
       if (this.matchesFilter(sender, response.sender)) {
+        this.abortedResponses.add(response.channelId);
         response.reject(reason);
       }
     }
   },
 
   /**
    * Aborts any pending message responses to the broker for the given
    * message manager.
@@ -767,16 +789,17 @@ this.MessageChannel = {
    * @param {object} reason
    *    An object describing the reason the responses were aborted.
    *    Will be passed to the promise rejection handler of all aborted
    *    responses.
    */
   abortMessageManager(target, reason) {
     for (let response of this.pendingResponses) {
       if (MessageManagerProxy.matches(response.messageManager, target)) {
+        this.abortedResponses.add(response.channelId);
         response.reject(reason);
       }
     }
   },
 
   observe(subject, topic, data) {
     switch (topic) {
       case "message-manager-close":