Created attachment 298370 [details] [review]
data: be consistent between filters and code

When adding the data file I didn't consider that the code
already has filters, so let's be consistent, otherwise
mission-control gets confused. We don't handle channels
with unknown handle type anyway.

Created attachment 298371 [details] [review]
Wait until activated to restore channels

1) We don't want to restore channels when dbus started, if the
   app is not activated
2) Most importantly, we need an existing window or _ensureRoom fails
   misteriously.
   Previously this would work because it would do:
   startup -> activate -> mainloop -> account manager prepared
   -> ensureRoom -> (-> handle channels -> ensureRoom)
   but in the mission control activation the flow is
   startup -> mainloop -> account manager prepared -> handle channels ->
   ensureRoom -> activate

   We need the second ensureRoom to be effective, and to do so we
   must avoid an ineffective ensureRoom before we handle the channels.

Review of attachment 298370 [details] [review]:

OK

Review of attachment 298371 [details] [review]:

LGTM

Review of attachment 298371 [details] [review]:

Sorry, didn't test properly - the patch has the tendency to wipe out the list of saved channels altogether

OK, so I got a minute to look into this - the issue is that with the patch applied, "late" init may be called earlier now, *before* the account manager has been prepared ...
The following works, though I can't say I'm a huge fan of it:

-            this._chatroomManager.lateInit();
+            if (this._accountsMonitor.accountManager.is_prepared(null))
+                this._chatroomManager.lateInit();
+            else
+                this._accountsMonitor.connect('account-manager-prepared',
+                    Lang.bind(this, function() {
+                        this._chatroomManager.lateInit();
+                    }));

Created attachment 298489 [details] [review]
Wait until activated to restore channels

1) We don't want to restore channels when dbus started, if the
   app is not activated
2) Most importantly, we need an existing window or _ensureRoom fails
   misteriously.
   Previously this would work because it would do:
   startup -> activate -> mainloop -> account manager prepared
   -> ensureRoom -> (-> handle channels -> ensureRoom)
   but in the mission control activation the flow is
   startup -> mainloop -> account manager prepared -> handle channels ->
   ensureRoom -> activate

   We need the second ensureRoom to be effective, and to do so we
   must avoid an ineffective ensureRoom before we handle the channels.

How about this one instead?

Review of attachment 298489 [details] [review]:

Not sure, lots of state all over the place. How about sth like:

lateInit: function() {
    if (!this._accountsMonitor.accountManager.is_prepared(null) ||
        !this._app.get_active_window())
        return; // not ready yet!

    [...]
},

then call lateInit() from both places?

Created attachment 298498 [details] [review]
Wait until activated to restore channels

1) We don't want to restore channels when dbus started, if the
   app is not activated
2) Most importantly, we need an existing window or _ensureRoom fails
   misteriously.
   Previously this would work because it would do:
   startup -> activate -> mainloop -> account manager prepared
   -> ensureRoom -> (-> handle channels -> ensureRoom)
   but in the mission control activation the flow is
   startup -> mainloop -> account manager prepared -> handle channels ->
   ensureRoom -> activate

   We need the second ensureRoom to be effective, and to do so we
   must avoid an ineffective ensureRoom before we handle the channels.

Review of attachment 298498 [details] [review]:

::: src/chatroomManager.js
@@ +91,3 @@
+    lateInit: function() {
+        let am = this._accountsMonitor.accountManager;
+        let ready = am.is_prepared(null) &&

Sorry, I didn't actually check that this worked - apparently you do need to pass a valid feature quark like Tp.AccountManager.get_feature_quark_core() here (or do add a _amIsPrepared property)

Created attachment 298599 [details] [review]
Wait until activated to restore channels

1) We don't want to restore channels when dbus started, if the
   app is not activated
2) Most importantly, we need an existing window or _ensureRoom fails
   misteriously.
   Previously this would work because it would do:
   startup -> activate -> mainloop -> account manager prepared
   -> ensureRoom -> (-> handle channels -> ensureRoom)
   but in the mission control activation the flow is
   startup -> mainloop -> account manager prepared -> handle channels ->
   ensureRoom -> activate

   We need the second ensureRoom to be effective, and to do so we
   must avoid an ineffective ensureRoom before we handle the channels.

Review of attachment 298599 [details] [review]:

OK

Attachment 298370 [details] pushed as 0b27b13 - data: be consistent between filters and code
Attachment 298599 [details] pushed as 8f8c685 - Wait until activated to restore channels