1204432 - Use LazyLoader in Sync app

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Sync, defect)

Description

[Michiel de Jong [:michielbdejong]]

We need a small bit of code (see https://github.com/michielbdejong/gaia/blob/1200539-sync-app-integration-tests/apps/sync/js/app.js) to LazyLoad these scripts before constructing the SyncEngine object with the options passed from IAC and calling syncEngine.sync().

All the actual functionality of the Sync app is then divided into:
* SyncEngine
* Kinto.js (external)
* FxSyncWebCrypto
* The various DataAdapters

Comment 1

[Michiel de Jong [:michielbdejong]]

The way it can work:
* When the Sync app is opened, it registers its IAC handler
* When the IAC handler is called from the system app, it lazy-loads all files (depending on collectionNames specified), constructs the SyncEngine object, and calls syncEngine.syncNow(collectionNames).
* If another IAC request comes in while one is already running, the second one is rejected.
* Once sync is finished, the app closes itself.

That way, we don't have to handle the case where the IAC handler is called twice with different options - just LazyLoad and construct everything once, and self-destruct when done. There are only three states:
* The Sync app is open, waiting for its IAC request (but scripts not LazyLoaded yet, and SyncEngine not constructed yet).
* The Sync app is running.
* The Sync app is closed.

I'll work on this once bug 1196096 has landed, so we can test the launching of the Sync app through IAC.

Comment 2

[Michiel de Jong [:michielbdejong]]

First draft in https://github.com/michielbdejong/gaia/tree/1204432-sync-app-lazyloader

Comment 3

[Michiel de Jong [:michielbdejong]]

To do:
* Make sure only one request is running at any time
* Add unit tests
* Document App.handleRequest Error types it can reject its promise with, oriented at what should be one about it. Something along the lines of:
  * Refresh the assertion
  * Try again in a couple of minutes (network failure, Backoff response, server failure)
  * Tell the user to sync from FxDesktop first, then retry (no crypto/keys found on the account)
  * Tell the user they are over quota on the FxSync server
  * Tell the user something seems wrong with (the data on) their FxSync account
  * Tell the user something went wrong and we're very sorry

Comment 4

[GitHub Autolander Bot]

Attachment: [gaia] michielbdejong:1204432-sync-app-lazyloader > mozilla-b2g:master

Comment 5

[Fernando Jiménez Moreno [:ferjm]]

Comment on attachment 8664872 [details] [review]
[gaia] michielbdejong:1204432-sync-app-lazyloader > mozilla-b2g:master

Nice work. I left a few comments in the PR, nothing major, but I'd like to see the last version. Thanks!

Comment 6

[Michiel de Jong [:michielbdejong]]

Thanks! Addressed your comments. Shall I rebase and squash?

See https://github.com/mozilla-b2g/gaia/pull/32004#commits-pushed-4e94ccf

Comment 7

[Fernando Jiménez Moreno [:ferjm]]

Thank you Michiel. I added a couple of comments to the PR. The thing that I'd like to address/response before landing is the usage of the shared lazy loader.

Comment 8

[Fernando Jiménez Moreno [:ferjm]]

Comment on attachment 8664872 [details] [review]
[gaia] michielbdejong:1204432-sync-app-lazyloader > mozilla-b2g:master

Thanks Michiel. Could you squash the commits in one or file a new bug for the second commit and amend the commit message to add the bug number, please?

Comment 9

[Michiel de Jong [:michielbdejong]]

I moved the unrelated commit out (will make that a separate bug) and rebased on master again.

I also added a comment [1] linking to bug 1209934, reminding us that we still need to update SyncEngine, because collections is now an object instead of an Array.


[1] https://github.com/michielbdejong/gaia/commit/8825fde9cc4069cb6b46ee534226b1e96fe92408#diff-b008ab8ad34943e9401ee0ced57da757R83

Comment 10

[Fernando Jiménez Moreno [:ferjm]]

https://github.com/mozilla-b2g/gaia/commit/be2da974ba076e06e2ff96cd3ac911769d3067a3