Issue 4580 - Replace ext.backgroundPage.sendMessage with runtime.sendMessage

ext/content.js

Index: ext/content.js
===================================================================
--- a/ext/content.js
+++ b/ext/content.js
@@ -24,62 +24,68 @@
 
   let backgroundFrame = document.createElement("iframe");
   backgroundFrame.setAttribute("src",
                                "background.html" + window.location.search);
   backgroundFrame.style.display = "none";
   window.addEventListener("DOMContentLoaded", () =>
   {
     document.body.appendChild(backgroundFrame);
-  }, false);

[Manish Jethani, 2017/10/11 15:42:41]

Not related but might as well.

+  });
 
   let messageQueue = [];
   let maxMessageId = -1;
   let loadHandler = (event) =>
   {
     if (event.data.type == "backgroundPageLoaded")
     {
       let queue = messageQueue;
       messageQueue = null;
       if (queue)
       {
         for (let message of queue)
           backgroundFrame.contentWindow.postMessage(message, "*");
       }
-      window.removeEventListener("message", loadHandler, false);
+      window.removeEventListener("message", loadHandler);
     }
   };
-  window.addEventListener("message", loadHandler, false);
+  window.addEventListener("message", loadHandler);
 
   ext.backgroundPage = {
     _sendRawMessage(message)
     {
       if (messageQueue)
         messageQueue.push(message);
       else
         backgroundFrame.contentWindow.postMessage(message, "*");
-    },
-    sendMessage(message, responseCallback)
-    {
-      let messageId = ++maxMessageId;
+    }
+  };
+
+  /* Polyfills */

[Manish Jethani, 2017/10/11 15:42:41]

This will move to polyfill.js as part of the other patch.

 
-      this._sendRawMessage({
-        type: "message",
-        messageId,
-        payload: message
-      });
+  if (!("runtime" in chrome))

[Thomas Greiner, 2017/10/11 15:58:16]

Suggestion: `chrome` is defined in `ext/common.js` so I'd recommend indicating
that via `/* globals chrome */` below the license header text

[Manish Jethani, 2017/10/11 16:11:20]

Is that really necessary? The chrome object is used all over the place, ESLint
never complains about this, I think because we've turned on that option in the
config.

If we do this though then we should do it in other files as well, in that case
as a separate commit.

+    chrome.runtime = {};
+
+  chrome.runtime.sendMessage = (message, responseCallback) =>
+  {
+    let messageId = ++maxMessageId;
 
-      if (responseCallback)
+    ext.backgroundPage._sendRawMessage({
+      type: "message",
+      messageId,
+      payload: message
+    });
+
+    if (responseCallback)
+    {
+      let callbackWrapper = event =>

[Manish Jethani, 2017/10/11 15:42:41]

Just modernized the code a bit, using arrow functions and dropping the false
argument to addEventListener.

       {
-        let callbackWrapper = function(event)
+        if (event.data.type == "response" && event.data.messageId == messageId)
         {
-          if (event.data.type == "response" &&
-              event.data.messageId == messageId)
-          {
-            window.removeEventListener("message", callbackWrapper, false);
-            responseCallback(event.data.payload);
-          }
-        };
-        window.addEventListener("message", callbackWrapper, false);
-      }
+          window.removeEventListener("message", callbackWrapper);
+          responseCallback(event.data.payload);
+        }
+      };
+
+      window.addEventListener("message", callbackWrapper);
     }
   };
 }());