1674908 - Refactor: too many clones of executeSoon()

Status: NEW

(Core :: XPCOM, task, P5)

Description

[Andrey Bienkowski]

There are several functions called executeSoon(). Some are identical or nearly identical. I understand that some of them are special for a reason, but others are just copy&paste. I can do the grunt work of unifying the ones that are identical.

devtools/shared/DevToolsUtils.js#46

exports.executeSoon = function(fn) {
  if (isWorker) {
    setImmediate(fn);
  } else {
    let executor;
    // Only enable async stack reporting when DEBUG_JS_MODULES is set
    // (customized local builds) to avoid a performance penalty.
    if (AppConstants.DEBUG_JS_MODULES || flags.testing) {
      const stack = getStack();
      executor = () => {
        callFunctionWithAsyncStack(fn, stack, "DevToolsUtils.executeSoon");
      };
    } else {
      executor = fn;
    }
    Services.tm.dispatchToMainThread({
      run: exports.makeInfallible(executor),
    });
  }
};

dom/indexedDB/test/helpers.js#27
dom/localstorage/test/helpers.js#9

function executeSoon(aFun) {
  SpecialPowers.Services.tm.dispatchToMainThread({
    run() {
      aFun();
    },
  });
}

js/xpconnect/tests/unit/head_ongc.js#24
testing/mochitest/browser-test.js#1424
testing/modules/TestUtils.jsm#27

function executeSoon(f) {
  Services.tm.dispatchToMainThread({ run: f });
}

testing/mochitest/tests/SimpleTest/SimpleTest.js#1411

SimpleTest.executeSoon = function(aFunc) {
  if ("SpecialPowers" in window) {
    return SpecialPowers.executeSoon(aFunc, window);
  }
  setTimeout(aFunc, 0);
  return null; // Avoid warning.
};

js/xpconnect/tests/unit/head_watchdog.js#53

function executeSoon(fn) {
  var tm = Cc["@mozilla.org/thread-manager;1"].getService(Ci.nsIThreadManager);
  tm.dispatchToMainThread({run: fn});
}

testing/marionette/sync.js#45
remote/Sync.jsm#87

function executeSoon(func) {
  if (typeof func != "function") {
    throw new TypeError();
  }

  Services.tm.dispatchToMainThread(func);
}

testing/specialpowers/content/SpecialPowersChild.jsm#1397

  // The optional aWin parameter allows the caller to specify a given window in
  // whose scope the runnable should be dispatched. If aFun throws, the
  // exception will be reported to aWin.
  executeSoon(aFun, aWin) {
    // Create the runnable in the scope of aWin to avoid running into COWs.
    var runnable = {};
    if (aWin) {
      runnable = Cu.createObjectIn(aWin);
    }
    runnable.run = aFun;
    Cu.dispatch(runnable, aWin);
  }

testing/xpcshell/head.js#761

function executeSoon(callback, aName) {
  let funcName = aName ? aName : callback.name;
  do_test_pending(funcName);

  _Services.tm.dispatchToMainThread({
    run() {
      try {
        callback();
      } catch (e) {
        // do_check failures are already logged and set _quit to true and throw
        // NS_ERROR_ABORT. If both of those are true it is likely this exception
        // has already been logged so there is no need to log it again. It's
        // possible that this will mask an NS_ERROR_ABORT that happens after a
        // do_check failure though.
        if (!_quit || e.result != Cr.NS_ERROR_ABORT) {
          let stack = e.stack ? _format_stack(e.stack) : null;
          _testLogger.testStatus(
            _TEST_NAME,
            funcName,
            "FAIL",
            "PASS",
            _exception_message(e),
            stack
          );
          _do_quit();
        }
      } finally {
        do_test_finished(funcName);
      }
    },
  });
}

dom/indexedDB/test/helpers.js#478
dom/quota/test/mochitest/helpers.js#232

  self.executeSoon = function(_fun_) {
    var channel = new MessageChannel();
    channel.port1.postMessage("");
    channel.port2.onmessage = function(event) {
      _fun_();
    };
  };

Comment 1

[Andrey Bienkowski]

Assuming SpecialPowers.Services is just a different name for Services I can replace

https://searchfox.org/mozilla-central/rev/16d30bafd4e5276d6d3c632fb52a6c71e739cc44/dom/indexedDB/test/helpers.js#27
https://searchfox.org/mozilla-central/rev/16d30bafd4e5276d6d3c632fb52a6c71e739cc44/dom/localstorage/test/helpers.js#9
https://searchfox.org/mozilla-central/rev/16d30bafd4e5276d6d3c632fb52a6c71e739cc44/js/xpconnect/tests/unit/head_ongc.js#24
https://searchfox.org/mozilla-central/rev/16d30bafd4e5276d6d3c632fb52a6c71e739cc44/testing/mochitest/browser-test.js#1424
https://searchfox.org/mozilla-central/rev/16d30bafd4e5276d6d3c632fb52a6c71e739cc44/testing/modules/TestUtils.jsm#27
https://searchfox.org/mozilla-central/rev/16d30bafd4e5276d6d3c632fb52a6c71e739cc44/testing/marionette/sync.js#45
https://searchfox.org/mozilla-central/rev/16d30bafd4e5276d6d3c632fb52a6c71e739cc44/remote/Sync.jsm#87

with

function executeSoon(func) {
  const Services = window.Services || SpecialPowers.Services;

  if (typeof func != "function") {
    throw new TypeError();
  }

  Services.tm.dispatchToMainThread(func);
}

I am not sure about

https://searchfox.org/mozilla-central/rev/16d30bafd4e5276d6d3c632fb52a6c71e739cc44/js/xpconnect/tests/unit/head_watchdog.js#53

function executeSoon(fn) {
  var tm = Cc["@mozilla.org/thread-manager;1"].getService(Ci.nsIThreadManager);
  tm.dispatchToMainThread({run: fn});
}

This two are the same
https://searchfox.org/mozilla-central/rev/16d30bafd4e5276d6d3c632fb52a6c71e739cc44/dom/indexedDB/test/helpers.js#478
https://searchfox.org/mozilla-central/rev/16d30bafd4e5276d6d3c632fb52a6c71e739cc44/dom/quota/test/mochitest/helpers.js#232

self.executeSoon = function(_fun_) {
    var channel = new MessageChannel();
    channel.port1.postMessage("");
    channel.port2.onmessage = function(event) {
      _fun_();
    };
  };

I do not know the codebase well enough to know what to do with the rest.

Comment 2

[Andrey Bienkowski]

One question: in which file should the unified executeSoon() live?

Comment 3

[BugBot [:suhaib / :marco/ :calixte]]

The Bugbug bot thinks this bug should belong to the 'Core::XPConnect' component, and is moving the bug to that component. Please revert this change in case you think the bot is wrong.

Comment 4

[Chris Peterson [:cpeterson]]

Moving to XPCOM component.

P5

Comment 5

[BugBot [:suhaib / :marco/ :calixte]]

The bug has a release status flag that shows some version of Firefox is affected, thus it will be considered confirmed.