Enabling Sync regresses Ts on all platforms.

RESOLVED FIXED

Status

()

defect
RESOLVED FIXED
9 years ago
9 years ago

People

(Reporter: mconnor, Assigned: mconnor)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

Posted patch first cutSplinter Review
Fennec doesn't have this, so it looks like Firefox is importing things too early.  This patch appears to fix the issue, still needs a bit of cleanup.  We'll be pushing this to Try with Talos coverage and getting results ASAP.
Assignee: nobody → mconnor
Posted patch v1Splinter Review
* Cull gSyncUI.init down to just weave:service:ready and set up the rest of the UI when that fires.
* Default to no "Connect" label in the statusbar until we init the UI (so we don't always show Connect for 5-10 seconds)
* Don't init via browserGlue, just let the Weave.js auto-import happen.
** In browserglue, listen for weave:service:ready and delay autoconnect appropriately. (We should probably subtract whatever delay Weave.js introduces)

The previous version mostly nailed Ts back to where it started, this version should get us there (or close enough).  Waiting on try to prove that, but we should be good to go tomorrow once this has review.
Attachment #465111 - Flags: review?(dolske)
Blocks: 583339
Comment on attachment 465111 [details] [diff] [review]
v1

>+    Services.obs.addObserver(this, "weave:service:ready", true);
...
>+    obs.forEach(function(topic) {
>+      Services.obs.addObserver(self, topic, true);
>+    });

One "hmmm" -- I thought adding weak-reference observers didn't work unless you explicitly added a nsISupportsWeakReference QI. But I guess since there's no QI at all here, you get the xpconnect "yes this object supports all interfaces!" magic, and in turn the weakref-for-js-objects magic also works.
Attachment #465111 - Flags: review?(dolske) → review+
I did in fact add that, and the nsIObserver QI, rather than relying on magic!
http://hg.mozilla.org/mozilla-central/rev/6b5c4e509001
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Depends on: 587036
Still have a small regression on Ts/WinXP

bug 587036 filed to track that.
You need to log in before you can comment on or make changes to this bug.