573899 - Only Clients gets synced

Status: RESOLVED FIXED

(Firefox :: Sync, defect)

Description

[Ed Lee :Mardak]

Seems like other engines start syncing after a few restarts. Probably related to bug 572436.

Comment 1

[Philipp von Weitershausen [:philikon]]

Attachment: v1

The problem occurs for me when the Sync add-on is added to a profile for the first time. It goes away on subsequent starts. I'm guessing we're racing add-on setup here. The attached patch fixes this problem by making the call to Weave.Service.onStartup async.

Comment 2

[Philipp von Weitershausen [:philikon]]

Attachment: v2

Even better, since Weave.Service.onStartup is now async, we can go through Weave.Svc in the overlays and don't have to be cautious about avoiding the 'Weave' namespace.

Comment 3

[Ed Lee :Mardak]

Comment on attachment 453378 [details] [diff] [review]
v2

A bunch of services tests are failing

Comment 4

[Ed Lee :Mardak]

Is there some way to do a chrome.manifest override for the default prefs so that we can just package them as part of prefs instead of setting them in js?

Comment 5

[Philipp von Weitershausen [:philikon]]

Attachment: fix up tests

Fix up tests that depend on Weave.Service.onStartup being run.

Comment 6

[Philipp von Weitershausen [:philikon]]

Comment on attachment 453378 [details] [diff] [review]
v2

Instead of changing the semantics of importing service.js, it would be more sensible to set the default preferences in an add-on specific component.

Comment 7

[Ed Lee :Mardak]

Hrm.. I was about to suggest that from the component, we could check if the pref isSet, and if not, set it to some default value. However, during setup, we might clear the prefbranch and kill the value from .startOver().

Comment 8

[Philipp von Weitershausen [:philikon]]

Attachment: Set default prefs in addon-specific component

Comment 9

[Philipp von Weitershausen [:philikon]]

Comment on attachment 453745 [details] [diff] [review]
Set default prefs in addon-specific component

Hrm, hit Enter too soon...

Anyway, this patch moves the default prefs from the overlay code to a separate, add-on specific component.

Comment 10

[Philipp von Weitershausen [:philikon]]

(In reply to comment #7)
> Hrm.. I was about to suggest that from the component, we could check if the
> pref isSet, and if not, set it to some default value. However, during setup, we
> might clear the prefbranch and kill the value from .startOver().

None of this is an issue I think since we're setting a *default* pref, not a user pref.

Comment 11

[Igor Velkov]

Please, add into glue case branch

      case SEAMONKEY_ID:
        engines = ["Form", "History", "Password"];
        break;

Comment 12

[Ed Lee :Mardak]

The previous code has form history password and tabs for seamonkey:

http://hg.mozilla.org/services/fx-sync/rev/e5bce5ae1712#l3.18

Comment 13

[Philipp von Weitershausen [:philikon]]

Apparently tab sync doesn't quite work yet in SeaMonkey (bug 567583) and it's unclear to me what other engines work and don't work, so I left it out for now. I'll be happy to put it back in, but perhaps it'd be better if the SeaMonkey folks would do it themselves?

Comment 14

[Philipp von Weitershausen [:philikon]]

Attachment: Set default prefs in addon-specific component (v1.1)

Add case SEAMONKEY_ID as suggested by Igor (he confirmed that those three engines indeed work on SeaMonkey).

Comment 15

[Ed Lee :Mardak]

http://hg.mozilla.org/services/fx-sync/rev/177ed7233022
Setting default preferences from overlay code has proven to be too unreliable. Structurally it's also misplaced since it's neither UI nor library code. So instead set the default prefs in a separate, add-on specific component.