feat: Run now installs as temporary add-on

[kumar303]

Fixes #93

There has been a long platform battle to stabilize the reload add-on function. To keep it simple, https://bugzilla.mozilla.org/show_bug.cgi?id=1269889 introduced a new requirement that the add-on must be installed temporarily before it can be reloaded. This patch uses the remote actor to install the add-on temporarily.

It's a bit brutal in that it fails hard if the Firefox is too old to allow temporary installation like this (it landed in 49). I think this approach is probably ok for now -- it's a lot easier to test and maintain!

[rpl]

@kumar303 I'm still working on this review, but here is a couple of issues that I found so far:

• in src/firefox/index.js at line 38 no-remote is currently set to false, is there a reason why it's not set to true? without the -no-remote command line option if I've another Firefox instance opened, the new Firefox instance (which points to the correct Firefox temporary profile with the needed preferences) is not started and a new tab is opened in the existent Firefox instance.
• currently the tcp port used is statically set to 6000, unfortunately if another process (which can be easily another existent Firefox instance) is already listening on that port, we are going to connect to something unexpected (e.g. in my case I had a Firefox Developer Edition already started and listening on the same port and the error logged was misleading, because it didn't recognize that the port was already occupied, but it raises the error about the missing addons actor)

How about one of the following strategies to fix the above issue:

• strategy 1: the client open a listening socket on a random ports (maybe in a defined range, e.g. 6000-7000), once it find a free tcp port, it stop the listening socket and start the Firefox instances with the first free tcp port which has been found
• strategy 2: the client check if the port 6000 is free, if it is not then a "Port 6000 is currently occupied" error (or something similar) is raised and logged

Personally in the long run I would prefer the strategy 1, but in my opinion it is ok even if we choose to use the simpler strategy 2 in this PR (and then introducing the strategy 1 in a follow up).

[rpl]

@kumar303 I'm still experiencing the following issue with the debounce:

• if the editor app creates temporary files before saving the new version in the original file name, even if the file name of the temporary is filtered by the FileFilter, the real change is skipped because it happens in a time slice shorter of the debounce value

In my opinion, the correct behavior here would be to debounce only the changes on files that are not filtered (and anyway any change on filtered files names is not going to reload the addon).

By looking at the code of the watcher.js module, looks like we can fix it with the following minor change (and I tried it locally and my emacs liked it a lot ;-), a.k.a. I've been finally able to change the "manifest.json" in my editor and look and the "about:addons" page refreshing the addon metadata as expected):

diff --git a/src/watcher.js b/src/watcher.js
index 80592ae..6af1635 100644
--- a/src/watcher.js
+++ b/src/watcher.js
@@ -13,11 +13,15 @@ export default function onSourceChange(
   // TODO: For network disks, we would need to add {poll: true}.
   const watcher = new Watchpack();

+  const executeImmediately = true;
+  const debouncedOnChange = debounce(() => {
+    onChange();
+  }, 1000, executeImmediately);
+
   const onFileChange = (filePath) => {
-    proxyFileChanges({artifactsDir, onChange, filePath, shouldWatchFile});
+    proxyFileChanges({artifactsDir, onChange: debouncedOnChange, filePath, shouldWatchFile});
   };
-  const executeImmediately = true;
-  watcher.on('change', debounce(onFileChange, 1000, executeImmediately));
+  watcher.on('change', onFileChange);

   log.debug(`Watching for file changes in ${sourceDir}`);
   watcher.watch([], [sourceDir], Date.now());

how it looks to you?

[rpl]

@kumar303 as discussed over IRC:

the Firefox instance logs are currently very verbose, as an enhancement we can probably:

• turn the Firefox instance messages as debug level logs
• if web-ext is not in verbose mode, warn the user that there could be error messages related to the addon (and that they are visible in the Browser Console or by re-running web-ext in verbose mode)

[kumar303]

Thanks! Here are the new fix-ups:

• If port 6000 is occupied, an error is thrown. I'll follow up with a patch that discovers an open port.
• no-remote is set to true
• debounce is in the correct place now
• firefox stderr is only shown on --verbose

[rpl]

@kumar303 r+

This PR is great improvement, and the new version of "addon reloading" feature is great.

[coveralls]

Changes Unknown when pulling 79c6bf5 on kumar303:temp-install into * on mozilla:master*.