Bug 1260548: Part 8 - Cut down on unnecessary console warnings during test runs. r?aswan draft
authorKris Maglione <maglione.k@gmail.com>
Sun, 29 Jan 2017 00:58:25 -0800
changeset 467792 0341862fceef458479ef113dcc3e77c48835b56f
parent 467791 00652a6cf510512e9d64943916f11677ac1d3917
child 543777 f6d8efb5ffa62017f805b9e103f95396df5711dd
push id43274
push usermaglione.k@gmail.com
push dateSun, 29 Jan 2017 20:10:11 +0000
reviewersaswan
bugs1260548
milestone54.0a1
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
@@ -37,16 +37,17 @@ const CATEGORY_EXTENSION_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,
   EventManager,
+  LimitedSet,
   SingletonEventManager,
   SpreadArgs,
   defineLazyGetter,
   getInnerWindowID,
   getMessageManager,
   getUniqueId,
   injectAPI,
   promiseEvent,
@@ -540,16 +541,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,
     });
   }
 
@@ -579,16 +581,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,
@@ -607,18 +610,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
@@ -568,17 +568,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
@@ -779,16 +779,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") {
@@ -1086,16 +1113,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.
@@ -1202,14 +1233,15 @@ this.ExtensionUtils = {
   runSafeWithoutClone,
   stylesheetMap,
   DefaultMap,
   DefaultWeakMap,
   EventEmitter,
   EventManager,
   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":