Bug 1287946 - Improve usage of StringParam in PrefsHelper; r=me
☠☠ backed out by d3a44f609b1f ☠ ☠
authorJim Chen <nchen@mozilla.com>
Thu, 21 Jul 2016 00:42:26 -0400
changeset 346046 e954e5a93482182eaddf52c9ba633d20c174c16e
parent 346045 684888aeee810cbf3ab28acdd4edb8e92d6cbc59
child 346047 0affe22555807bf2b349f94e115ce0f038ff989d
push id6389
push userraliiev@mozilla.com
push dateMon, 19 Sep 2016 13:38:22 +0000
treeherdermozilla-beta@01d67bfe6c81 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersme
bugs1287946
milestone50.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 1287946 - Improve usage of StringParam in PrefsHelper; r=me PrefsHelper uses Maybe<> as a replacement for a ternary expression, in order to work around jni::StringParam's lack of copy constructor. However, we can add a move constructor to StringParam, which lets us use it in a ternary expression, and avoid the awkwardness of Maybe<>.
widget/android/PrefsHelper.h
widget/android/jni/Refs.h
--- a/widget/android/PrefsHelper.h
+++ b/widget/android/PrefsHelper.h
@@ -7,20 +7,18 @@
 #define PrefsHelper_h
 
 #include "GeneratedJNINatives.h"
 #include "MainThreadUtils.h"
 #include "nsAppShell.h"
 #include "nsCOMPtr.h"
 #include "nsVariant.h"
 
-#include "mozilla/Maybe.h"
 #include "mozilla/Preferences.h"
 #include "mozilla/Services.h"
-#include "mozilla/UniquePtr.h"
 
 namespace mozilla {
 
 class PrefsHelper
     : public java::PrefsHelper::Natives<PrefsHelper>
     , public UsesGeckoThreadProxy
 {
     PrefsHelper() = delete;
@@ -63,30 +61,27 @@ class PrefsHelper
                 if (NS_FAILED(aVariant->GetAsAString(strVal))) {
                     return false;
                 }
                 break;
             default:
                 return false;
         }
 
-        Maybe<jni::StringParam> jstrVal;
-        jstrVal.emplace(nullptr);
-        if (type == java::PrefsHelper::PREF_STRING) {
-            jstrVal.reset();
-            jstrVal.emplace(strVal, aPrefName.Env());
-        }
+        jni::StringParam jstrVal(type == java::PrefsHelper::PREF_STRING ?
+                jni::StringParam(strVal, aPrefName.Env()) :
+                jni::StringParam(nullptr));
 
         if (aPrefHandler) {
             java::PrefsHelper::CallPrefHandler(
                     aPrefHandler, type, aPrefName,
-                    boolVal, intVal, jstrVal.ref());
+                    boolVal, intVal, jstrVal);
         } else {
             java::PrefsHelper::OnPrefChange(
-                    aPrefName, type, boolVal, intVal, jstrVal.ref());
+                    aPrefName, type, boolVal, intVal, jstrVal);
         }
         return true;
     }
 
     static bool SetVariantPref(nsIObserverService* aObsServ,
                                nsIWritableVariant* aVariant,
                                jni::String::Param aPrefName,
                                bool aFlush,
@@ -189,26 +184,21 @@ public:
                     if (!GetVariantPref(obsServ, value,
                                         aPrefHandler, nameStr)) {
                         NS_WARNING(nsPrintfCString("Failed to get pref %s",
                                                    name.get()).get());
                     }
                     continue;
             }
 
-            Maybe<jni::StringParam> jstrVal;
-            jstrVal.emplace(nullptr);
-            if (type == java::PrefsHelper::PREF_STRING) {
-                jstrVal.reset();
-                jstrVal.emplace(strVal, aCls.Env());
-            }
-
             java::PrefsHelper::CallPrefHandler(
-                    aPrefHandler, type, nameStr,
-                    boolVal, intVal, jstrVal.ref());
+                    aPrefHandler, type, nameStr, boolVal, intVal,
+                    jni::StringParam(type == java::PrefsHelper::PREF_STRING ?
+                        jni::StringParam(strVal, aCls.Env()) :
+                        jni::StringParam(nullptr)));
         }
 
         java::PrefsHelper::CallPrefHandler(
                 aPrefHandler, java::PrefsHelper::PREF_FINISH,
                 nullptr, false, 0, nullptr);
     }
 
     static void SetPref(jni::String::Param aPrefName,
@@ -318,23 +308,18 @@ public:
                 }
                 break;
             default:
                 NS_WARNING(nsPrintfCString("Invalid pref %s",
                                            name.get()).get());
                 return;
         }
 
-        Maybe<jni::StringParam> jstrVal;
-        jstrVal.emplace(nullptr);
-        if (type == java::PrefsHelper::PREF_STRING) {
-            jstrVal.reset();
-            jstrVal.emplace(strVal);
-        }
-
         java::PrefsHelper::OnPrefChange(
-                name, type, boolVal, intVal, jstrVal.ref());
+                name, type, boolVal, intVal,
+                jni::StringParam(type == java::PrefsHelper::PREF_STRING ?
+                    jni::StringParam(strVal) : jni::StringParam(nullptr)));
     }
 };
 
 } // namespace
 
 #endif // PrefsHelper_h
--- a/widget/android/jni/Refs.h
+++ b/widget/android/jni/Refs.h
@@ -675,19 +675,26 @@ public:
         , mEnv(env)
     {}
 
     MOZ_IMPLICIT StringParam(const char* str, JNIEnv* env = Ref::FindEnv())
         : Ref(GetString(env, NS_ConvertUTF8toUTF16(str)))
         , mEnv(env)
     {}
 
+    StringParam(StringParam&& other)
+        : Ref(other.Get())
+        , mEnv(other.mEnv)
+    {
+        other.mInstance = nullptr;
+    }
+
     ~StringParam()
     {
-        if (mEnv) {
+        if (mEnv && Get()) {
             mEnv->DeleteLocalRef(Get());
         }
     }
 
     operator String::LocalRef() const
     {
         // We can't return our existing ref because the returned
         // LocalRef could be freed first, so we need a new local ref.