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 696771 1d7012af17e598d99557ee786c730bc2e5a6f452
parent 696770 9ac3f44d3cc7b259904b9fce8d4291103856f0a1
child 696772 8114c9ccfd4d421118e4adf690d1da370d23bbf6
push id88784
push usercholler@mozilla.com
push dateSat, 11 Nov 2017 10:21:53 +0000
reviewerssnorp
bugs1337078
milestone58.0a1
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());