In an email conversation with rnewman, he recommended: "[The Sync code] in HG is a snapshot, so it makes sense to move out of mobile. But Sync alone makes up something like 25% of Fennec's total code size, and it's de facto a major module, so it seems weird for it to end up crunched into a subdirectory (that's not /services/, at least). ... if you're thinking about changes that involve moving third party code out of mobile/ and into other directories, which will involve a bunch of Makefile changes, I'd be inclined to see if we can get to a situation where: * Sync moves to a more sane place, like services/android/ (perhaps with subdirs for {sync,aitc,notifications,persona,deps}) * We consider taking the opportunity to move to a structure that matches the Git layout more closely (or even 100%).

CCing folks who might have thoughts on this. gps, mconnor: note that this would leave us with: services/ aitc/ android/ << NEW ... all kinds of AITC/Sync/Persona code common/ crypto/ notifications/ sync/ which doesn't seem ideal, but then the alternative is for chunks of Services Integration code to end up scattered widely around the tree. I also think it's better than living in mobile/android/base/sync. [[ I'm also killing Target Milestone, because we track that for landings not targets (I know, I know). Would someone with Bugzilla magic privs please set tracking-fennec to 17+? ]]

+1

In general, +1 to keeping Services Integration code in one place. Cultural concern: I have heard that ServInt might function as an internal consultancy team, with ServInt devs embedded into different teams to help move Fx toward a Services-aware future. If this is a long term goal, does separating ServInt code erect walls and/or assert code ownership counter to that goal? Engineering concern: integration code will likely have tendrils into several parts of the greater code base. How has that worked (with Desktop Sync) in the past? With AITC? Brass tacks: Java cares about path names. How does this affect building Fennec? Any hope for building a separate android-sync JAR that is linked into the Fennec APK? I would love to make it faster to actually build Fennec, and having a separate sync build target that could be updated in the built APK (== signed aligned zip file) might reduce build times.

(In reply to Nick Alexander :nalexander from comment #3) > Cultural concern: I have heard that ServInt might function as an internal > consultancy team, with ServInt devs embedded into different teams to help > move Fx toward a Services-aware future. If this is a long term goal, does > separating ServInt code erect walls and/or assert code ownership counter to > that goal? Meh. This is more about consolidating related code for interaction with external services than it is trying to erect walls around team boundaries. We're all big one happy family. This isn't a large corporate enterprise where such political games are necessary. > Engineering concern: integration code will likely have tendrils into several > parts of the greater code base. How has that worked (with Desktop Sync) in > the past? With AITC? Services code is typically thought of as middleware. It's a standalone library with tentacles coming out in every direction. It can't belong elsewhere because that would be everywhere. The only important split is the divide between backend service and frontend interaction. Things in services/ are today currently backendy only. But, there's nothing that says we can't put full apps (like Android Sync) there, if warranted. > Brass tacks: Java cares about path names. How does this affect building > Fennec? Any hope for building a separate android-sync JAR that is linked > into the Fennec APK? I would love to make it faster to actually build > Fennec, and having a separate sync build target that could be updated in the > built APK (== signed aligned zip file) might reduce build times. Patches welcome. Seriously, I doubt you will find someone else willing to do this legwork for you, unfortunately.

(In reply to Nick Alexander :nalexander from comment #3) > Cultural concern: I have heard that ServInt might function as an internal > consultancy team, with ServInt devs embedded into different teams to help > move Fx toward a Services-aware future. If this is a long term goal, does > separating ServInt code erect walls and/or assert code ownership counter to > that goal? > > Engineering concern: integration code will likely have tendrils into several > parts of the greater code base. How has that worked (with Desktop Sync) in > the past? With AITC? > > Brass tacks: Java cares about path names. How does this affect building > Fennec? Any hope for building a separate android-sync JAR that is linked > into the Fennec APK? I would love to make it faster to actually build > Fennec, and having a separate sync build target that could be updated in the > built APK (== signed aligned zip file) might reduce build times. All these issues might be addressed if the Android Sync code was simply consolidated into a directory like mobile/android/sync. Sync is part of Fennec, but it is a distinct module. Actually, mobile/android/sync already exists, but it only contains some .mn manifest files right now.

(In reply to Chris Peterson (:cpeterson) from comment #5) > > Brass tacks: Java cares about path names. How does this affect building > > Fennec? Any hope for building a separate android-sync JAR that is linked > > into the Fennec APK? I would love to make it faster to actually build > > Fennec, and having a separate sync build target that could be updated in the > > built APK (== signed aligned zip file) might reduce build times. I'm not particularly swayed about the Java path issue; source files can go pretty much anywhere. We do that already. My first avenue for Fennec integration was actually to attempt to combine two JARs or APKs, and it didn't go so well (they need to be built together in order for Android resources to work), but interested parties are welcome to try again under less time pressure! > Sync is part of Fennec, but it is a distinct module. Yeah, the question is what's the best place for code that's currently primarily used in Fennec, but is a distinct module and might be part of other products in the future. > Actually, mobile/android/sync already exists, but it only contains some .mn > manifest files right now. At the time we decided that source could/should go in base, and manifests would go elsewhere.

rnewman and I discussed deprecating the android-sync github repo today. The motivation is to reduce development process overhead. (For example, back-porting contributor patches from m-c to a-s.) We agree on a few requirements: * being able to run JUnit 3 and JUnit 4 test suites from m-c locally; * being able to rev ch.boye and other third party libraries easily; * keeping background services "pure" enough that other potential consumers (partners, etc) could integrate the existing android-sync code base into their products reasonably. This ticket could form part of that work, since the first step is likely populating mobile/android/services and making the android-sync code a functioning Gradle-style project.

Bug 773050 - Decouple base and services: TabReceivedService. r?mcomella This does a few things: * Move TabReceivedService out of org.mozilla.gecko.sync; * Add an <intent-filter>, so the service isn't referred to by name in the command processor; * Use the existing Intent.EXTRA_TITLE instead of a custom field.

Bug 773050 - Decouple base and services: SKIP_TAB_QUEUE_FLAG. r?mcomella This just moves a shared flag out of base and into the shared BrowserContract to be consumed by services.

Bug 773050 - Decouple base and services: reflect BrowserLocaleManager. r?rnewman There are two pieces to this that are undesireable: * using reflection to access BrowserLocaleManager; * stuffing the contracts provided by base and consumed by services into the constants.jar. Expedience.

Bug 773050 - Build services.jar to consume in gecko-browser.jar. r?rnewman

Comment on attachment 8679185 [details] MozReview Request: Bug 773050 - Decouple base and services: reflect BrowserLocaleManager. r?rnewman https://reviewboard.mozilla.org/r/23385/#review20917 This is fine, but I think a small amount of extra shuffling can eliminate the need for reflection here. s/constants/fundamentals and put BLM into fundamentals. ::: mobile/android/base/Locales.java:30 (Diff revision 1) > + final Class<?> clazz = Class.forName("org.mozilla.gecko.BrowserLocaleManager"); BLM's only significant dependency is on GeckoJarReader, and that's only for getPackagedLocaleTags, which we only use from preferences UI code, and thus wouldn't be hard to tease out. Can we move BLM itself into a precursor package? ('constants'!) It's very glue-ey. ::: mobile/android/base/moz.build:15 (Diff revision 1) > constants_jar.sources = [ "constants" is now a pretty bad name… presumably fixed in a later commit.

Comment on attachment 8679186 [details] MozReview Request: Bug 773050 - Build services.jar to consume in gecko-browser.jar. r?rnewman https://reviewboard.mozilla.org/r/23387/#review20919 ::: mobile/android/base/background/healthreport/HealthReportDatabaseStorage.java:478 (Diff revision 1) > + @SuppressWarnings("fallthrough") Trailing whitespace (throughout the entire diff).

Comment on attachment 8679185 [details] MozReview Request: Bug 773050 - Decouple base and services: reflect BrowserLocaleManager. r?rnewman r+. See notes on the RB req.

Comment on attachment 8679183 [details] MozReview Request: Bug 773050 - Decouple base and services: TabReceivedService. r?mcomella Bug 773050 - Decouple base and services: TabReceivedService. r?mcomella This does a few things: * Move TabReceivedService out of org.mozilla.gecko.sync; * Use the existing Intent.EXTRA_TITLE instead of a custom field; * Refer to the service by name in the command processor, to break a compile time dependency. We'd like a static check that the service was launched but I don't have a good pattern for this across module boundaries yet.

Comment on attachment 8679184 [details] MozReview Request: Bug 773050 - Decouple base and services: SKIP_TAB_QUEUE_FLAG. r?mcomella Bug 773050 - Decouple base and services: SKIP_TAB_QUEUE_FLAG. r?mcomella This just moves a shared flag out of base and into the shared BrowserContract to be consumed by services.

Comment on attachment 8679185 [details] MozReview Request: Bug 773050 - Decouple base and services: reflect BrowserLocaleManager. r?rnewman Bug 773050 - Decouple base and services: reflect BrowserLocaleManager. r?rnewman There are two pieces to this that are undesireable: * using reflection to access BrowserLocaleManager; * stuffing the contracts provided by base and consumed by services into the constants.jar. Expedience.

Comment on attachment 8679186 [details] MozReview Request: Bug 773050 - Build services.jar to consume in gecko-browser.jar. r?rnewman Bug 773050 - Build services.jar to consume in gecko-browser.jar. r?rnewman

Comment on attachment 8679185 [details] MozReview Request: Bug 773050 - Decouple base and services: reflect BrowserLocaleManager. r?rnewman https://reviewboard.mozilla.org/r/23385/#review20975

Comment on attachment 8679183 [details] MozReview Request: Bug 773050 - Decouple base and services: TabReceivedService. r?mcomella https://reviewboard.mozilla.org/r/23381/#review21213 ::: mobile/android/base/db/BrowserContract.java:492 (Diff revision 2) > + public static final String TAB_RECEIVED_SERVICE_CLASS_NAME = "org.mozilla.gecko.tabqueue.TabReceivedService"; This seems really fragile. Is there any plan to improve this (e.g. preprocessing)? ::: mobile/android/base/sync/CommandProcessor.java:259 (Diff revision 2) > - context.startService(sendTabNotificationIntent); > + final ComponentName componentName = context.startService(sendTabNotificationIntent); Unused variable? ::: mobile/android/base/tabqueue/TabReceivedService.java:5 (Diff revision 2) > -package org.mozilla.gecko.sync; > +package org.mozilla.gecko.tabqueue; tabqueue still seems like a weird place to put this but with bug 1214332, wfm.

Comment on attachment 8679183 [details] MozReview Request: Bug 773050 - Decouple base and services: TabReceivedService. r?mcomella https://reviewboard.mozilla.org/r/23381/#review21219 > We'd like a static check that the service was launched but I don't > have a good pattern for this across module boundaries yet. What do you mean by this? What are you going to use it for?

Comment on attachment 8679184 [details] MozReview Request: Bug 773050 - Decouple base and services: SKIP_TAB_QUEUE_FLAG. r?mcomella https://reviewboard.mozilla.org/r/23383/#review21221

https://hg.mozilla.org/integration/fx-team/rev/e904bde0c73db52ec863f02f6136a7ee9ccb2850 Bug 773050 - Decouple base and services: TabReceivedService. r=mcomella https://hg.mozilla.org/integration/fx-team/rev/fb54de74ddc0ebdca100b3581426a4a8121cb499 Bug 773050 - Decouple base and services: SKIP_TAB_QUEUE_FLAG. r=mcomella https://hg.mozilla.org/integration/fx-team/rev/13c459f1f8a74dbe13b6202758a8b8ec16980fff Bug 773050 - Decouple base and services: reflect BrowserLocaleManager. r=rnewman https://hg.mozilla.org/integration/fx-team/rev/36f84d4539503b3f6848f8ab49ea98b02dfef960 Bug 773050 - Build services.jar to consume in gecko-browser.jar. r=rnewman

https://hg.mozilla.org/mozilla-central/rev/e904bde0c73d https://hg.mozilla.org/mozilla-central/rev/fb54de74ddc0 https://hg.mozilla.org/mozilla-central/rev/13c459f1f8a7 https://hg.mozilla.org/mozilla-central/rev/36f84d453950