513944 - Weave should not load / do anything until it absolutely needs to

Status: RESOLVED FIXED

(Cloud Services :: General, defect, P1)

Description

[Dan Mills [:thunder]]

In order to make sure Weave has minimal impact on Ts, it should load/initialize/do an initial sync as late as possible.

Beware: Weave tracks changes in tracker objects.  We want those to initialize late too because they often require registering an observer object with another component (e.g. Places), thus forcing it to initialize as well.  But at the same time, not tracking changes might lead to dataloss.

Places sends out a notification when it initializes, something we might listen for.

Comment 1

[Mike Connor [:mconnor]]

Attachment: delays init until all tabs are loaded, cleans some stuff up

UI pieces (i.e. about:weave) will need to check if Weave.Service.status.service == STATUS_DELAYED before trying to do anything.

Comment 2

[Ed Lee :Mardak]

Comment on attachment 402465 [details] [diff] [review]
delays init until all tabs are loaded, cleans some stuff up

>+++ b/source/modules/service.js
>@@ -289,14 +289,67 @@
>+  // make sure we're not still loading a bunch of tabs.
>+  _readyForStartup: function WeaveSvc__readyForStartup() {
>+    switch (Svc.AppInfo.ID) {
>+      case FENNEC_ID:
>+        return true; // FIXME
I don't think fennec has session restore right now, and "sync on idle" needs to wait for the screen to go dim. But I suppose people running in to issues on Firefox startup aren't actually syncing but auto-logging-in.

>+      case FIREFOX_ID:
>+          if (tabbrowser.mIsBusy)
>+            return false;
Is this just to short circuit the looping over tabs? mIsBusy is only for the currently selected tab, no?

>+          for (let i = 0;i < tabbrowser.mTabs.length;i++)
>+            if (tabbrowser.mTabs[i].hasAttribute("busy"))
>+              return false;
This potentially would always return false on some slow loading page or user activity way past startup. A tab is marked busy anytime it has network activity, so does that include any iframes or continuous connections?

After looking at the patch, I'm thinking maybe we can just have a function that return a number of seconds to delay startup (once). So for Firefox, it can count the number of busy tabs across all windows and wait... a second for each. Maybe even cap it on both ends with a minimum of 5 seconds and max of a minute. ?

Comment 3

[Ed Lee :Mardak]

(In reply to comment #0)
> Places sends out a notification when it initializes
places-init-complete gets sent before sessionstore-windows-restored, so Weave is already triggering on the later notification. If we're okay with missing out on potentially some early-in-session page visits and/or bookmark changes, etc. we can arbitrarily delay ourselves some seconds.

Comment 4

[Ed Lee :Mardak]

http://hg.mozilla.org/labs/weave/rev/97b125ac104d
Land some initial statusbar UI bits of bug 513944  and remove unused/debug code. 

Basically everything except service.js changes with some additional code removal + add comments.

Comment 5

[Ed Lee :Mardak]

http://hg.mozilla.org/labs/weave/rev/c7116cf1b7db
Weave already triggers on a late notification and puts itself on the event loop, so just additionally delay startup based on the number of open tabs (which will all be busy at startup).