1229352 - Make Loop fully restartless capable

Status: RESOLVED INCOMPLETE

(Hello (Loop) :: Client, defect, P2)

Description

[Mark Banner (:standard8)]

Currently Loop as a system add-on isn't fully restartless.

We need to be unloading the loop-panel when the add-on is shutdown, so that we drop the references to the backend, and that allows the new versions of the backend modules to be reloaded.

Comment 1

[Mark Banner (:standard8)]

Rob, do you know when the latest we should aim for this is?

Comment 2

[Robert Helmer [:rhelmer]]

(In reply to Mark Banner (:standard8) from comment #1)
> Rob, do you know when the latest we should aim for this is?

The other missing piece for delivering restartless updates is to make sure we don't interrupt an ongoing call, or other important task.

Did you want to handle that in this bug, or just unloading the loop-panel?

Comment 3

[Mark Banner (:standard8)]

(In reply to Robert Helmer [:rhelmer] from comment #2)
> (In reply to Mark Banner (:standard8) from comment #1)
> > Rob, do you know when the latest we should aim for this is?
> 
> The other missing piece for delivering restartless updates is to make sure
> we don't interrupt an ongoing call, or other important task.
> 
> Did you want to handle that in this bug, or just unloading the loop-panel?

Presumably there will be core parts to the interrupting a call - how about doing those as a separate bug, and we'll do the rest in this one?

Comment 4

[Robert Helmer [:rhelmer]]

(In reply to Mark Banner (:standard8) from comment #3)
> (In reply to Robert Helmer [:rhelmer] from comment #2)
> > (In reply to Mark Banner (:standard8) from comment #1)
> > > Rob, do you know when the latest we should aim for this is?
> > 
> > The other missing piece for delivering restartless updates is to make sure
> > we don't interrupt an ongoing call, or other important task.
> > 
> > Did you want to handle that in this bug, or just unloading the loop-panel?
> 
> Presumably there will be core parts to the interrupting a call - how about
> doing those as a separate bug, and we'll do the rest in this one?

OK I filed bug 1231172 to figure out the add-ons manager side of this.

Comment 5

[Mark Banner (:standard8)]

Attachment: WIP - Make the Loop panel unloadable by the system add-on so that it can be restartless

Comment 6

[Mark Banner (:standard8)]

Attachment: WIP - Enable PanelFrames to be destroyed easily (Social API patch)

This would allow the panel frames to be easily destroyed. Social API patch for central, needs a test.

Comment 7

[Mark Banner (:standard8)]

Attachment: Unload the panel and more modules when the extension is uninstalled (patch against Loop repo)

Patch for the Loop repo - this does the necessary to cause the unload of the panel (if supported in the version of FF), and unload extra modules.

At this stage, I think we'll deal with conversation window closing / pausing restarts in a separate bug (we need bug 1231172 to be fixed).

We still need some manual verification here to ensure that when an add-on is restarted, then everything is loaded correctly. I think we could test this by:

- Create two versions of the add-on, both with this fix.
- The second version should have some changes visually, e.g. add extra log output in MozLoopService, change the colour of something on the panel.
- Startup FF, install the unmodified version. Restart FF.
- Install the second version, check the modifications show up correctly.

Comment 8

[Robert Helmer [:rhelmer]]

Basic support for this has landed in bug 1231172.

We'll have MDN docs for this feature soon, but basically it works like this:
```
AddonManager.addUpgradeListener(instanceID, upgrade => {
  upgrade.install();
});
```

You may hang on to the `upgrade` object for later, or simply never call `install()` on it in which case the upgrade will be installed on next restart.

Note that system add-ons are special-cased right now to always require restart, bug 1204156 will remove that and make them install just as normal add-ons do, so you have until that bug ships to implement this if you want to delay silent background updates :)

Comment 9

[Robert Helmer [:rhelmer]]

(In reply to Robert Helmer [:rhelmer] from comment #8)
> We'll have MDN docs for this feature soon, but basically it works like this:
> ```
> AddonManager.addUpgradeListener(instanceID, upgrade => {
>   upgrade.install();
> });
> ```

Oh, and `instanceID` is passed to bootstrap `startup()` on the data object:

```
function startup(data, reason) {
  if (data.hasOwnProperty("instanceID") && data.instanceID) {
    // data.instanceID is a Symbol that is only accessible by this add-on
  }
}
```

Since `instanceID` is a symbol you cannot serialize it, you must keep a reference to it.

Comment 10

[Mark Banner (:standard8)]

(In reply to Robert Helmer [:rhelmer] from comment #8)
> Note that system add-ons are special-cased right now to always require
> restart, bug 1204156 will remove that and make them install just as normal
> add-ons do, so you have until that bug ships to implement this if you want
> to delay silent background updates :)

At the moment for Hello its not just delaying silent updates that affects us. We're missing some vital parts that mean reloading the add-on will likely break the UI.

Therefore I think this blocks bug 1204156 at the moment.

Some of our newer code is already being written with restartless in mind, but I think we still need to fix some parts, especially the panel. I'm not sure if we'll get to this for the next few weeks.

Comment 11

[Mark Banner (:standard8)]

Support for Hello/Loop has been discontinued.

https://support.mozilla.org/kb/hello-status

Hence closing the old bugs. Thank you for your support.