Make safe for other listeners

piroor · May 1, 2019 · df95d0b · df95d0b
1 parent 9cad91b
commit df95d0b
Show file tree

Hide file tree

Showing 3 changed files with 19 additions and 4 deletions.
diff --git a/webextensions/background/background.js b/webextensions/background/background.js
@@ -159,8 +159,8 @@ async function notifyReadyToSidebars() {
   log('notifyReadyToSidebars: start');
   const promisedResults = [];
   for (const window of TabsStore.windows.values()) {
-    // Send PING to all windows whether they are detected as "opened" or "not opened yet",
-    // because the connection may be established before this background page starts listening
+    // Send PING to all windows whether they are detected as opened or not, because
+    // the connection may be established before this background page starts listening
     // of messages from sidebar pages.
     // See also: https://github.com/piroor/treestyletab/issues/2200
     TabsUpdate.completeLoadingTabs(window.id); // failsafe

diff --git a/webextensions/background/handle-misc.js b/webextensions/background/handle-misc.js
@@ -60,6 +60,13 @@ Tab.onPinned.addListener(tab => {
 
 /* message observer */
 
+// We cannot start listening of messages of browser.runtime.onMessage(External)
+// at here and wait processing until promises are resolved like ApiTabsListener
+// and BackgroundConnection, because making listeners asynchornous (async
+// functions) will break things - those listeners must not return Promise for
+// unneeded cases.
+// See also: https://github.com/piroor/treestyletab/issues/2200
+
 Background.onInit.addListener(() => {
   browser.browserAction.onClicked.addListener(onToolbarButtonClick);
   browser.commands.onCommand.addListener(onShortcutCommand);
@@ -266,6 +273,8 @@ async function onShortcutCommand(command) {
   }
 }
 
+// This must be synchronous and return Promise on demando, to avoid
+// blocking to other listeners.
 function onMessage(message, sender) {
   if (!message ||
       typeof message.type != 'string' ||
@@ -393,6 +402,8 @@ function onMessage(message, sender) {
   }
 }
 
+// This must be synchronous and return Promise on demando, to avoid
+// blocking to other listeners.
 function onMessageExternal(message, sender) {
   //log('onMessageExternal: ', message, sender);
   switch (message.type) {

diff --git a/webextensions/sidebar/sidebar.js b/webextensions/sidebar/sidebar.js
@@ -502,11 +502,12 @@ export async function rebuildAll(importedTabs, cache) {
 
 const mImportedTabs = new Promise((resolve, _reject) => {
   log('preparing mImportedTabs');
-  const onBackgroundIsReady = async message => {
+  // This must be synchronous , to avoid blocking to other listeners.
+  const onBackgroundIsReady = message => {
     // This handler may be called before mTargetWindow is initialized, so
     // we need to wait until it is resolved.
     // See also: https://github.com/piroor/treestyletab/issues/2200
-    const windowId = mTargetWindow || await mPromisedTargetWindow;
+    mPromisedTargetWindow.then(windowId => {
     log(`mImportedTabs (${windowId}): onBackgroundIsReady `, message && message.type, message && message.windowId);
     if (!message ||
         !message.type ||
@@ -516,6 +517,7 @@ const mImportedTabs = new Promise((resolve, _reject) => {
     browser.runtime.onMessage.removeListener(onBackgroundIsReady);
     log(`mImportedTabs is resolved with ${message.tabs.length} tabs`);
     resolve(message.tabs);
+    });
   };
   browser.runtime.onMessage.addListener(onBackgroundIsReady);
 });
@@ -870,6 +872,8 @@ function onConfigChange(changedKey) {
 }
 
 
+// This must be synchronous and return Promise on demando, to avoid
+// blocking to other listeners.
 function onMessage(message, _sender, _respond) {
   if (!message ||
       typeof message.type != 'string' ||