Bug 1287946 - Improve usage of StringParam in PrefsHelper; r=me
authorJim Chen <nchen@mozilla.com>
Thu, 21 Jul 2016 13:49:05 -0400
changeset 306136 44454776e07f7e8d47ea82d8d73171f4380520f8
parent 306135 a4baceb961e6c6dda640d93ab99ab167e363dbc7
child 306137 4842ac3d86e9905bd08a59859e3806d05787f4d2
push id79779
push usernchen@mozilla.com
push dateThu, 21 Jul 2016 17:49:29 +0000
treeherdermozilla-inbound@4842ac3d86e9 [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.