Avoid creating sp<> from a pointer

The recommended practice is to use sp<>::make,
sp<>::fromExisting, or making copies from other sp<>.
This is to avoid various edge cases with RefBase.

Set ANDROID_UTILS_REF_BASE_DISABLE_IMPLICIT_CONSTRUCTION for
libaudioeffect_jni to catch non-recommended usages
as compile time.

Bug: 243364224
Test: m
Change-Id: I697b33bd701b324da44ae4694ca7a1ec0e9afe77

5 files changed, 15 insertions, 12 deletions

diff --git a/media/jni/audioeffect/Android.bp b/media/jni/audioeffect/Android.bp
index 2ddfacf3884a..8b5b726fd2db 100644
--- a/media/jni/audioeffect/Android.bp
+++ b/media/jni/audioeffect/Android.bp
@@ -28,7 +28,7 @@ cc_library_shared {
         "libaudioclient",
         "libaudioutils",
         "libaudiofoundation",
-        "libbinder"
+        "libbinder",
     ],
 
     export_shared_lib_headers: [
@@ -42,6 +42,7 @@ cc_library_shared {
         "-Werror",
         "-Wunused",
         "-Wunreachable-code",
+        "-DANDROID_UTILS_REF_BASE_DISABLE_IMPLICIT_CONSTRUCTION",
     ],
 
     // Workaround Clang LTO crash.
diff --git a/media/jni/audioeffect/Visualizer.cpp b/media/jni/audioeffect/Visualizer.cpp
index d0f1ec6064c1..09c45ea97e9d 100644
--- a/media/jni/audioeffect/Visualizer.cpp
+++ b/media/jni/audioeffect/Visualizer.cpp
@@ -142,7 +142,8 @@ status_t Visualizer::setCaptureCallBack(capture_cbk_t cbk, void* user, uint32_t
     mCaptureRate = rate;
 
     if (cbk != NULL) {
-        mCaptureThread = new CaptureThread(this, rate, ((flags & CAPTURE_CALL_JAVA) != 0));
+        mCaptureThread = sp<CaptureThread>::make(
+                sp<Visualizer>::fromExisting(this), rate, ((flags & CAPTURE_CALL_JAVA) != 0));
     }
     ALOGV("setCaptureCallBack() rate: %d thread %p flags 0x%08x",
             rate, mCaptureThread.get(), mCaptureFlags);
@@ -439,7 +440,7 @@ void Visualizer::controlStatusChanged(bool controlGranted) {
 
 //-------------------------------------------------------------------------
 
-Visualizer::CaptureThread::CaptureThread(Visualizer* receiver, uint32_t captureRate,
+Visualizer::CaptureThread::CaptureThread(const sp<Visualizer>& receiver, uint32_t captureRate,
         bool bCanCallJava)
     : Thread(bCanCallJava), mReceiver(receiver)
 {
diff --git a/media/jni/audioeffect/Visualizer.h b/media/jni/audioeffect/Visualizer.h
index 3d5d74a99d0e..b38c01f62cf1 100644
--- a/media/jni/audioeffect/Visualizer.h
+++ b/media/jni/audioeffect/Visualizer.h
@@ -157,7 +157,8 @@ private:
     class CaptureThread : public Thread
     {
     public:
-        CaptureThread(Visualizer* visualizer, uint32_t captureRate, bool bCanCallJava = false);
+        CaptureThread(const sp<Visualizer>& visualizer,
+                uint32_t captureRate, bool bCanCallJava = false);
 
     private:
         friend class Visualizer;
diff --git a/media/jni/audioeffect/android_media_AudioEffect.cpp b/media/jni/audioeffect/android_media_AudioEffect.cpp
index 2fb85a7f4b92..63e48aa622d0 100644
--- a/media/jni/audioeffect/android_media_AudioEffect.cpp
+++ b/media/jni/audioeffect/android_media_AudioEffect.cpp
@@ -205,15 +205,15 @@ static sp<AudioEffect> getAudioEffect(JNIEnv* env, jobject thiz)
     Mutex::Autolock l(sLock);
     AudioEffect* const ae =
             (AudioEffect*)env->GetLongField(thiz, fields.fidNativeAudioEffect);
-    return sp<AudioEffect>(ae);
+    return sp<AudioEffect>::fromExisting(ae);
 }
 
 static sp<AudioEffect> setAudioEffect(JNIEnv* env, jobject thiz,
                                     const sp<AudioEffect>& ae)
 {
     Mutex::Autolock l(sLock);
-    sp<AudioEffect> old =
-            (AudioEffect*)env->GetLongField(thiz, fields.fidNativeAudioEffect);
+    sp<AudioEffect> old = sp<AudioEffect>::fromExisting(
+            (AudioEffect*)env->GetLongField(thiz, fields.fidNativeAudioEffect));
     if (ae.get()) {
         ae->incStrong((void*)setAudioEffect);
     }
@@ -347,8 +347,8 @@ android_media_AudioEffect_native_setup(JNIEnv *env, jobject thiz, jobject weak_t
     // create the native AudioEffect object
     parcel = parcelForJavaObject(env, jAttributionSource);
     attributionSource.readFromParcel(parcel);
-    lpAudioEffect = new AudioEffect(attributionSource);
-    if (lpAudioEffect == 0) {
+    lpAudioEffect = sp<AudioEffect>::make(attributionSource);
+    if (lpAudioEffect == 0) {  // FIXME: I don't think this is actually possible.
         ALOGE("Error creating AudioEffect");
         goto setup_failure;
     }
diff --git a/media/jni/audioeffect/android_media_Visualizer.cpp b/media/jni/audioeffect/android_media_Visualizer.cpp
index 7b00f9392395..8cd6dfa02aa6 100644
--- a/media/jni/audioeffect/android_media_Visualizer.cpp
+++ b/media/jni/audioeffect/android_media_Visualizer.cpp
@@ -251,15 +251,15 @@ static sp<Visualizer> getVisualizer(JNIEnv* env, jobject thiz)
     Mutex::Autolock l(sLock);
     Visualizer* const v =
             (Visualizer*)env->GetLongField(thiz, fields.fidNativeVisualizer);
-    return sp<Visualizer>(v);
+    return sp<Visualizer>::fromExisting(v);
 }
 
 static sp<Visualizer> setVisualizer(JNIEnv* env, jobject thiz,
                                     const sp<Visualizer>& v)
 {
     Mutex::Autolock l(sLock);
-    sp<Visualizer> old =
-            (Visualizer*)env->GetLongField(thiz, fields.fidNativeVisualizer);
+    sp<Visualizer> old = sp<Visualizer>::fromExisting(
+            (Visualizer*)env->GetLongField(thiz, fields.fidNativeVisualizer));
     if (v.get()) {
         v->incStrong((void*)setVisualizer);
     }