Closed
Bug 701068
Opened 13 years ago
Closed 11 years ago
investigate the locale setting code and implications on android
Categories
(Firefox for Android Graveyard :: General, defect, P3)
Tracking
(fennec-)
RESOLVED
FIXED
Firefox 28
Tracking | Status | |
---|---|---|
fennec | - | --- |
People
(Reporter: Pike, Assigned: rnewman)
References
Details
Doug mentioned that the getting/setting locale dance was removed, but it's unclear what the fall-out is.
Doug, can you provide pointers on what exactly the code change was, so that we can try to reproduce?
Updated•13 years ago
|
Assignee: nobody → doug.turner
Priority: -- → P3
Comment 1•13 years ago
|
||
we removed the following from the startup path:
We want to disable this code if possible. It is about 145ms
SharedPreferences settings = getPreferences(Activity.MODE_PRIVATE);
String localeCode = settings.getString(getPackageName() + ".locale", "");
if (localeCode != null && localeCode.length() > 0)
GeckoAppShell.setSelectedLocale(localeCode);
Reporter | ||
Comment 2•13 years ago
|
||
We're talking about http://hg.mozilla.org/users/dougt_mozilla.com/birch_native_ui_pre_wipe/rev/5b8a01f59171, right?
So, here's my take:
This is only an issue if we're trying to do explicit locale selection in a multi-locale build. In my local testing with the multi-locale builds I did post bug 702302, things seemed to work allright for the matched locales. And that tree had the commenting out already.
I assume in our planned setup with single-locale builds with an option to do multi, that should be OK.
I also speculate that the expensive call is setting the locale, as that's probably ripping out strings in the android process and paging in new ones. Not sure if getting the pref is the expensive part.
Which leads me to, was the conditional and the pref handling working on the right conditions? Maybe it'd be cheap to keep, if we make sure the pref isn't set accidently. Which might be easier, if we restrict it to explicitly set within java code, and not an observer of stuff within gecko.
Depends on: 702302
Updated•13 years ago
|
tracking-fennec: --- → 11+
Comment 3•13 years ago
|
||
i am not working on these right now. resetting assignee.
Assignee: doug.turner → nobody
Updated•12 years ago
|
tracking-fennec: 11+ → ?
Updated•12 years ago
|
tracking-fennec: ? → -
Reporter | ||
Comment 4•11 years ago
|
||
I recall that Brad added more detail:
The costly part is reading the android pref.
Which makes all other optimizations mute. I'm wondering if there'd be a point in our startup path where we read prefs anyway and the getter may be cheap. In that scenario, we could be able to make startup regressions only affect users that actually do switch locales, which might be OK.
Assignee | ||
Comment 5•11 years ago
|
||
Things of note:
* That code ran in a background thread.
* And in the exact same runnable, we check for Sync migration, _write to prefs_ for our last launch time, and send some broadcasts to trigger FHR and product announcements.
* Reading a prefs file that's already been loaded, or will be loaded shortly thereafter, is essentially free.
In other words, I think there's a path forward here that doesn't impact main-thread perf.
Assignee | ||
Comment 6•11 years ago
|
||
We fixed this in Bug 936756.
Updated•11 years ago
|
Assignee: nobody → rnewman
Target Milestone: --- → Firefox 28
Updated•4 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•