Robustness changes from watching logs in Fennec

[AlexVallat]

I've been testing in Fennec with the changes from a2626a9 and found some cases where errors sometimes occur with restarting Firefox, and toggling enabling and disabling the addon. These change add a little robustness to avoid those errors.

[Deathamns]

I looked into it, and realized that the errors appear when you disable the extension when you view the list of the add-ons, not when disable/enable from the page of the add-on (this is why I didn't see the errors @gorhill saw).
So, the problem is that the button is null when you disable it from the list of the add-ons. Because of that the script will stop there, and won't register the unload event, which performs the cleanup tasks.
So, simply add an if button is null then return check, instead of unregistering the factory, because while that solves the httpobserver issue, it hides the bigger problem.

[AlexVallat]

There is also a possible path where browser here is null, and that causes the same effect of stopping the script prematurely.

I didn't see the button issue you mention because on Fennec you can only disable and enable it from the addon page itself, not from the list of addons (there are no buttons there).

In my opinion, the factory unregistering should stay in any case, because it's a way to recover from any error that causes cleanup to fail. Without it, once cleanup fails, the addon will never work properly again until Firefox is restarted, so I think it's worth having.

If you are concerned about problem hiding, then I think the correct solution to that would be to log the very end of cleanup, and of startup, and if the end doesn't happen then we know something horrible has gone wrong. It would even be possible to set a timeout that if the end hasn't been reached to make some sort of alert that it failed - but possibly only for a debug mode.

[Deathamns]

There is also a possible path where browser here is null

That's okay, I talked about only the factory.

Without it, once cleanup fails, the addon will never work properly again until Firefox is restarted

I'm not sure if it even should work if cleanup fails. Currently the whole background is held back from garbage collection because some references couldn't be removed.
If you add these kind of workarounds just to make it work, a previous version of the background page will still be hanging there, and it may not be that easy to notice it.

[AlexVallat]

I'm not sure if it even should work if cleanup fails.

In that case it should report failure to startup so that the addon system informs the user they have to restart to enable it. The current behaviour of partial broken loading can't be right, though...