Bug 1287946 - Improve usage of StringParam in PrefsHelper; r=me
authorJim Chen <nchen@mozilla.com>
Thu, 21 Jul 2016 13:49:05 -0400
changeset 346242 44454776e07f7e8d47ea82d8d73171f4380520f8
parent 346241 a4baceb961e6c6dda640d93ab99ab167e363dbc7
child 346243 4842ac3d86e9905bd08a59859e3806d05787f4d2
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.