Closed Bug 993948 Opened 5 years ago Closed 5 years ago

[Settings] refactor to identify root panel scope inside of settings.js

Categories

(Firefox OS Graveyard :: Gaia, defect)

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

(feature-b2g:2.0)

RESOLVED FIXED
2.0 S1 (9may)
feature-b2g 2.0

People

(Reporter: gasolin, Assigned: gasolin)

References

Details

(Whiteboard: [FT:System-Platform][p=1])

Attachments

(1 file)

Move root panel related operation into a instantialble module inside of settings.js and not affect current functionality.

To prepare for finally move root panel out of settings.js.

And findout possible load time boost tasks.
Assignee: nobody → gasolin
Status: NEW → ASSIGNED
Target Milestone: --- → 1.4 S6 (25apr)
* Use instantiable root panel to wrap root panel related operations
* `getSupportedLanguages` is moved to utils.js for language panel reuse
* flatten some functions such as `_panelsWithClass`
* use bind(this) instead of Settings.[function] call

current root panel separation made load time increased from 1667 -> 1730
Attachment #8404429 - Flags: feedback?(arthur.chen)
We could remove Settings.getSettings/settingsCache from main/settings.js to boost loadtime:

There are 5 files still use Settings.getSettings:
call, call_voicemail_settings, firefox_accounts/menu_loader, media_storage, storage
latest test result is 1697(master) -> 1721(patch)
in inari, 1631 -> 1621

master
time: median:1614, mean:1631, std: 67, max:1869, min:1533, all:1649,1614,1676,1608,1597,1608,1629,1619,1618,1635,1590,1869,1657,1610,1615,1631,1603,1614,1591,1593,1621,1628,1566,1612,1612,1652,1533,1857,1624,1602

after patch
time: median:1610, mean:1621, std: 41, max:1802, min:1575, all:1703,1608,1594,1611,1610,1602,1615,1575,1594,1608,1586,1601,1608,1672,1638,1608,1602,1627,1593,1620,1625,1631,1607,1620,1620,1612,1626,1802,1618,1606
Comment on attachment 8404429 [details] [review]
pull request redirect to github

Now get similar load time!
Attachment #8404429 - Flags: feedback?(arthur.chen) → review?(arthur.chen)
Blocks: 973453
Comment on attachment 8404429 [details] [review]
pull request redirect to github

Thanks for the patch! Please check my comments in github.
Attachment #8404429 - Flags: review?(arthur.chen)
Comment on attachment 8404429 [details] [review]
pull request redirect to github

comments addressed, please kindly review it again
Attachment #8404429 - Flags: review?(arthur.chen)
Comment on attachment 8404429 [details] [review]
pull request redirect to github

The language support function seems not cache the data as expected, could you check it again?
Attachment #8404429 - Flags: review?(arthur.chen)
Comment on attachment 8404429 [details] [review]
pull request redirect to github

I've wrapped getSupportedLanguages with IIFE, please kindly review again
Attachment #8404429 - Flags: review?(arthur.chen)
Comment on attachment 8404429 [details] [review]
pull request redirect to github

r=me with the comment addressed, thanks!
Attachment #8404429 - Flags: review?(arthur.chen) → review+
Whiteboard: [p=1]
Target Milestone: 1.4 S6 (25apr) → 2.0 S1 (9may)
Whiteboard: [p=1] → [FT:System-Platform][p=1]
merged to gaia-master https://github.com/mozilla-b2g/gaia/commit/79b6f24adf5204fb807f20676ea7b3bd725fe1b2

thanks for help!
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
feature-b2g: --- → 2.0
You need to log in before you can comment on or make changes to this bug.