799536 - Sync services are running on b2g

Status: RESOLVED FIXED

(Firefox OS Graveyard :: General, defect)

Description

[Justin Lebar (not reading bugmail)]

I see e.g.

│  │  │  ├──────83,096 B (00.12%) -- compartment([System Principal], resource://services-sync/identity.js)

These compartments don't appear to be taking up a ton of memory, but we definitely should turn them off.  It's also possible they're using more memory than we think, due to message-manager overhead (i.e., these or other scripts could be causing bug 7980020).

Comment 1

[Justin Lebar (not reading bugmail)]

> (i.e., these or other scripts could be causing bug 7980020).

bug 798002.

That sync is running appears to be intentional.  in b2g/confvars.sh, we have:

> MOZ_SERVICES_SYNC=1

I thought we weren't using sync in b2g v1?

Comment 2

[Justin Lebar (not reading bugmail)]

Attachment: Patch, v1

Comment 3

[Justin Lebar (not reading bugmail)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/0eaac88ab86d

Comment 5

[Andrew Overholt [:overholt]]

Apparently we're not doing sync in v1.

Comment 6

[Justin Lebar (not reading bugmail)]

I'll land in Aurora once I get some greenage on m-i.

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/0eaac88ab86d

Comment 8

[Justin Lebar (not reading bugmail)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/08abc3599a33

We don't currently have bugzilla flags to indicate fixed-on-aurora, but we'll have them on Thursday.

Comment 9

[Jonathan Griffin (:jgriffin)]

This change breaks Marionette because we use log4moz.js.

Comment 10

[Gregory Szorc [:gps]]

Comment on attachment 669599 [details] [diff] [review]
Patch, v1

Review of attachment 669599 [details] [diff] [review]:
-----------------------------------------------------------------

::: b2g/confvars.sh
@@ +16,5 @@
>  MOZ_OFFICIAL_BRANDING_DIRECTORY=b2g/branding/official
>  # MOZ_APP_DISPLAYNAME is set by branding/configure.sh
>  
>  MOZ_SAFE_BROWSING=
> +MOZ_SERVICES_SYNC=

For future reference, you should just delete the variable wholesale.

Comment 11

[Justin Lebar (not reading bugmail)]

(In reply to Jonathan Griffin (:jgriffin) from comment #9)
> This change breaks Marionette because we use log4moz.js.

Not to be glib, but it seems like a bug in Marionette that it relies on a module from sync.

Anyway, I'm happy for you to back this out while you fix Marionette, so long as you'll put it back in once the dependency is resolved.

Comment 12

[Justin Lebar (not reading bugmail)]

> For future reference, you should just delete the variable wholesale.

Should I also delete the other variables?  Or are there circumstances where "FOO=" is the right thing to do?

Comment 13

[Gregory Szorc [:gps]]

Marionette does not rely on Sync. There are toolkit-like files in service/common (like log4moz.js) that Marionette relies on. The build system isn't smart enough to differentiate between services/common and services/sync. I'll code that up in bug 800154.

(In reply to Justin Lebar [:jlebar] from comment #12)
> > For future reference, you should just delete the variable wholesale.
> 
> Should I also delete the other variables?  Or are there circumstances where
> "FOO=" is the right thing to do?

It's a style thing. The default value for an unknown variable is undefined. It's safer to just not define a variable than to define one with an empty value, IMO.