1172699 - Migrate timeIcon to Intl

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::System::Status bar, Utility tray, Notification, defect)

Description

[Zibi Braniecki [:zbraniecki][:gandalf]]

TimeIcon currently uses l10n_date.js shim, we should migrate it to Intl.

Comment 1

[Zibi Braniecki [:zbraniecki][:gandalf]]

Attachment: pull request

Comment 2

[Zibi Braniecki [:zbraniecki][:gandalf]]

Comment on attachment 8616960 [details] [review]
pull request

Michael, we want to start moving FxOS from l10n_date shim to Intl (bug 1170963).

Intl has a static cost that we have to pay, and since system needs it, we should pay it early (discussed with :fabrice, :waldo and :tedders).

If we load it here, we will not have to preload it in Nuwa, because Gecko will have it available for all apps later.
The total outcome should be neutral, as we will be able to remove l10n_date which is slow (bug 1015181).

This patch covers timeIcon.

We'll have to wait for bug 1172609 to be fixed, so I won't land it until it works, but wanted to get your review.

Comment 3

[Michael Henretty [:mikehenrty][:mhenretty]]

What does it mean, load Intl? What causes it to load, this line?
`this.timeFormatter = new Intl.DateTimeFormat(navigator.languages...`

Should we profile boot time as part of this? What is the expected impact?

Overall, I like the changes I see here, so I'm inclined to give it an r+ once I understand the impact.

Comment 4

[Zibi Braniecki [:zbraniecki][:gandalf]]

Sure, let me explain:

Intl is lazy loaded, which means that the first use of the API costs. It costs about 800kb or memory. What we use right now is l10n_date.js which costs ~150kb per use and is extremely slow (bug 1015181).

The good thing about Intl is that every consecutive load costs close to zero. So the total outcome is more comparable, because with three apps running, we're currently paying 150x3 plus CPU.

Switching to Intl is a huge win for us not only because of that. It also provides better internationalization, reduces the work for localizers and makes our code more global (and also allows us to remove mozL10n.get calls).

But we need to load it somewhere, and the choices are either Nuwa or System. Because we will be using Intl in the system, :fabrice suggested to load it here. This patch turns timeIcon to use Intl, but what it also does is that it turns on Intl for the rest of the system.

It's hard to measure the impact honestly, because we still have more work to do in the system to remove the dependency on l10n_date.js which is when we'll drop the mem/cpu cost of that. 
I have a patch to move lockscreen (bug 1172621), and I'll be following up with moving all pieces, but wanted to start with this.

You can read more in bug 1171856.

Comment 5

[Michael Henretty [:mikehenrty][:mhenretty]]

Comment on attachment 8616960 [details] [review]
pull request

Thank you for the detailed explanation, sounds like a big win.

I'm leaving the r+ with the understanding that you'll make sure this works properly after bug 1172609 lands. Code looks good to me.

Comment 6

[Zibi Braniecki [:zbraniecki][:gandalf]]

NI Johan for QA.

Johan, this is the first patch that will make Intl be loaded in the system and switch the statusbar time icon to be formatted by Intl API. Can you confirm that we're ok to land it?

Comment 7

[KTucker [:KTucker][Inactive 3/4/2016]]

Adding qawanted to test this thoroughly before it lands.

Comment 8

[Johan Lorenzo [:jlorenzo]]

Kevin will assign somebody for testing today.

Comment 9

[Duane Dixon [:ddixon]]

I have tested the Date & Time behaviors in the following custom build:

Environmental Variables:
Device: Flame 2.5
Build ID: 20150723141007
Gaia: ec2199b324304d3678b6a98a08a31bdc13c9e984
Gecko: Unknown
Gonk: 41d3e221039d1c4486fc13ff26793a7a39226423
Version: 42.0a1 (2.5)
Firmware Version: v18D
User Agent: Mozilla/5.0 (Mobile; rv:42.0) Gecko/42.0 Firefox/42.0

Custom Build results (Flame):
[Settings>Date & Time] Changing the region and city then toggling "Set Automatically" on, causes the clock to re-adjust, however, the clocks on the status bar and lockscreen do not adjust accordingly.

This is already a known issue...

Comment 10

[Zibi Braniecki [:zbraniecki][:gandalf]]

Commit: https://github.com/mozilla-b2g/gaia/commit/36827542237b90edc3e8f9c226b6e82e4d5e4014

Thanks a lot for help everyone!