1210697 - [smart-system] Add SyncManager and SyncStateMachine

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::TV::System, defect, P1)

Description

[yifan [:yifan][:yliao]]

Add SyncManager and SyncStateMachine from apps/system to tv_apps/smart-system.

Comment 1

[GitHub Autolander Bot]

Attachment: [gaia] begeeben:1210697_add_syncmanager > mozilla-b2g:master

Comment 2

[yifan [:yifan][:yliao]]

Comment on attachment 8668831 [details] [review]
[gaia] begeeben:1210697_add_syncmanager > mozilla-b2g:master

Hi Sean & Fernando, could you please help to review this patch? Thanks!

Comment 3

[Fernando Jiménez Moreno [:ferjm]]

Comment on attachment 8668831 [details] [review]
[gaia] begeeben:1210697_add_syncmanager > mozilla-b2g:master

Thank you YiFan. Looks good, however we need bug 1182071 and bug 1202342 to make your patch work properly. I'll file a bug for that. I left some comments on the PR I'd like you to address or answer before r+ing the patch.

Thank you for all the work you are doing with FxA/Sync!

Comment 4

[Sean Lee [:seanlee][:weilonge]]

Comment on attachment 8668831 [details] [review]
[gaia] begeeben:1210697_add_syncmanager > mozilla-b2g:master

Hi Yifan,

LGTM :)

Comment 5

[Fernando Jiménez Moreno [:ferjm]]

BTW, we also need to make use of the preprocessor here to enable/disable Sync. Check bug 1191770 and bug 1202383.

Comment 6

[yifan [:yifan][:yliao]]

(In reply to Fernando Jiménez Moreno [:ferjm] from comment #5)
> BTW, we also need to make use of the preprocessor here to enable/disable
> Sync. Check bug 1191770 and bug 1202383.

Thank you! Opened Bug 1211833 - Merge preprocessing functionality to TV to enable/disable sync to follow up.

Comment 7

[yifan [:yifan][:yliao]]

Comment on attachment 8668831 [details] [review]
[gaia] begeeben:1210697_add_syncmanager > mozilla-b2g:master

The patch and comments were updated. Please help to review it, thank you!

Comment 8

[Fernando Jiménez Moreno [:ferjm]]

Comment on attachment 8668831 [details] [review]
[gaia] begeeben:1210697_add_syncmanager > mozilla-b2g:master

Thank you YiFan. I am afraid that this still doesn't work. If you want to test the patch, you can do it by adding the test_sync app from [1] to your build.

I suspect that what I am seeing is a BaseModule issue. 

What I see is that the state never changes in the Sync state machine. When we query the state via Service.query('StateMachine.state') we always get 'disabled'.

And I see this on the logcat:

ObjectActor.prototype.grip previewer function threw an exception: [Exception... "Factory not registered"  nsresult: "0x80040154 (NS_ERROR_FACTORY_NOT_REGISTERED)"  location: "JS frame :: app://smart-system.gaiamobile.org/js/service.js :: exports.Service.query :: line 195"  data: no]
Stack: exports.Service.query@app://smart-system.gaiamobile.org/js/service.js:195:9
.updateState@app://smart-system.gaiamobile.org/js/sync_manager.js:410:29
._handle_onsyncenabling@app://smart-system.gaiamobile.org/js/sync_manager.js:261:7
EventMixin.handleEvent@app://smart-system.gaiamobile.org/js/base_module.js:344:9
BaseModule.prototype.publish@app://smart-system.gaiamobile.org/js/base_module.js:597:7
._createTransition/<@app://smart-system.gaiamobile.org/js/sync_state_machine.js:140:1
exports.Service.request/<@app://smart-system.gaiamobile.org/js/service.js:58:21
exports.Service.request@app://smart-system.gaiamobile.org/js/service.js:57:18
["_handle_iac-gaia-sync-management"]@app://smart-system.gaiamobile.org/js/sync_manager.js:207:13
EventMixin.handleEvent@app://smart-system.gaiamobile.org/js/base_module.js:344:9
onReceivedMessage@app://smart-system.gaiamobile.org/shared/js/iac_handler.js:39:11

Could you take a look please?

Maybe it is easier to move away from BaseModule instead of fixing BaseModule itself.

[1] https://bugzilla.mozilla.org/attachment.cgi?id=8661685

Comment 9

[yifan [:yifan][:yliao]]

Thank you! Tried and ended up with another error message with BaseModule in service.js, maybe I did something wrong running the test apps. 

Anyway after offline discussion with Evelyn and Luke, we agree that we probably should move away from BaseModule in this case. Are you OK to provide a patch? It will be very appreciated!

Comment 10

[Fernando Jiménez Moreno [:ferjm]]

Sure. Moving away from BaseModule sounds good. Thanks.

Comment 11

[GitHub Autolander Bot]

Attachment: [gaia] ferjm:bug1210697.syncsystemtv > mozilla-b2g:master

Comment 12

[yifan [:yifan][:yliao]]

Comment on attachment 8672760 [details] [review]
[gaia] ferjm:bug1210697.syncsystemtv > mozilla-b2g:master

Thank you very much Fernando! That is quite some changes. LGTM!

Comment 13

[Sean Lee [:seanlee][:weilonge]]

Comment on attachment 8672760 [details] [review]
[gaia] ferjm:bug1210697.syncsystemtv > mozilla-b2g:master

LGTM! and I would like to give f+ here :)

Comment 14

[Fernando Jiménez Moreno [:ferjm]]

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