1161927 - [Settings] Migrate settings app to use new system update API

Status: RESOLVED WONTFIX

(Firefox OS Graveyard :: Gaia::Settings, defect)

Description

[Arthur Chen [:arthurcc][inactive after 6/30]]

Bug 1037329 introduces a new set of APIs for system update. All functions regarding system update should migrate to the new APIs.

ux spec: https://bug1034735.bugzilla.mozilla.org/attachment.cgi?id=8453638
document: https://docs.google.com/presentation/d/11Bco460JwmUUDg3itwOI0QsnkvSDicT1Bk5bi2gZ2IU/edit?usp=sharing

Comment 1

[Tim Guan-tin Chien [:timdream] (please needinfo)]

What about System app? Do we need another bug filed for that?

Comment 2

[Arthur Chen [:arthurcc][inactive after 6/30]]

I was planning to make the changes in the same patch because the migration should better be done at once.

Comment 3

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Cool!

Comment 4

[GitHub Autolander Bot]

Attachment: [gaia] crh0716:1161927 > mozilla-b2g:master

Comment 5

[Arthur Chen [:arthurcc][inactive after 6/30]]

Comment on attachment 8605705 [details] [review]
[gaia] crh0716:1161927 > mozilla-b2g:master

Hey James,

Here is the WIP patch. I've done some initial tests and it seemed work fine, please help check in your end.

In addition, I assume the goal was to remove all mozChromeEvent and mozContentEvent created for system update. With the proposed API I'm still not able to remove 'update-prompt-apply-result'. We should discuss more on how you plan to remove those events from the gecko side and include the cases in the proposed API.

Comment 6

[James Cheng[:JamesCheng]]

Hi Arthur,

Our proposed API substituting for 'update-prompt-apply-result' is applyUpdate().

Let's discuss if you think applyUpdate is not enough for all the cases for you to migrate the current logic.

Thanks.

Comment 7

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Temporary assign to Fred until we could find better candidate to take over.

Comment 8

[GitHub Autolander Bot]

Attachment: [gaia] gasolin:issue-1161927 > mozilla-b2g:master

Comment 9

[[e-mail]]

Not trace code yet. Just rebase so the code can be merged easier.

Comment 10

[[e-mail]]

James since I'm not that familiar with this patch yet, please try the last PR and let's discuss about what can we do to make some progress. Thanks.

Comment 11

[James Cheng[:JamesCheng]]

Hi Fred,

Since Bug 1037329 is landed, 

This patch should work fine with the gecko updateprompt.js using WebAPI.

I will try it later. 

Thanks.

Comment 12

[James Cheng[:JamesCheng]]

Hi Fred,

I apply the patch and just do a simple test since I am not familiar with the coverage of this patch.

I set dom.system_update.debug to true for enabling the log from SystemUpdateService.jsm.

I can see the log means this patch is using the WebAPI to communicate with updateprompt.js.

I tried checkupdate, download update package, applyupdate from setting app and notification and it works fine.

Thanks.

Comment 13

[[e-mail]]

@Gerry per offline discussion we'd like ask you a favor to help us test if the patch works well with current FOTA flow.

Please get nightly gecko and apply this patch to gaia master

`curl https://patch-diff.githubusercontent.com/raw/mozilla-b2g/gaia/pull/31124.patch | git apply`

Comment 14

[[e-mail]]

@james I've check related bugs and only seen the ppt related to the system update API
https://docs.google.com/presentation/d/11Bco460JwmUUDg3itwOI0QsnkvSDicT1Bk5bi2gZ2IU/edit#slide=id.p

Is there any online reference (mozilla wiki, mdn) that describe this system update API?

Comment 15

[James Cheng[:JamesCheng]]

@Fred,
Sorry,
Currently we only have this google document for reference. Since this document is quite clear for the developer for understanding the usage of the APIs.

Comment 16

[Gerry Chang [:gchang]]

Following is my steps:
1. I flash the latest build with 20150728175206 and make reset-gaia with above 31124.patch. 
2. Put a gaia OTA package in my local server.
3. Change the Update URL to my local server.
4. Perform OTA

I can see the package is downloaded and uncompressed and device is rebooted.

Comment 17

[[e-mail]]

Comment on attachment 8639678 [details] [review]
[gaia] gasolin:issue-1161927 > mozilla-b2g:master

Thanks James and Gerry. I'd set review to myself and tim for Gaia Settings & System. If there's any issue I'll fix it and set review to other Settings peer.

Comment 18

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Comment on attachment 8639678 [details] [review]
[gaia] gasolin:issue-1161927 > mozilla-b2g:master

I've only read the System app part. See comment, please fix that before landing.

Comment 19

[[e-mail]]

Settings part looks fine to me. 

According to treeherder result, several tests are broken and missed unit tests for new files.
Will fix and add some unit tests and see if need other else's review.

Comment 20

[[e-mail]]

The rewrite are most done and I've fixed most tests (1 file left).

Gerry can you help test with this PR again? 
And please notify me when you prepare to start the test. I'd like to learn so I can do it myself next time.

Comment 21

[[e-mail]]

Thanks gerry's help. I'll update the PR according to manual test result.

Comment 22

[[e-mail]]

Since the API changed, there're about 2~30 test cases needs to be re-written for update.js to make sure everything works fine.

Due to current status of regression/blockers in settings, I'd postpone this issue until I got more spare time.

Comment 23

[BMO Automation]

Firefox OS is not being worked on