Bug 1337078 - Improve the use of Android API in OSPreferences r=snorp
authorDylan Roeh <droeh@mozilla.com>
Fri, 10 Nov 2017 09:20:04 -0600
changeset 391378 1d7012af17e598d99557ee786c730bc2e5a6f452
parent 391377 9ac3f44d3cc7b259904b9fce8d4291103856f0a1
child 391379 8114c9ccfd4d421118e4adf690d1da370d23bbf6
push id32874
push userapavel@mozilla.com
push dateSat, 11 Nov 2017 09:59:48 +0000
treeherdermozilla-central@57eb0baf17ce [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssnorp
bugs1337078
milestone58.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1337078 - Improve the use of Android API in OSPreferences r=snorp Add BrowserLocaleManager.refreshLocales, a native function which calls OSPreferences::Refresh, and BrowserLocaleManager.getLocale, which returns the current locale string. Use these in place of observing modification of the intl.locale.os pref.
intl/locale/LocaleService.cpp
intl/locale/android/OSPreferences_android.cpp
mobile/android/base/java/org/mozilla/gecko/BrowserLocaleManager.java
mobile/android/geckoview/src/main/java/org/mozilla/gecko/GeckoAppShell.java
widget/android/GeneratedJNIWrappers.cpp
widget/android/GeneratedJNIWrappers.h
widget/android/fennec/FennecJNINatives.h
widget/android/fennec/FennecJNIWrappers.cpp
widget/android/fennec/FennecJNIWrappers.h
widget/android/nsAppShell.cpp
--- a/intl/locale/LocaleService.cpp
+++ b/intl/locale/LocaleService.cpp
@@ -19,24 +19,19 @@
 
 #include "unicode/uloc.h"
 
 #define INTL_SYSTEM_LOCALES_CHANGED "intl:system-locales-changed"
 
 #define MATCH_OS_LOCALE_PREF "intl.locale.matchOS"
 #define SELECTED_LOCALE_PREF "general.useragent.locale"
 
-//XXX: This pref is used only by Android and we use it to emulate
-//     retrieving OS locale until we get proper hook into JNI in bug 1337078.
-#define ANDROID_OS_LOCALE_PREF "intl.locale.os"
-
 static const char* kObservedPrefs[] = {
   MATCH_OS_LOCALE_PREF,
   SELECTED_LOCALE_PREF,
-  ANDROID_OS_LOCALE_PREF,
   nullptr
 };
 
 using namespace mozilla::intl;
 using namespace mozilla;
 
 NS_IMPL_ISUPPORTS(LocaleService, mozILocaleService, nsIObserver,
                   nsISupportsWeakReference)
@@ -570,26 +565,20 @@ LocaleService::Observe(nsISupports *aSub
                       const char16_t *aData)
 {
   MOZ_ASSERT(mIsServer, "This should only be called in the server mode.");
 
   if (!strcmp(aTopic, INTL_SYSTEM_LOCALES_CHANGED)) {
     RequestedLocalesChanged();
   } else {
     NS_ConvertUTF16toUTF8 pref(aData);
-
-    // This is a temporary solution until we get bug 1337078 landed.
-    if (pref.EqualsLiteral(ANDROID_OS_LOCALE_PREF)) {
-      OSPreferences::GetInstance()->Refresh();
-    }
     // At the moment the only thing we're observing are settings indicating
     // user requested locales.
     if (pref.EqualsLiteral(MATCH_OS_LOCALE_PREF) ||
-        pref.EqualsLiteral(SELECTED_LOCALE_PREF) ||
-        pref.EqualsLiteral(ANDROID_OS_LOCALE_PREF)) {
+        pref.EqualsLiteral(SELECTED_LOCALE_PREF)) {
       RequestedLocalesChanged();
     }
   }
 
   return NS_OK;
 }
 
 bool
--- a/intl/locale/android/OSPreferences_android.cpp
+++ b/intl/locale/android/OSPreferences_android.cpp
@@ -2,32 +2,35 @@
  *
  * This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "OSPreferences.h"
 #include "mozilla/Preferences.h"
 
+#include "FennecJNIWrappers.h"
+#include "GeneratedJNIWrappers.h"
+
 using namespace mozilla::intl;
 
 bool
 OSPreferences::ReadSystemLocales(nsTArray<nsCString>& aLocaleList)
 {
-  //XXX: This is a quite sizable hack to work around the fact that we cannot
-  //     retrieve OS locale in C++ without reaching out to JNI.
-  //     Once we fix this (bug 1337078), this hack should not be necessary.
-  //
+  if (!mozilla::jni::IsAvailable()) {
+    return false;
+  }
+
   //XXX: Notice, this value may be empty on an early read. In that case
   //     we won't add anything to the return list so that it doesn't get
   //     cached in mSystemLocales.
-  nsAutoCString locale;
-  Preferences::GetCString("intl.locale.os", locale);
-  if (!locale.IsEmpty()) {
-    aLocaleList.AppendElement(locale);
+  auto locale = mozilla::jni::IsFennec() ? java::BrowserLocaleManager::GetLocale() :
+                java::GeckoAppShell::GetDefaultLocale();
+  if (locale) {
+    aLocaleList.AppendElement(locale->ToCString());
     return true;
   }
   return false;
 }
 
 bool
 OSPreferences::ReadRegionalPrefsLocales(nsTArray<nsCString>& aLocaleList)
 {
--- a/mobile/android/base/java/org/mozilla/gecko/BrowserLocaleManager.java
+++ b/mobile/android/base/java/org/mozilla/gecko/BrowserLocaleManager.java
@@ -12,16 +12,17 @@ import java.util.Locale;
 import java.util.Set;
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.concurrent.atomic.AtomicReference;
 
 import org.json.JSONArray;
 import org.json.JSONException;
 import org.json.JSONObject;
 import org.mozilla.gecko.annotation.ReflectionTarget;
+import org.mozilla.gecko.annotation.WrapForJNI;
 import org.mozilla.gecko.util.GeckoBundle;
 import org.mozilla.gecko.util.GeckoJarReader;
 
 import android.content.BroadcastReceiver;
 import android.content.Context;
 import android.content.Intent;
 import android.content.IntentFilter;
 import android.content.SharedPreferences;
@@ -220,16 +221,22 @@ public class BrowserLocaleManager implem
         prefs.edit().putString("osLocale", osLocaleString).apply();
 
         // The value we send to Gecko should be a language tag, not
         // a Java locale string.
         final GeckoBundle data = new GeckoBundle(1);
         data.putString("languageTag", Locales.getLanguageTag(osLocale));
 
         EventDispatcher.getInstance().dispatch("Locale:OS", data);
+
+        if (GeckoThread.isRunning()) {
+            refreshLocales();
+        } else {
+            GeckoThread.queueNativeCall(BrowserLocaleManager.class, "refreshLocales");
+        }
     }
 
     @Override
     public String getAndApplyPersistedLocale(Context context) {
         initialize(context);
 
         final long t1 = android.os.SystemClock.uptimeMillis();
         final String localeCode = getPersistedLocale(context);
@@ -446,9 +453,23 @@ public class BrowserLocaleManager implem
     /**
      * @return the single default locale baked into this application.
      *         Applicable when there is no multilocale.json present.
      */
     @SuppressWarnings("static-method")
     public String getFallbackLocaleTag() {
         return FALLBACK_LOCALE_TAG;
     }
+
+    @WrapForJNI
+    public static native void refreshLocales();
+
+
+    @WrapForJNI
+    private static String getLocale() {
+        try {
+            return Locales.getLocaleManager().getCurrentLocale(GeckoAppShell.getApplicationContext()).toString();
+        } catch (NullPointerException e) {
+            Log.i(LOG_TAG, "Couldn't get current locale.");
+            return null;
+        }
+    }
 }
--- a/mobile/android/geckoview/src/main/java/org/mozilla/gecko/GeckoAppShell.java
+++ b/mobile/android/geckoview/src/main/java/org/mozilla/gecko/GeckoAppShell.java
@@ -15,16 +15,17 @@ import java.io.PipedInputStream;
 import java.io.PipedOutputStream;
 import java.net.MalformedURLException;
 import java.net.Proxy;
 import java.net.URLConnection;
 import java.nio.ByteBuffer;
 import java.util.ArrayList;
 import java.util.Iterator;
 import java.util.List;
+import java.util.Locale;
 import java.util.Map;
 import java.util.StringTokenizer;
 import java.util.TreeMap;
 
 import org.mozilla.gecko.annotation.JNITarget;
 import org.mozilla.gecko.annotation.RobocopTarget;
 import org.mozilla.gecko.annotation.WrapForJNI;
 import org.mozilla.gecko.gfx.BitmapUtils;
@@ -1859,9 +1860,14 @@ public class GeckoAppShell
             return 0;
         }
         final String prop = am.getProperty(AudioManager.PROPERTY_OUTPUT_SAMPLE_RATE);
         if (prop == null) {
             return 0;
         }
         return Integer.parseInt(prop);
     }
+
+    @WrapForJNI
+    public static String getDefaultLocale() {
+        return Locale.getDefault().toString();
+    }
 }
--- a/widget/android/GeneratedJNIWrappers.cpp
+++ b/widget/android/GeneratedJNIWrappers.cpp
@@ -313,16 +313,24 @@ auto GeckoAppShell::GetCurrentBatteryInf
 constexpr char GeckoAppShell::GetCurrentNetworkInformation_t::name[];
 constexpr char GeckoAppShell::GetCurrentNetworkInformation_t::signature[];
 
 auto GeckoAppShell::GetCurrentNetworkInformation() -> mozilla::jni::DoubleArray::LocalRef
 {
     return mozilla::jni::Method<GetCurrentNetworkInformation_t>::Call(GeckoAppShell::Context(), nullptr);
 }
 
+constexpr char GeckoAppShell::GetDefaultLocale_t::name[];
+constexpr char GeckoAppShell::GetDefaultLocale_t::signature[];
+
+auto GeckoAppShell::GetDefaultLocale() -> mozilla::jni::String::LocalRef
+{
+    return mozilla::jni::Method<GetDefaultLocale_t>::Call(GeckoAppShell::Context(), nullptr);
+}
+
 constexpr char GeckoAppShell::GetDensity_t::name[];
 constexpr char GeckoAppShell::GetDensity_t::signature[];
 
 auto GeckoAppShell::GetDensity() -> float
 {
     return mozilla::jni::Method<GetDensity_t>::Call(GeckoAppShell::Context(), nullptr);
 }
 
--- a/widget/android/GeneratedJNIWrappers.h
+++ b/widget/android/GeneratedJNIWrappers.h
@@ -893,16 +893,35 @@ public:
         static const mozilla::jni::CallingThread callingThread =
                 mozilla::jni::CallingThread::GECKO;
         static const mozilla::jni::DispatchTarget dispatchTarget =
                 mozilla::jni::DispatchTarget::CURRENT;
     };
 
     static auto GetCurrentNetworkInformation() -> mozilla::jni::DoubleArray::LocalRef;
 
+    struct GetDefaultLocale_t {
+        typedef GeckoAppShell Owner;
+        typedef mozilla::jni::String::LocalRef ReturnType;
+        typedef mozilla::jni::String::Param SetterType;
+        typedef mozilla::jni::Args<> Args;
+        static constexpr char name[] = "getDefaultLocale";
+        static constexpr char signature[] =
+                "()Ljava/lang/String;";
+        static const bool isStatic = true;
+        static const mozilla::jni::ExceptionMode exceptionMode =
+                mozilla::jni::ExceptionMode::ABORT;
+        static const mozilla::jni::CallingThread callingThread =
+                mozilla::jni::CallingThread::ANY;
+        static const mozilla::jni::DispatchTarget dispatchTarget =
+                mozilla::jni::DispatchTarget::CURRENT;
+    };
+
+    static auto GetDefaultLocale() -> mozilla::jni::String::LocalRef;
+
     struct GetDensity_t {
         typedef GeckoAppShell Owner;
         typedef float ReturnType;
         typedef float SetterType;
         typedef mozilla::jni::Args<> Args;
         static constexpr char name[] = "getDensity";
         static constexpr char signature[] =
                 "()F";
--- a/widget/android/fennec/FennecJNINatives.h
+++ b/widget/android/fennec/FennecJNINatives.h
@@ -34,16 +34,31 @@ const JNINativeMethod ANRReporter::Nativ
             ::template Wrap<&Impl::ReleaseNativeStack>),
 
     mozilla::jni::MakeNativeMethod<ANRReporter::RequestNativeStack_t>(
             mozilla::jni::NativeStub<ANRReporter::RequestNativeStack_t, Impl>
             ::template Wrap<&Impl::RequestNativeStack>)
 };
 
 template<class Impl>
+class BrowserLocaleManager::Natives : public mozilla::jni::NativeImpl<BrowserLocaleManager, Impl>
+{
+public:
+    static const JNINativeMethod methods[1];
+};
+
+template<class Impl>
+const JNINativeMethod BrowserLocaleManager::Natives<Impl>::methods[] = {
+
+    mozilla::jni::MakeNativeMethod<BrowserLocaleManager::RefreshLocales_t>(
+            mozilla::jni::NativeStub<BrowserLocaleManager::RefreshLocales_t, Impl>
+            ::template Wrap<&Impl::RefreshLocales>)
+};
+
+template<class Impl>
 class GeckoJavaSampler::Natives : public mozilla::jni::NativeImpl<GeckoJavaSampler, Impl>
 {
 public:
     static const JNINativeMethod methods[1];
 };
 
 template<class Impl>
 const JNINativeMethod GeckoJavaSampler::Natives<Impl>::methods[] = {
--- a/widget/android/fennec/FennecJNIWrappers.cpp
+++ b/widget/android/fennec/FennecJNIWrappers.cpp
@@ -19,16 +19,30 @@ constexpr char ANRReporter::GetNativeSta
 constexpr char ANRReporter::GetNativeStack_t::signature[];
 
 constexpr char ANRReporter::ReleaseNativeStack_t::name[];
 constexpr char ANRReporter::ReleaseNativeStack_t::signature[];
 
 constexpr char ANRReporter::RequestNativeStack_t::name[];
 constexpr char ANRReporter::RequestNativeStack_t::signature[];
 
+const char BrowserLocaleManager::name[] =
+        "org/mozilla/gecko/BrowserLocaleManager";
+
+constexpr char BrowserLocaleManager::GetLocale_t::name[];
+constexpr char BrowserLocaleManager::GetLocale_t::signature[];
+
+auto BrowserLocaleManager::GetLocale() -> mozilla::jni::String::LocalRef
+{
+    return mozilla::jni::Method<GetLocale_t>::Call(BrowserLocaleManager::Context(), nullptr);
+}
+
+constexpr char BrowserLocaleManager::RefreshLocales_t::name[];
+constexpr char BrowserLocaleManager::RefreshLocales_t::signature[];
+
 const char DownloadsIntegration::name[] =
         "org/mozilla/gecko/DownloadsIntegration";
 
 constexpr char DownloadsIntegration::GetTemporaryDownloadDirectory_t::name[];
 constexpr char DownloadsIntegration::GetTemporaryDownloadDirectory_t::signature[];
 
 auto DownloadsIntegration::GetTemporaryDownloadDirectory() -> mozilla::jni::String::LocalRef
 {
--- a/widget/android/fennec/FennecJNIWrappers.h
+++ b/widget/android/fennec/FennecJNIWrappers.h
@@ -74,16 +74,65 @@ public:
     };
 
     static const mozilla::jni::CallingThread callingThread =
             mozilla::jni::CallingThread::ANY;
 
     template<class Impl> class Natives;
 };
 
+class BrowserLocaleManager : public mozilla::jni::ObjectBase<BrowserLocaleManager>
+{
+public:
+    static const char name[];
+
+    explicit BrowserLocaleManager(const Context& ctx) : ObjectBase<BrowserLocaleManager>(ctx) {}
+
+    struct GetLocale_t {
+        typedef BrowserLocaleManager Owner;
+        typedef mozilla::jni::String::LocalRef ReturnType;
+        typedef mozilla::jni::String::Param SetterType;
+        typedef mozilla::jni::Args<> Args;
+        static constexpr char name[] = "getLocale";
+        static constexpr char signature[] =
+                "()Ljava/lang/String;";
+        static const bool isStatic = true;
+        static const mozilla::jni::ExceptionMode exceptionMode =
+                mozilla::jni::ExceptionMode::ABORT;
+        static const mozilla::jni::CallingThread callingThread =
+                mozilla::jni::CallingThread::ANY;
+        static const mozilla::jni::DispatchTarget dispatchTarget =
+                mozilla::jni::DispatchTarget::CURRENT;
+    };
+
+    static auto GetLocale() -> mozilla::jni::String::LocalRef;
+
+    struct RefreshLocales_t {
+        typedef BrowserLocaleManager Owner;
+        typedef void ReturnType;
+        typedef void SetterType;
+        typedef mozilla::jni::Args<> Args;
+        static constexpr char name[] = "refreshLocales";
+        static constexpr char signature[] =
+                "()V";
+        static const bool isStatic = true;
+        static const mozilla::jni::ExceptionMode exceptionMode =
+                mozilla::jni::ExceptionMode::ABORT;
+        static const mozilla::jni::CallingThread callingThread =
+                mozilla::jni::CallingThread::ANY;
+        static const mozilla::jni::DispatchTarget dispatchTarget =
+                mozilla::jni::DispatchTarget::CURRENT;
+    };
+
+    static const mozilla::jni::CallingThread callingThread =
+            mozilla::jni::CallingThread::ANY;
+
+    template<class Impl> class Natives;
+};
+
 class DownloadsIntegration : public mozilla::jni::ObjectBase<DownloadsIntegration>
 {
 public:
     static const char name[];
 
     explicit DownloadsIntegration(const Context& ctx) : ObjectBase<DownloadsIntegration>(ctx) {}
 
     struct GetTemporaryDownloadDirectory_t {
--- a/widget/android/nsAppShell.cpp
+++ b/widget/android/nsAppShell.cpp
@@ -32,16 +32,17 @@
 #include "nsGeoPosition.h"
 
 #include "mozilla/ArrayUtils.h"
 #include "mozilla/Telemetry.h"
 #include "mozilla/Services.h"
 #include "mozilla/Preferences.h"
 #include "mozilla/Hal.h"
 #include "mozilla/dom/TabChild.h"
+#include "mozilla/intl/OSPreferences.h"
 #include "prenv.h"
 
 #include "AndroidBridge.h"
 #include "AndroidBridgeUtilities.h"
 #include "GeneratedJNINatives.h"
 #include <android/log.h>
 #include <pthread.h>
 #include <wchar.h>
@@ -375,16 +376,28 @@ public:
         }
 
         widget::AndroidAlerts::NotifyListener(
                 aName->ToString(), aTopic->ToCString().get(),
                 aCookie->ToString().get());
     }
 };
 
+
+class BrowserLocaleManagerSupport final
+  : public java::BrowserLocaleManager::Natives<BrowserLocaleManagerSupport>
+{
+public:
+  static void RefreshLocales()
+  {
+    intl::OSPreferences::GetInstance()->Refresh();
+  }
+};
+
+
 nsAppShell::nsAppShell()
     : mSyncRunFinished(*(sAppShellLock = new Mutex("nsAppShell")),
                        "nsAppShell.SyncRun")
     , mSyncRunQuit(false)
 {
     {
         MutexAutoLock lock(*sAppShellLock);
         sAppShell = this;
@@ -407,16 +420,17 @@ nsAppShell::nsAppShell()
         mozilla::GeckoNetworkManager::Init();
         mozilla::GeckoProcessManager::Init();
         mozilla::GeckoScreenOrientation::Init();
         mozilla::PrefsHelper::Init();
         mozilla::GeckoVRManager::Init();
         nsWindow::InitNatives();
 
         if (jni::IsFennec()) {
+            BrowserLocaleManagerSupport::Init();
             mozilla::ANRReporter::Init();
             mozilla::MemoryMonitor::Init();
             mozilla::widget::Telemetry::Init();
             mozilla::ThumbnailHelper::Init();
         }
 
         java::GeckoThread::SetState(java::GeckoThread::State::JNI_READY());