Use LazyLoader in Sync app

RESOLVED FIXED in FxOS-S8 (02Oct)

Status

Firefox OS
Sync
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: michielbdejong, Assigned: michielbdejong)

Tracking

unspecified
FxOS-S8 (02Oct)
ARM
Gonk (Firefox OS)
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [partner-cherry-picked<2015/11/10>])

Attachments

(1 attachment)

(Assignee)

Description

3 years ago
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
(Assignee)

Updated

3 years ago
Assignee: nobody → mbdejong
Blocks: 1200539
(Assignee)

Comment 1

3 years ago
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.
(Assignee)

Updated

3 years ago
Depends on: 1205220
(Assignee)

Updated

3 years ago
Depends on: 1204827, 1202627
(Assignee)

Comment 3

3 years ago
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
(Assignee)

Updated

3 years ago
Blocks: 1195647
No longer blocks: 1200539
(Assignee)

Updated

3 years ago
Target Milestone: --- → FxOS-S8 (02Oct)
(Assignee)

Updated

3 years ago
No longer depends on: 1202627
Created attachment 8664872 [details] [review]
[gaia] michielbdejong:1204432-sync-app-lazyloader > mozilla-b2g:master
(Assignee)

Updated

3 years ago
Attachment #8664872 - Flags: review?(ferjmoreno)
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!
Attachment #8664872 - Flags: review?(ferjmoreno) → feedback+
Blocks: 1205220
No longer depends on: 1205220
(Assignee)

Comment 6

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

See https://github.com/mozilla-b2g/gaia/pull/32004#commits-pushed-4e94ccf
Flags: needinfo?(ferjmoreno)
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.
Flags: needinfo?(ferjmoreno)
(Assignee)

Updated

3 years ago
Attachment #8664872 - Flags: review?(ferjmoreno)
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?
Attachment #8664872 - Flags: review?(ferjmoreno) → review+
(Assignee)

Comment 9

3 years ago
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
Flags: needinfo?(ferjmoreno)
https://github.com/mozilla-b2g/gaia/commit/be2da974ba076e06e2ff96cd3ac911769d3067a3
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Flags: needinfo?(ferjmoreno)
Resolution: --- → FIXED
Whiteboard: partner-cherry-pick
Whiteboard: partner-cherry-pick
Whiteboard: [partner-cherry-pick]

Updated

3 years ago
Whiteboard: [partner-cherry-pick] → [partner-cherry-picked<2015/11/10>]
You need to log in before you can comment on or make changes to this bug.