706545 - Implement a sync engine for apps exposed by navigator.mozApps

Status: RESOLVED FIXED

(Firefox :: Sync, defect)

Description

[[:fabrice] Fabrice Desré]

We need to be able to sync apps that are installed and uninstalled.

Comment 1

[[:fabrice] Fabrice Desré]

Attachment: patch

Works fine here, but this is my first sync engine so I may have missed something.

Comment 2

[Philipp von Weitershausen [:philikon]]

Comment on attachment 578309 [details] [diff] [review]
patch

This is a great start! Some general notes:

* This needs tests. xpcshell unit tests as well as end-to-end TPS tests. It would be great if you could help us out with the xpcshell unit tests. For the TPS tests I'll ping jgriffin.
* Thanks for tackling the UI pieces. You missed one piece in the setup wizard, though (syncSetup.xul/js).
* It would be good to have separate patches and perhaps even bugs for each of these:
  (a) dom/base/WebApps.*
  (b) services/sync/modules/*
  (c) browser/*

In particular, I think we could land some of the backend code sooner than later, but defer the UI and TPS pieces until later. That way we can do some in-the-field testing and dogfooding by flipping about:config settings, but we won't actually ship anything user-visible.

But, this is also up to the rest of the Sync team and how they want to roll with this. IMHO, having the backend code + unit tests in there would be good because if Webapps.* gets refactored, the Sync code will get refactored along with it. Even if it's not enabled and visible in the UI yet.


>+  /** added to support the sync engine */
>+
>+  getAppFromId: function(aId) {
>+    if (this.webapps[aId]) {
>+      let app = this._cloneAppObject(this.webapps[aId]);
>+      app.manifest = this._readManifest(aId);

Ohgodohgodohgod. You're doing file I/O on the mainthread?!? I filed bug 707090! This stuff is definitely going to fly in Sync because this way, Sync can easily lock up the whole browser on an initial sync (lots of shit to sync).

>diff --git a/services/sync/modules/engines/apps.js b/services/sync/modules/engines/apps.js
>new file mode 100644
>--- /dev/null
>+++ b/services/sync/modules/engines/apps.js

For new files we encourage "use strict";

>+var EXPORTED_SYMBOLS = ['AppsEngine', 'AppRec'];

nit: const

>+let Cu = Components.utils;

You can just do

  const {classes: Cc, interfaces: Ci, utils: Cu, results: Cr} = Components;

and then refer to Ci below.

>+AppStore.prototype = {
>+  __proto__: Store.prototype,
>+
>+  getAllIDs: function _getAllIDs() {

Nit: no underscore in the function name (applies to all other method definitions too).

>+  create: function _create(record) {
>+    DOMApplicationRegistry.confirmInstall({
>+        app: record.value
>+    }, true);

Nit: indention. (Probably don't even need to wrap this line at all...)

>+  remove: function _remove(record) {
>+    DOMApplicationRegistry.uninstallById(record.id);
>+  },
>+
>+  update: function _update(record) {
>+    DOMApplicationRegistry.uninstall(record.value, true);
>+    DOMApplicationRegistry.confirmInstall({
>+        app: record.value
>+    }, true);

ditto.

>+AppTracker.prototype = {
>+  __proto__: Tracker.prototype,
>+  QueryInterface: XPCOMUtils.generateQI([Components.interfaces.nsIObserver]),

Use Ci here.

>+  observe: function(aSubject, aTopic, aData) {
>+    this._log.debug("AppTracker:observe " + aTopic);
>+    switch (aTopic) {
>+      case "webapps-sync-install":
>+      case "webapps-sync-uninstall":
>+        // 1000 points for either changed
>+        this.score = 1000;

Please const this magic number! Though, really, you probably just want to use SCORE_INCREMENT_XLARGE if you want to trigger an Instant Sync here. Also, score updates should always do +=.

(Also, that comment is rather superfluous ;). I can *see* what it's doing... What I can't see is *why* :))

>+        this.modified = true;

This seems unnecessary.

Comment 3

[Bill Walker [:bwalker] [@wfwalker]]

Please do not proceed any further in this direction without getting buy-in from Ragavan, Mike Hanson, and Todd Simpson. The Q4 apps implementation is going in a totally different direction, for some good reasons I'd like to see discussed in more detail.

thanks,
-Bill

Comment 4

[Brendan Eich [:brendan]]

(In reply to Bill Walker from comment #3)
> Please do not proceed any further in this direction without getting buy-in
> from Ragavan, Mike Hanson, and Todd Simpson. The Q4 apps implementation is
> going in a totally different direction, for some good reasons I'd like to
> see discussed in more detail.

What are those reasons?

/be

Comment 5

[Andreas Gal :gal]

Fabrice, please continue with the implementation. I will coordinate with Bill and discuss in parallel whether these two approaches conflict at all. I can't see how they would. Multiple sync providers can sync apps.

Comment 6

[Allison Naaktgeboren :ally]

This will require additions to the TPS engine, in addition to TPS tests themselves. philikon is correct, jgriffin is the person to talk to about that. However, he will not have free cycles for a while yet.

I am not opposed to adding the engine, but I would like to hear more about the project, the motivation, the timeline, security or privacy implications, etc.

Comment 7

[[:fabrice] Fabrice Desré]

(In reply to :Ally Naaktgeboren from comment #6)
> 
> I am not opposed to adding the engine, but I would like to hear more about
> the project, the motivation, the timeline, security or privacy implications,
> etc.

The motivation is that users should have their installed apps synced between firefox desktop, fennec and b2g. We are syncing json objects that are tyically only a few Kbytes, not the apps themselves.
For the timeline, my goal is to get this landed in time for fx13. I'm not aware of any security and privacy issues - I thought using sync would give us these guarantees.

Comment 8

[Richard Newman [:rnewman]]

(In reply to Fabrice Desré [:fabrice] from comment #7)

Just weighing in on this one point:

> I'm not
> aware of any security and privacy issues - I thought using sync would give
> us these guarantees.

Sync gives some guarantees, but the meat of what's being synced needs also to be considered.

We typically have a sec and privacy review for new features like this, including new Sync engines, because it's new information being exchanged or stored. There are often non-obvious leakage opportunities (e.g., for add-ons, a naïve implementation would allow an observer to gain information about the machines you have syncing by watching fetches of addons).

The security team has done a really great job of making this process brief, easy, and constructive, so I don't really consider it something that you should try to avoid.

You can see an example review of a new Sync engine here:

https://wiki.mozilla.org/Security/Reviews/Firefox/AddOnSync

Comment 9

[[:fabrice] Fabrice Desré]

Thanks Richard, I started a similar page for this engine here : https://wiki.mozilla.org/Security/Reviews/Firefox/AppsSync

Comment 10

[Philipp von Weitershausen [:philikon]]

(In reply to Fabrice Desré [:fabrice] from comment #9)
> Thanks Richard, I started a similar page for this engine here :
> https://wiki.mozilla.org/Security/Reviews/Firefox/AppsSync

Awesome. Please schedule a sec review with Curtis & team. This can happen in parallel to writing tests, etc.

Comment 11

[Gregory Szorc [:gps]]

Can you explain a bit on how the IDs are generated? Looking at the existing code, there is a path to generate a new random UUID. This is good. Sync's privacy policy insists that the record IDs be random so even if the set of record IDs is compromised, someone cannot determine what they correlate to based on public knowledge. In other words, the same entity should have different record IDs on different sync accounts. If there is an ID that is common/public, that ID should be inside the encrypted record payload. When the record arrives, the sync engine applies duplication detection to reconcile the different record IDs.

Comment 12

[Philipp von Weitershausen [:philikon]]

(In reply to Gregory Szorc [:gps] from comment #11)
> Can you explain a bit on how the IDs are generated? Looking at the existing
> code, there is a path to generate a new random UUID.

I'm guessing you're referring to https://mxr.mozilla.org/mozilla-central/source/dom/base/Webapps.jsm#175. As far as I can tell and Fabrice has assured me, this means that each installation instance of a webapp gets an entirely new random UUID. Not necessarily our preferred Sync GUID format, but at least it conforms with our existing privacy conventions.

Comment 13

[[:fabrice] Fabrice Desré]

Philipp is right, each new installed application gets a random UUID. If you want me to change it to a Sync GUID format let me know, I have no other constraint on this ID than being able to use it as a safe filename.

Comment 14

[Philipp von Weitershausen [:philikon]]

Sync's minimum requirement is URL safety because GUIDs are part of the URL. The preferred format is 12 characters base64url. See https://mxr.mozilla.org/mozilla-central/source/services/sync/modules/util.js#197

Comment 15

[Bill Walker [:bwalker] [@wfwalker]]

(In reply to Brendan Eich [:brendan] from comment #4)
> (In reply to Bill Walker from comment #3)
> > Please do not proceed any further in this direction without getting buy-in
> > from Ragavan, Mike Hanson, and Todd Simpson. The Q4 apps implementation is
> > going in a totally different direction, for some good reasons I'd like to
> > see discussed in more detail.
> 
> What are those reasons?
> 
> /be

Brendan, Andreas,

Per Todd, I'd like us all to agree on a unified client-facing API (including authentication scheme) for all our appSync implementations. And I don't see any compelling reason to have more than one implementation. We'll set up a meeting as soon as we have email and calendars back.

thanks,
-Bill

Comment 16

[Andreas Gal :gal]

We already have that. Its called Firefox sync, no? I am a bit confused why we are creating a 2nd, independent scheme that doesn't inter-operate with our first, which is in production, available today, without any scheduling risk. Fabrice has a 20 lines or so patch to make it apps-sync-capable. I don't think we can clarifies this in a private meeting. We should take this to dev.platform, since we are basically re-creating a Gecko subsystem.

Comment 17

[Brendan Eich [:brendan]]

This is an open source project. We don't do meetings with (unreliable, apparently) closed calendars and mail servers to make decisions. Dropping names of people without module owner or peer standing is not going to cut any ice with contributors either.

There's a patch in this bug undergoing code review and development. It integrates B2G apps with our existing Firefox Sync service, today. Why should we suspend progress here and wait for a new Sync service that (I'm told) is not yet up, has new dependencies (all of which multiply risk), and has not had security reviews yet? Why not do what's real now and switch to a better system when that comes online?

/be

Comment 18

[Michael Hanson]

This conversation is getting unnecessarily hot - we're all just trying to make sure the right people talk to each other.  Let's unpack the requirements and goals to figure out how we got here.

The original App Sync requirements, which were driven by UX concerns (single sign on, works in any browser) and a principled data retention stance, were:

1. Authenticated using BrowserID
2. Accessible from content in an generic (HTML5ish) browser
3. Immediately available following a login; no additional key exchange needed
4. Implemented using Mozilla User Data principles [1]
5. Available, an an opt-in basis, for app recommendation
6. A guaranteed-persistence contract (as opposed to best effort)

App Sync was identified as a low-risk pilot project for Sauropod [2], which was an attempt to describe a server storage system that encoded the User Data principles.  Note that, as defined, App Sync is not a "Gecko subsystem" - the product definition was that it would work with any browser.

I think, as the Apps project has picked up steam, and the Sync use case has become a hotter corner of it, that "low risk" no longer describes the App Sync project.  As the Apps project has gained speed in the Gecko platform, there is a natural tendency to see it in terms of Gecko features.

Here's my take on a gap analysis from current Firefox Sync:

* Sync currently uses a username/password pair for authentication; we require BrowserID authentication for receipt verification so there's potential for session mismatch.  There is also potential for user confusion about two different authentication steps.  We have steered clear of asking for Sync username/password login in content so this is a security risk.
* Sync's client-based load balancing strategy may increase the difficulty of accessing content from an generic browser (AFAIK, correct me if I'm wrong)
* The Mozilla User Data principles indicate that we always use a user credential for data access; Sync does not currently have this architecture, because of the protection provided by encrypted data.  If we retain app sync data in plaintext (achieving requirement #3), we would have plaintext data without an administrative audit trail. 
* Alternatively, if we encrypt app sync records client-side, the cross-browser implementation is very problematic.  Key exchange is hard, and app recommendation becomes quite a bit harder to do.  Alternatively we move to a user-key-on-the-server architecture, with data that was not designed for that trust topology.  This is especially complicated because we use the same key for low-sensitivity data (apps) and high-sensitivity data (passwords).
* Sync does not provide a guarantee of persistence.  This is potentially a big problem, if it means that we might lose user receipts.  I can imagine a flow where they are restored from one or more marketplaces but that's a big pain for the user.

If we decide to prioritize up B2G app sync, and prioritize down cross-browser app sync, we can do that.  We would need to address #1, #5, and #6 in any case.  We would be accumulating technical debt on the cross-browser implementation (#2 and #3).  One approach may be to create a service that implements the Sauropod API over the Sync backend; this would have critical privacy issues unless app data were stored unencrypted, but storing app data unencrypted may have user-data principle issues (#4).  (Strawman: SauroAPI server performs BrowserID-to-Sync account binding; on data request inspects key and maps to Sync or Sauro backend; is responsible for data coherence between Gecko-only and HTML5-only clients)

AFAICT, there is no product or module owner empowered to make this call which is why we're having a tense discussion, I think.

1. https://wiki.mozilla.org/User_Data_Principles
2. https://wiki.mozilla.org/Sauropod

Comment 19

[Andreas Gal :gal]

This bug is not about the App Sync project. As much as I would love to discuss the technical merits of comment 18, thats really out of scope here.

This bug is also not about B2G.

This bug is about sync support for Gecko, and Gecko-based products, based on the current production quality sync client, and the web apps support we landed in Gecko. We have a patch, and its waiting for review by the responsible module owners of Sync.

As long review and tests are done in time, the patch should be able to ship with Firefox 12 in 3 months.

Independently, we can discuss the architecture proposed above, and implement it. Based on the complexity of this system, I think its fair to say it won't ship any time soon. I understand the current schedule asks for a product in 6 months. Based on my past experience with Gecko and Firefox, and the complexity of this system you describe, I think that number is off by about factor 4. But even if we can pull of a miracle, there is no reason to stop a simple feature for Firefox that will get apps sync in the hands of half a billion Firefox users way before this proposed architecture ships.

If there is any tense discussion here, I think it arises from the attempt to interfere with the open source governance of the Mozilla project. This patch is between Fabrice and the Sync module owners. Asking them to stand down improving Firefox because of some promised future sky castle is not how the Mozilla project operates. Once your promised land has arrived and is production ready, we are happy to land it and switch over to the new world. We all want the best possible product. But we also want it in finite time. Thats what engineering is all about. Delivering what we can as quickly as we can, with stepping stones and partial features along the way.

Comment 20

[Brendan Eich [:brendan]]

> If there is any tense discussion here, I think it arises from the attempt to interfere
> with the open source governance of the Mozilla project.

Yup. Secret reasons, private calendars, face-to-face meetings -- all completely the wrong thing. Anyone who doesn't get this, put yourself in the position of a volunteer.

Mozilla has been doing open development for almost 14 years. We had to fight Netscape on this kind of stuff in the old days. Other open source projects (including some run by competitors who are using asymmetric means against us) imitate our governance structures, but fail to keep the playing field level, notoriously -- and get scored off for it.

We cannot afford to step back from our obligations and standards. I happen to think we should not, and that's a point of agreement in the project leadership. But as a practical matter, we can't afford to -- we rely on too many volunteers for critical work.

/be

Comment 21

[Anant Narayanan [:anant]]

(In reply to Andreas Gal :gal from comment #19)
> This bug is about sync support for Gecko, and Gecko-based products, based on
> the current production quality sync client, and the web apps support we
> landed in Gecko. We have a patch, and its waiting for review by the
> responsible module owners of Sync.

My main fear is that implementing a Gecko specific sync solution goes against one of the most important features of web apps: the ability to run them in any standards compliant browser. If we go ahead with this, all we are doing is syncing "Firefox Apps", not unlike "Chrome Apps" offered by Google; because there is no way for me to then use an app I purchased from something that is not based on Gecko. I don't think it's the Mozilla way.

The alternative proposal in comment 18 addresses that concern. I understand the engineering risk of betting everything on something we haven't even fully built out yet, but I am of the opinion that we should devote our limited time and resources into getting cross browser sync working. Being able to use our existing production-quality Sync infrastructure would be great. Unfortunately, as pointed out by Michael Hanson, that is nearly impossible without surmounting some pretty tough challenges.

Comment 22

[Brendan Eich [:brendan]]

The Mozilla way is not to uplift all browsers at once, especially not via polyfilling fake DOM APIs from served JS (if that's how navigator.apps works in the cross-browser case).

Just extending the DOM with navigator.* signals intent to standardize a cross-browser apps object. App sync is already in Chrome. So starting with Firefox Sync today and evolving both API object and server-side code to support more browsers and other protocols is possible. The API object cannot lock into only one protocol or browser if it's to become a standard.

When you try to do N>3 hard and independent new things at once, the odds ratios multiple down to a small fraction. This is a good way to be late to market, and lose to others who did one thing at a time (yes, this is the WiB arguemnt again).

The Mozilla way to create cross-browser standards is to extend Firefox and Gecko, get users and developers working with the extension, and *then* standardize. In the past we have succeeded at this with JS and DOM APIs, and we're still doing it. It works provided you stay in the market.

The Mozilla way is to get code before users sooner, all else equal, provided there's no fatal flaw in the currently supported service being used. There can't be, or we'd pull the current Sync product, right?

I do not believe anyone wants to stick to the old Sync service when BrowserID and new Sync are up and running well. B2G wants to interoperate with Firefox and ultimately other browsers on apps and app sync. But you can't get there from here and stay in the market (even the early B2G proto-market) without taking the necessary steps, and not all at once in an inter-dependent big jump.

/be

Comment 23

[Brendan Eich [:brendan]]

Just out of curiosity: has anyone looked at Chrome's chrome-app-sync to see whether it could handle open web apps? If navigator.apps becomes a web API standard, Google will likely want it in Chrome to hook up (as 1 of N if not exclusively) to their sync service.

/be

Comment 24

[Brendan Eich [:brendan]]

Sorry, last comment from me till others add theirs:

One more "Mozilla way" point. Putting other browsers *in the future* ahead of B2G and Firefox *now* is deeply wrong. In addition to the risk-multiplying, it leaves us without any minimum viable app-sync story in the "now" time frame (the B2G run-up to MWC).

Contrary to comment 15, there are compelling reasons to avoid multiplying risk and adding delay. We don't know exactly how a cross-browser open web app sync standard might emerge, assuming an open web app standard emerges. But tying our one-and-only approach all together into a new identity system and new user-data storage back end is not obviously the best way to find out. That makes a huge all-or-nothing bet.

Meanwhile, Chrome has apps and app sync. Meeting our shorter-term B2G goals, getting user and developer experience, and then meeting Chrome in the middle by moving this patch forward is much more compelling than waiting.

We would be in a better place to start standardization if we had Gecko-specific app sync with real Firefox and B2G users, than we would coming in later with a total identity-based solution. Protocols evolve in any case.

We are really arguing about releasing early and often, not about conserving scarce resources to make one giant bet that's anti-leveraged for Mozilla's actual users.

/be

Comment 25

[Ragavan S [:rags]]

How do we address #1, #3, #5 and #6 from comment #18 if we use the existing Firefox Sync client/service?

Comment 26

[Brendan Eich [:brendan]]

(In reply to Ragavan S [:rags] from comment #25)
> How do we address #1, #3, #5 and #6 from comment #18 if we use the existing
> Firefox Sync client/service?

You don't, in the near term (#6 may not be materially different from today's Sync -- we shall see). They are addressed, if necessary, later.

Near term = this month, by the 20th if the RR schedule holds. This patch is still being reviewed but should go into the pipe in time for Firefox 12. Any holiday break from the schedule may not provide more time to land.

Later = more than a couple of months. You tell me -- the schedule has been slipping for a while. Big risk-multiplying plans are like that.

When we get new Sync etc. further along, not just prototyped but actually deployed with users, we can switch both Firefox and Gecko's app sync to it. More likely we'll want to support both old and new (and Chrome's protocol too? I don't rule it out, could be good competitively and for standardization).

/be

Comment 27

[Todd Simpson]

I think we have run into a situation with too many moving pieces...and trying to put them all together is causing angst.  So, let's not put them all together.

1. For B2G and Firefox, using Sync to sync Apps seems pragmatic and low risk.  If, at some future point, the Cloud version of a persons Apps list becomes the definitive list, then we can unify the system...and potentially integrate with Chrome's protocol as well.

2. For Apps, the product requirements and usage don't lend themselves to the existing Sync service, so we either have to evolve Sync to meet the requirements of Comment 18, or stand up a new service.  I think we can decouple some parts of this, as Brendan suggests, to mitigate risk.  For example, I think the most basic requirement from Apps is a simple sign-in (BrowserID - requirement #1 from above) and setup process.  I don't think cross-browser is as important as ensuring the system is exceptionally simple for existing Firefox users.


My 2,
Todd

Comment 28

[Ragavan S [:rags]]

Just so I understand what's being proposed in comments #26 and #27.

The Firefox and B2G teams will go ahead and implement App Sync support as a Firefox Sync engine - with the same UX and product characteristics as Firefox Sync (no backup/restore, complicated setup/encryption, no browserid support etc). I don't even know how things like receipt validation would work without BrowserID support, but perhaps you guys have figured all that out? 

The Apps team will continue to build an App Sync service that meets our requirements from comment #18 (whether that's by changing Sync or standing up a new service).

Perhaps sometime in the future (say "Later" as Brendan defines it in #26), these two might merge?

In the meantime, we will have two separate engineering efforts with completely different product and user experience for Apps in Firefox/B2G?

Am I understanding this right?

Comment 29

[Ragavan S [:rags]]

By the way, we (Apps Product and UX) have just started talking with the Firefox Product team (Asa and Chris) on Apps integration in Firefox and are working on some feature pages. Has this feature (Apps Sync in Firefox) been requested by the Firefox Product team and/or the Sync Product team? It didn't come up in our previous chat, so curious to hear the product prioritization here.

Comment 30

[Anant Narayanan [:anant]]

(In reply to Brendan Eich [:brendan] from comment #22)
> The Mozilla way is not to uplift all browsers at once, especially not via
> polyfilling fake DOM APIs from served JS (if that's how navigator.apps works
> in the cross-browser case).

It was pretty disappointing that you couldn't "install" Angry Birds from the Chrome Store into Firefox, even though the game worked just fine. The nice thing about a polyfill is that it lets us do better, and the user isn't locked into a specific browser at the start (even though they definitely won't be in the future). That's pretty much the only thing that doesn't feel right about a Fx-sync based solution as proposed in the bug; but as pointed out - it may not be relevant in the long run :)

Comment 31

[Andreas Gal :gal]

> How do we address #1, #3, #5 and #6 from comment #18 if we use the existing Firefox Sync client/service?

Lets deconstruct and go through this one by one.

Comment 6 first. You seem to criticize a quality of service issue in Firefox sync if I understand this correct (weren't you the PM of that until recently?). Persistence guarantees seems like a good target for any service. Can you please explain at a technical level why it is impossible for the current Firefox sync service to give persistence guarantees, and why your proposed solution can give such guarantees.

Comment 32

[Ragavan S [:rags]]

(In reply to Andreas Gal :gal from comment #31)

> 
> Comment 6 first. You seem to criticize a quality of service issue in Firefox
> sync if I understand this correct (weren't you the PM of that until
> recently?). Persistence guarantees seems like a good target for any service.
> Can you please explain at a technical level why it is impossible for the
> current Firefox sync service to give persistence guarantees, and why your
> proposed solution can give such guarantees.

Not a criticism of Sync as much as an observation based on what you astutely point out (me being the PM for Sync until recently).  Firefox Sync was a service that had different design goals (and hence a different implementation). 

With Sync, we relied on the client to guarantee persistence and didn't worry about losing data on the server. It created a couple of hard situations - if you somehow lost all of your clients, you lost your data if you didn't backup your key. If you wanted access to your data from another computer (or another browser), you couldn't.  Whether or not Sync should also offer data backup is a different topic, but I digress.

With Apps, we don't want to rely on the client always being the authoritative source. So, backup and recovery is a P1 for Apps(which was not the case for Firefox Sync). If you lose your device, all you need is to log back in with your BrowserID (which also has a recovery mode for lost password), and all the Apps you have bought across how many ever stores are back. 

Our proposed solution addresses this by storing a list of Apps (and receipts) tied to a user's BrowserID, unencrypted. 

I don't know how this would work in the case of Firefox Sync where the data is encrypted and there is a very high likelihood that a user does not have a backup of their Sync Key.

Comment 33

[Ragavan S [:rags]]

Also, Andreas - me asking how a particular requirement can be met (which is what I did in comment #25 above) does not constitute a criticism. I'd appreciate it if you didn't draw conclusions or put words in my mouth.

Comment 34

[Philipp von Weitershausen [:philikon]]

(In reply to Ragavan S [:rags] from comment #32)
> I don't know how this would work in the case of Firefox Sync where the data
> is encrypted and there is a very high likelihood that a user does not have a
> backup of their Sync Key.

This is a tricky problem indeed, and we've been thinking for a while on how to address this. There are several potential solutions out there. mconnor has proposed a "key escrow" service where users can (optionally) store their Sync Key with Mozilla. Another, slightly more low-tech, solution would be sending users their Sync Key from Firefox directly via email. Including a special URI that Firefox has a handler for and perhaps even a QR code could make setting up new devices pretty easy.

As far as BrowserID is concerned, I think we should try to figure out how to integrate it with Sync as soon as we can. Ryan Kelly has shown interest here (both from a server as well as a client perspective). So my suggestion would be to enable him (with beer, scotch, or whatever the man likes ;)).

So yeah, there's definitely opportunity to improve the Sync experience while a perhaps more integrated apps experience is ramping up, the apps APIs and protocols are being standardized, etc. I suggest that if we want to discuss potential solutions, be it the ones I outlined above or not, we move the discussion to separate bugs or the services-dev mailinglist [1], since this bug is getting quite long and OT already.

[1] https://wiki.mozilla.org/Services/services-dev

Comment 35

[[:fabrice] Fabrice Desré]

Attachment: Part 1: Webapps.jsm support for app sync

Comment 36

[[:fabrice] Fabrice Desré]

Attachment: Part 2: sync engine

Comment 37

[[:fabrice] Fabrice Desré]

Attachment: Part 3: firefox desktop UI

Comment 38

[Andreas Gal :gal]

"complicated setup/encryption" were your words, nobody put that in your mouth, and it sounds like criticism of the FF Sync service to me (criticism I agree with, by the way). The reason I raised this point is that I am trying to show you that you seem to have valid concerns how FF Sync could be improved. In my opinion small improvements on top of FF Sync is a less risky and faster path that a from-scratch design. I am confident most people with an engineering background will agree on this.

FF Sync does not guarantee persistence at the moment. This is a quality of service issue, and it has been raised by others as problematic for sync users (apps aside). We should discuss with the FF Sync team to upgrade the persistence guarantee of sync. Doing that is not any harder than persistently storing data in the new Apps Sync service (both ways in fact its a really hard problem to solve, and expensive). This has never been a sync design issue, its merely an operational (and cost) choice. We can also enable this only for parts of the data in FF Sync (e.g. Apps).

As Philipp already pointed out the Sync key situation is suboptimal and should be improved. There are hard trade-offs, but we have some reasonable alternatives. Again, I believe we can ship these changes many months sooner than we can stand up a brand new service.

Do you still believe FF Sync cannot satisfy the persistence requirements of your product? If so, why?

Comment 39

[Richard Newman [:rnewman]]

(In reply to Andreas Gal :gal from comment #38)

> FF Sync does not guarantee persistence at the moment. This is a quality of
> service issue, and it has been raised by others as problematic for sync
> users (apps aside). We should discuss with the FF Sync team to upgrade the
> persistence guarantee of sync.

Without addressing whether this is a good idea or not, or making any kind of recommendation:

There are two parts to your point about Sync persistence.

First is a cost issue: "guaranteed persistence" is a thorny problem. Now we're talking about georedundant replication, and you've probably just multiplied the cost per user by ten.

(If the alternative plans for app sync don't involve "and here is how we're going to replicate your receipts to two other sites around the world", then the correct term is still "best effort persistence", perhaps underlined twice or something.)

Second is a protocol/design issue. The Sync client's response to apparent consistency problems with the server is to wipe it, and our operations handbook does not include data migration as part of user migration. "Nuke and rebuild" renders moot whatever effort you put into persistence. If you can't replay transactions over time, or restore backed up state, or otherwise undo the actions of a client -- none of which are possible with Sync right now -- then all the reliability in the world won't help you. (This is the "but I reused my backup disk" problem.)

Making the Sync service a safe place to permanently store data would require a different style of interaction, and the costs of doing so need to be carefully considered.

Apologies to Fabrice for continuing the barn dance all over his bug :)

Comment 40

[Philipp von Weitershausen [:philikon]]

Comment on attachment 579569 [details] [diff] [review]
Part 1: Webapps.jsm support for app sync

Does Webapps.jsm have any tests and API documentation at all? I can't find it. If not, please file a follow-up to bug 697383. The JSM should have xpcshell-tests, the navigator.mozApps API should have mochitests, and at a minimum there should be some Javadoc-style comments in the JSM and the IDL. *Especially* the IDL since we want web developers to use this API, so we'll want to have docs on MDN.


>-  confirmInstall: function(aData) {
>+
>+  confirmInstall: function(aData, aFromSync) {
>     let app = aData.app;
>-    let id = this._appId(app.origin);
>+    let id = app.syncId || this._appId(app.origin);
> 
>     // install an application again is considered as an update
>     if (id) {
>@@ -193,9 +193,11 @@ let DOMApplicationRegistry = {
> 
>     this._writeFile(this.appsFile, JSON.stringify(this.webapps), (function() {
>       this.mm.sendAsyncMessage("Webapps:Install:Return:OK", aData);
>+      if (!aFromSync)
>+          Services.obs.notifyObservers(this, "webapps-sync-install", id);

I'm not sure why you need aFromSync. Sync is smart enough to ignore change notifications when it itself is manipulating the store.

Also nit: indent by 2 spaces, not 4. 

>+  /** added to support the sync engine */
>+
>+  getAppFromId: function(aId) {

Nit: getAppById() makes more sense I think, but I won't bikeshed over it ;)

>+    if (this.webapps[aId]) {
>+      let app = this._cloneAppObject(this.webapps[aId]);
>+      app.manifest = this._readManifest(aId);
>+      return app;
>+    }
>+    else {
>+      return null;
>+    }
>+  },

Nit: style guide says cuddling 'else' statements, and no 'else' statements after 'return' statements. Personally I'm a friend of bail-out early:

  if (!this.webapps[aId]) {
    return null;
  }
  let app = ...

>+  itemExists: function(aId) {
>+    return this.webapps[aId] != null && this.webapps[aId] != undefined;
>+  },

As it happens, null == undefined, so just the first == check there would be enough. Also, could also just check for truthiness, no? Like:

  return !!this.webapps[aId];

>+  getAllIDs: function() {
>+    let apps = {};
>+    for (let id in this.webapps) {
>+      // only sync http and https apps
>+      if (this.webapps[id].origin.indexOf("http") == 0)
>+        apps[id] = true;
>+    }
>+    return apps;
>+  },

This is a very Sync specific method. Particularly, it's specific to the way Sync engines are currently implemented, and we want to change that. So at the very least you want to be aware of that fact, but perhaps it would be nice to put a comment in there explaining that.

>+  uninstallById: function(aId) {
>+    if (this.webapps[aId]) {
>+      let msg = { origin: this.webapps[aId].origin };
>+      delete this.webapps[aId];
>+      this._writeFile(this.appsFile, JSON.stringify(this.webapps));

This method is async, which is good! But you're not waiting for the operation to complete, so we're going to queue up a bunch of uninstall requests on the I/O thread while the mainthread keeps going on. I'm worried about race conditions here. I suggest making this an async function taking a callback (Sync's store.remove() API is still synchronous, so you're going to have to do what the rest of Sync does and use Aysnc.spinningCallback [1].

[1] https://mxr.mozilla.org/mozilla-central/source/services/sync/modules/async.js#162

>+  wipe: function() {
>+    let ids = this.getAllIDs();
>+    for (let id in ids) {
>+      let data = {
>+        origin: this.webapps[id].origin
>+      }
>+      if (data.origin.indexOf("http") == 0)
>+        this.uninstall(data, true);
>+    }
>+   }
> };

Seems like this could be a lot simpler (and faster) if you used uninstallById().

Comment 41

[Philipp von Weitershausen [:philikon]]

Comment on attachment 579570 [details] [diff] [review]
Part 2: sync engine

>+ * The Original Code is Apps Sync.
>+ *
>+ * The Initial Developer of the Original Code is Mozilla.

"the Mozilla Foundation." on a new line.

>+  create: function create(record) {
>+    DOMApplicationRegistry.confirmInstall({ app: record.value }, true);
>+  },
>+
>+  remove: function remove(record) {
>+    DOMApplicationRegistry.uninstallById(record.id);
>+  },
>+
>+  update: function update(record) {
>+    DOMApplicationRegistry.uninstall(record.value, true);
>+    DOMApplicationRegistry.confirmInstall({ app: record.value }, true);
>+  },
>+
>+  wipe: function wipe(record) {
>+    DOMApplicationRegistry.wipe();
>+  }

Same thing what I said about part 1: These are async operations, so just queuing write operations on the I/O thread and continuing on the mainthread can easily yield to race conditions that will get your manifest in an inconsistent state.

I wish I had realized this before, but I think what you want to do is add a bulk API to Webapps.jsm, e.g.

  Webapps.updateApps([app1, app2, app3, ...], callback);

It would update the state for app1, app2, etc. (e.g. installing, uninstalling, etc.) and then just write the manifest once at the end. You can then skip implementing the create/remove/update APIs and just implement applyIncomingBatch. Take a look at the history engine for a (admittedly complex) example. You'll still have to implement wipe(), but that can also easily be a batch operation that just writes the manifest once.

>diff --git a/services/sync/services-sync.js b/services/sync/services-sync.js
>--- a/services/sync/services-sync.js
>+++ b/services/sync/services-sync.js
>@@ -22,6 +22,7 @@ pref("services.sync.engine.history", tru
> pref("services.sync.engine.passwords", true);
> pref("services.sync.engine.prefs", true);
> pref("services.sync.engine.tabs", true);
>+pref("services.sync.engine.apps", true);

Let's defer the enabling the engine by default and the UI pieces of this until webapps are actually a visible feature in Firefox. I think as a minimum this means having bug 697006 landed.

Comment 42

[Ragavan S [:rags]]

(In reply to Andreas Gal :gal from comment #38)
> In my opinion small improvements on top of FF Sync is a less risky
> and faster path that a from-scratch design. I am confident most people with
> an engineering background will agree on this.

I disagree. I won't comment on the engineering risks, but from a product, marketing, even operational security perspective, I don't think changes to Sync's encryption feature are less risky or present a faster path.

> 
> FF Sync does not guarantee persistence at the moment. This is a quality of
> service issue, and it has been raised by others as problematic for sync
> users (apps aside). We should discuss with the FF Sync team to upgrade the
> persistence guarantee of sync. Doing that is not any harder than
> persistently storing data in the new Apps Sync service (both ways in fact
> its a really hard problem to solve, and expensive). This has never been a
> sync design issue, its merely an operational (and cost) choice. We can also
> enable this only for parts of the data in FF Sync (e.g. Apps).

What does backing up encrypted data buy the user when they don't have a backup of their key? You can't solve the backup/restore use case without addressing the encryption requirement. You can't change the encryption requirement without breaking some fundamental product decisions that we made at that time. If you change those product decisions, you will also need to change the messaging/marketing that's been done based on those product attributes. You will also need to revisit the security/threat assessments. This is why I maintain this was a design choice and not merely an operational (and cost) choice. Talk to thunder or mconnor or others who were involved in this in the early days for even more of the details if you're interested.


> As Philipp already pointed out the Sync key situation is suboptimal and
> should be improved. There are hard trade-offs, but we have some reasonable
> alternatives. Again, I believe we can ship these changes many months sooner
> than we can stand up a brand new service.

I'm not particularly wedded to standing up a new service. All I want are the requirements in comment #18 met (esp. the ones in comment #25). If we can do that faster and safer on top of the existing Firefox Sync infrastructure, I'd love to learn more about that.

> 
> Do you still believe FF Sync cannot satisfy the persistence requirements of
> your product? If so, why?

Yes, for all the reasons I listed above. 

Also, I know we are focusing on just #6 now, but #1 is really important too as BrowserID is at the heart of everything in the Apps architecture -  the store authentication, the receipt, validating the receipt, storing the receipts in a dashboard etc. That also needs to happen and from past conversations, I've gathered that is also not a trivial change.

Comment 43

[Philipp von Weitershausen [:philikon]]

Comment on attachment 579571 [details] [diff] [review]
Part 3: firefox desktop UI

What I said in comment 41: Let's defer this piece to a follow-up but that depends on the main Firefox UI pieces (bug 697006). There's no point in enabling and promoting an engine for things that users have no other way of discovering.

The patch itself looks good, so I'll happily rubberstamp it in the new bug.

Apart from that, we still need xpcshell tests. I think you already filed a bug for that, so please make sure it blocks this bug.

Comment 44

[Brendan Eich [:brendan]]

(In reply to Ragavan S [:rags] from comment #28)
> The Firefox and B2G teams will go ahead and implement App Sync support as a
> Firefox Sync engine - with the same UX and product characteristics as
> Firefox Sync (no backup/restore, complicated setup/encryption, no browserid
> support etc). I don't even know how things like receipt validation would
> work without BrowserID support, but perhaps you guys have figured all that
> out?

Free apps are the trend and they'll do for now. When B2G needs to deal with non-free, it'll also probably have to deal with identity in ways we can't know right now (partners coming online won't get to this until next year). We're pitching browserid. We may need to support other monickers/protocols than email.

> The Apps team will continue to build an App Sync service that meets our
> requirements from comment #18 (whether that's by changing Sync or standing
> up a new service).

Someone has to deal with the in-browser API. If it's navigator.apps at the end of the day, then we can't have polyfills clobbering built-ins.

> Perhaps sometime in the future (say "Later" as Brendan defines it in #26),
> these two might merge?

That's possible, but the situation is fluid. Something might change one or both plans in a big way. We'll see. Main thing is to get users and grow usage.

> In the meantime, we will have two separate engineering efforts with
> completely different product and user experience for Apps in Firefox/B2G?

I'm not sure about that. If time to market for a new sync is long, and some of the good ideas in comment 34 first paragraph can help old Sync, then we might improve old Sync's UX in the shorter term.

> Am I understanding this right?

As well as any of us, but I do not believe we should project anything too far out. Also, we should keep browser API design alignment. We all want to standardize the API (tell me if not).

/be

Comment 45

[Brendan Eich [:brendan]]

(In reply to Philipp von Weitershausen [:philikon] from comment #40)
> Nit: style guide says cuddling 'else' statements, and no 'else' statements
> after 'return' statements. Personally I'm a friend of bail-out early

So is the style guide, in a misstated way:

https://developer.mozilla.org/En/Mozilla_Coding_Style_Guide#Return_from_errors_immediately

s/errors/abnormal conditions/

/be

Comment 46

[Andreas Gal :gal]

> What does backing up encrypted data buy the user when they don't have a
> backup of their key? You can't solve the backup/restore use case without
> addressing the encryption requirement.

This is wrong. Please read Philipp's comment. Various solutions from a key escrow to email-key-to-user to username/password-derived keys were proposed (and partially explained in this bug).

> Also, I know we are focusing on just #6 now, but #1 is really important too
> as BrowserID is at the heart of everything in the Apps architecture -  the
> store authentication, the receipt, validating the receipt, storing the
> receipts in a dashboard etc. That also needs to happen and from past
> conversations, I've gathered that is also not a trivial change.

Sure, lets finish #6 and then we will move up your list. After re-reading Philipp's comment, do you still believe FF sync is not acceptable, assuming we fix the key recovery issue? If so, why?

Comment 47

[Andreas Gal :gal]

> Free apps are the trend and they'll do for now. When B2G needs to deal with
> non-free, it'll also probably have to deal with identity in ways we can't
> know right now (partners coming online won't get to this until next year).
> We're pitching browserid. We may need to support other monickers/protocols
> than email.

I absolutely agree. Browserid is our best foot forward, but betting on it as only solution multiplies risk and makes it that much less likely that webapps will succeed. We already have a lot of overlap with Google's current webapps strategy. We can likely use that to standardize the webapps part. Identity is an entirely different ballgame. Vendors like Google might have a strong interest to push their proprietary identity APIs (also think Facebook connect and friends). If we tie your web apps strategy to browserid succeeding, there is a good chance both will fail to become standards.

Just to be clear, if we implement a kick ass B2G phone, but we fail to standardize most key APIs, the project will be a failure. Mozilla wants to move the web, not just our own code/client/browser/services.

Similarly, if you build a great webapps service, even if you get people to use it, if we fail to move the needle of the web and establish an interoperable standard with multiple implementations, the webapps team will have failed.

Comment 48

[[:fabrice] Fabrice Desré]

Attachment: Part 1: Webapps.jsm support for app sync

Updated according to comments.

Comment 49

[[:fabrice] Fabrice Desré]

Attachment: Part 2: sync engine

Now using applyIncomingBatch, which simplifies things quite a bit.

I let the pref to true just for testing, and will remove it when landing.

Comment 50

[Philipp von Weitershausen [:philikon]]

Comment on attachment 579890 [details] [diff] [review]
Part 1: Webapps.jsm support for app sync

>-  confirmInstall: function(aData) {
>+
>+  confirmInstall: function(aData, aFromSync) {
>     let app = aData.app;
>-    let id = this._appId(app.origin);
>+    let id = app.syncId || this._appId(app.origin);
> 
>     // install an application again is considered as an update
>     if (id) {
>@@ -191,11 +191,14 @@ let DOMApplicationRegistry = {
>     delete this.webapps[id].manifest;
>     this.webapps[id].installTime = (new Date()).getTime()
> 
>-    this._writeFile(this.appsFile, JSON.stringify(this.webapps), (function() {
>+    
>+    this._saveApps((function() {
>       this.mm.sendAsyncMessage("Webapps:Install:Return:OK", aData);
>+      if (!aFromSync)
>+        Services.obs.notifyObservers(this, "webapps-sync-install", id);
>     }).bind(this));
>   },

So, in my previous review comment, I said that you don't have to do the aFromSync check because Sync will ignore the tracker while it's updating the store. However, the aFromSync flag would be useful here in a different way: you don't want to call this._saveApps() for every single call to confirmInstall when it's called from Sync, because you write the manifest once in the end.

>+  updateApps: function(aRecords, aCallback) {
>+    for (let i = 0; i < aRecords.length; i++) {
>+      let record = aRecords[i];
>+      if (record.deleted) {
>+        if (!this.webapps[record.id])
>+          continue;
>+        let origin = this.webapps[record.id].origin;
>+        delete this.webapps[record.id];
>+        let dir = FileUtils.getDir("ProfD", ["webapps", record.id], true, true);
>+        try {
>+          dir.remove(true);
>+        } catch (e) {
>+        }
>+        this.mm.sendAsyncMessage("Webapps:Uninstall:Return:OK", { origin: origin });
>+      } else {
>+        if (this.itemExists(record.id)) // update

What does this comment mean? I would suggest inlining itemExists for perf reasons (tight loop on the main thread).

>+          this.webapps[record.id] = this._cloneAppObject(record.value);

No need to deepcopy the record's data. You own the record, you may modify it however you want. Take record.value, morph it the way you want, and then just take that. Saves creating lots of new objects during a sync and thus putting pressure on the GC in a tight loop.

>+        else // new app
>+          this.confirmInstall({ app: record.value }, true);

See above: confirmInstall will call _saveApps for every invocation right now, making the _saveApps call below useless.

>+      }
>+    }
>+    this._saveApps(aCallback);
>+  },

r=me with those changes.

Comment 51

[Philipp von Weitershausen [:philikon]]

Comment on attachment 579891 [details] [diff] [review]
Part 2: sync engine

>+  applyIncomingBatch: function applyIncomingBatch(aRecords) {
>+    let callback = Async.makeSyncCallback();
>+    DOMApplicationRegistry.updateApps(aRecords, callback);
>+    Async.waitForSyncCallback(callback);
>+    return [];

Returning an empty array means there were no failures. In this case it means there will never ever be any failures :). This will, for instance, hide errors when writing the manifests and other state. I don't think it's a big deal because it's an edge case, but we should have a follow-up for investigating those corner cases. If we return a list of IDs here that failed to apply, Sync is actually smart enough to re-fetch them on the next sync. That way it prevents terminal data loss, or at least tries to.

r=philikon otherwise. You may land this code in services-central since it's completely disabled for now. The next steps for this would be (in order):

1. tests (bug 708462)
2. figure out what the main product UI in Firefox should be (bug 697006)
3. enable engine + and add Sync UI for engine (bug 708462)

Comment 52

[Philipp von Weitershausen [:philikon]]

(In reply to Philipp von Weitershausen [:philikon] from comment #51)
> 1. tests (bug 708462)

Sorry, I meant bug 708184

> 2. figure out what the main product UI in Firefox should be (bug 697006)
> 3. enable engine + and add Sync UI for engine (bug 708462)

Comment 53

[Ian Bicking (:ianbicking)]

(In reply to Brendan Eich [:brendan] from comment #23)
> Just out of curiosity: has anyone looked at Chrome's chrome-app-sync to see
> whether it could handle open web apps? If navigator.apps becomes a web API
> standard, Google will likely want it in Chrome to hook up (as 1 of N if not
> exclusively) to their sync service.

Chrome uses Protocol Buffers over XMPP (http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/sync/) - I understand there is optional crypto in there somewhere, but I'm not sure at what layer.  It's a pretty different approach, and operationally unlike anything we do.  It's not unreasonable for multiple sync systems to be working with the same data, which seems like the most realistic approach to compatibility across browsers (maybe with some entirely different third sync protocol that was agreed upon).

Comment 54

[[:fabrice] Fabrice Desré]

pushed:
http://hg.mozilla.org/services/services-central/rev/0e6fc020716f
http://hg.mozilla.org/services/services-central/rev/0c6f1480408b

Comment 55

[Brendan Eich [:brendan]]

(In reply to Anant Narayanan [:anant] from comment #30)
> (In reply to Brendan Eich [:brendan] from comment #22)
> > The Mozilla way is not to uplift all browsers at once, especially not via
> > polyfilling fake DOM APIs from served JS (if that's how navigator.apps works
> > in the cross-browser case).
> 
> It was pretty disappointing that you couldn't "install" Angry Birds from the
> Chrome Store into Firefox, even though the game worked just fine.

Yes, agreed -- blame flows separately from creativity about workarounds on that score ;-).

> The nice
> thing about a polyfill is that it lets us do better, and the user isn't
> locked into a specific browser at the start (even though they definitely
> won't be in the future). That's pretty much the only thing that doesn't feel
> right about a Fx-sync based solution as proposed in the bug; but as pointed
> out - it may not be relevant in the long run :)

You're right that standing up a web service allows downloading JS to extend the browser APIs (writ large). I mean, c'mon: Facebook ;-).

But the trick is to leverage our assets to grow from zero open web apps users to significant numbers. If apps are new and (at first) not necessarily as shiny or compelling cross-browser as any one browser's apps, or one mobile OS's native apps, then getting the users we have, of Firefox-the-browser, may help us grow to win users we don't have -- and to forge cross-browser standards.

FB's JS injections have not led to cross-browser standards, that is, browser standards (ones implemented in WebKit, Gecko, Trident, etc. -- standards you can count on even when offline, no matter the state of your caches).

/be

Comment 56

[Gregory Szorc [:gps]]

Pushed to m-c:

https://hg.mozilla.org/mozilla-central/rev/0c6f1480408b
https://hg.mozilla.org/mozilla-central/rev/0e6fc020716f

Comment 57

[Curtis Koenig [:curtisk-use curtis.koenig+bzATgmail.com]]]

need to schedule a review with secteam

Comment 58

[Curtis Koenig [:curtisk-use curtis.koenig+bzATgmail.com]]]

removing [secr:curtisk] as I think this might be covered somewhere else, will triage with team and discuss.

Comment 59

[Anant Narayanan [:anant]]

The new Browser-ID based "Apps in the Cloud" implementation for Firefox is being tracked in bug 744257.