Merge "Strong pointer fixes in libinputservice" into main

7 files changed, 60 insertions, 43 deletions

diff --git a/libs/input/Android.bp b/libs/input/Android.bp
index 6c0fd5f65359..5ce990fdeb82 100644
--- a/libs/input/Android.bp
+++ b/libs/input/Android.bp
@@ -23,6 +23,12 @@ package {
 
 cc_library_shared {
     name: "libinputservice",
+    defaults: [
+        // Build using the same flags and configurations as inputflinger.
+        "inputflinger_defaults",
+    ],
+    host_supported: false,
+
     srcs: [
         "PointerController.cpp",
         "PointerControllerContext.cpp",
@@ -50,12 +56,4 @@ cc_library_shared {
     ],
 
     include_dirs: ["frameworks/native/services"],
-
-    cflags: [
-        "-Wall",
-        "-Wextra",
-        "-Werror",
-        "-Wthread-safety",
-    ],
-
 }
diff --git a/libs/input/PointerController.cpp b/libs/input/PointerController.cpp
index e21d6fb2fe14..bb3d9d7c680c 100644
--- a/libs/input/PointerController.cpp
+++ b/libs/input/PointerController.cpp
@@ -103,7 +103,7 @@ PointerController::PointerController(const sp<PointerControllerPolicyInterface>&
                                      WindowListenerConsumer unregisterListener)
       : mContext(policy, looper, spriteController, *this),
         mCursorController(mContext),
-        mDisplayInfoListener(new DisplayInfoListener(this)),
+        mDisplayInfoListener(sp<DisplayInfoListener>::make(this)),
         mUnregisterWindowInfosListener(std::move(unregisterListener)) {
     std::scoped_lock lock(getLock());
     mLocked.presentation = Presentation::SPOT;
diff --git a/libs/input/PointerControllerContext.cpp b/libs/input/PointerControllerContext.cpp
index f30e8d8e33a5..c15451072f54 100644
--- a/libs/input/PointerControllerContext.cpp
+++ b/libs/input/PointerControllerContext.cpp
@@ -36,8 +36,8 @@ PointerControllerContext::PointerControllerContext(
       : mPolicy(policy),
         mLooper(looper),
         mSpriteController(spriteController),
-        mHandler(new MessageHandler()),
-        mCallback(new LooperCallback()),
+        mHandler(sp<MessageHandler>::make()),
+        mCallback(sp<LooperCallback>::make()),
         mController(controller),
         mAnimator(*this) {
     std::scoped_lock lock(mLock);
diff --git a/libs/input/SpriteController.cpp b/libs/input/SpriteController.cpp
index 130b204954b4..d40f49e4bc38 100644
--- a/libs/input/SpriteController.cpp
+++ b/libs/input/SpriteController.cpp
@@ -31,12 +31,19 @@ SpriteController::SpriteController(const sp<Looper>& looper, int32_t overlayLaye
                                    ParentSurfaceProvider parentSurfaceProvider)
       : mLooper(looper),
         mOverlayLayer(overlayLayer),
+        mHandler(sp<Handler>::make()),
         mParentSurfaceProvider(std::move(parentSurfaceProvider)) {
-    mHandler = new WeakMessageHandler(this);
     mLocked.transactionNestingCount = 0;
     mLocked.deferredSpriteUpdate = false;
 }
 
+void SpriteController::setHandlerController(const sp<android::SpriteController>& controller) {
+    // Initialize the weak message handler outside the constructor, because we cannot get a strong
+    // pointer to self in the constructor as the initial ref count is only incremented after
+    // construction.
+    mHandler->spriteController = controller;
+}
+
 SpriteController::~SpriteController() {
     mLooper->removeMessages(mHandler);
 
@@ -47,7 +54,7 @@ SpriteController::~SpriteController() {
 }
 
 sp<Sprite> SpriteController::createSprite() {
-    return new SpriteImpl(this);
+    return sp<SpriteImpl>::make(sp<SpriteController>::fromExisting(this));
 }
 
 void SpriteController::openTransaction() {
@@ -65,7 +72,7 @@ void SpriteController::closeTransaction() {
     mLocked.transactionNestingCount -= 1;
     if (mLocked.transactionNestingCount == 0 && mLocked.deferredSpriteUpdate) {
         mLocked.deferredSpriteUpdate = false;
-        mLooper->sendMessage(mHandler, Message(MSG_UPDATE_SPRITES));
+        mLooper->sendMessage(mHandler, Message(Handler::MSG_UPDATE_SPRITES));
     }
 }
 
@@ -76,7 +83,7 @@ void SpriteController::invalidateSpriteLocked(const sp<SpriteImpl>& sprite) {
         if (mLocked.transactionNestingCount != 0) {
             mLocked.deferredSpriteUpdate = true;
         } else {
-            mLooper->sendMessage(mHandler, Message(MSG_UPDATE_SPRITES));
+            mLooper->sendMessage(mHandler, Message(Handler::MSG_UPDATE_SPRITES));
         }
     }
 }
@@ -85,18 +92,7 @@ void SpriteController::disposeSurfaceLocked(const sp<SurfaceControl>& surfaceCon
     bool wasEmpty = mLocked.disposedSurfaces.empty();
     mLocked.disposedSurfaces.push_back(surfaceControl);
     if (wasEmpty) {
-        mLooper->sendMessage(mHandler, Message(MSG_DISPOSE_SURFACES));
-    }
-}
-
-void SpriteController::handleMessage(const Message& message) {
-    switch (message.what) {
-    case MSG_UPDATE_SPRITES:
-        doUpdateSprites();
-        break;
-    case MSG_DISPOSE_SURFACES:
-        doDisposeSurfaces();
-        break;
+        mLooper->sendMessage(mHandler, Message(Handler::MSG_DISPOSE_SURFACES));
     }
 }
 
@@ -327,7 +323,7 @@ void SpriteController::doDisposeSurfaces() {
 
 void SpriteController::ensureSurfaceComposerClient() {
     if (mSurfaceComposerClient == NULL) {
-        mSurfaceComposerClient = new SurfaceComposerClient();
+        mSurfaceComposerClient = sp<SurfaceComposerClient>::make();
     }
 }
 
@@ -353,12 +349,29 @@ sp<SurfaceControl> SpriteController::obtainSurface(int32_t width, int32_t height
     return surfaceControl;
 }
 
-// --- SpriteController::SpriteImpl ---
+// --- SpriteController::Handler ---
 
-SpriteController::SpriteImpl::SpriteImpl(const sp<SpriteController> controller) :
-        mController(controller) {
+void SpriteController::Handler::handleMessage(const android::Message& message) {
+    auto controller = spriteController.promote();
+    if (!controller) {
+        return;
+    }
+
+    switch (message.what) {
+        case MSG_UPDATE_SPRITES:
+            controller->doUpdateSprites();
+            break;
+        case MSG_DISPOSE_SURFACES:
+            controller->doDisposeSurfaces();
+            break;
+    }
 }
 
+// --- SpriteController::SpriteImpl ---
+
+SpriteController::SpriteImpl::SpriteImpl(const sp<SpriteController>& controller)
+      : mController(controller) {}
+
 SpriteController::SpriteImpl::~SpriteImpl() {
     AutoMutex _m(mController->mLock);
 
@@ -461,7 +474,7 @@ void SpriteController::SpriteImpl::invalidateLocked(uint32_t dirty) {
     mLocked.state.dirty |= dirty;
 
     if (!wasDirty) {
-        mController->invalidateSpriteLocked(this);
+        mController->invalidateSpriteLocked(sp<SpriteImpl>::fromExisting(this));
     }
 }
 
diff --git a/libs/input/SpriteController.h b/libs/input/SpriteController.h
index 1f113c045360..3144401257d3 100644
--- a/libs/input/SpriteController.h
+++ b/libs/input/SpriteController.h
@@ -109,7 +109,7 @@ public:
  *
  * Clients are responsible for animating sprites by periodically updating their properties.
  */
-class SpriteController : public MessageHandler {
+class SpriteController : public RefBase {
 protected:
     virtual ~SpriteController();
 
@@ -117,6 +117,9 @@ public:
     using ParentSurfaceProvider = std::function<sp<SurfaceControl>(int /*displayId*/)>;
     SpriteController(const sp<Looper>& looper, int32_t overlayLayer, ParentSurfaceProvider parent);
 
+    /* Initialize the callback for the message handler. */
+    void setHandlerController(const sp<SpriteController>& controller);
+
     /* Creates a new sprite, initially invisible. */
     virtual sp<Sprite> createSprite();
 
@@ -129,9 +132,12 @@ public:
     virtual void closeTransaction();
 
 private:
-    enum {
-        MSG_UPDATE_SPRITES,
-        MSG_DISPOSE_SURFACES,
+    class Handler : public virtual android::MessageHandler {
+    public:
+        enum { MSG_UPDATE_SPRITES, MSG_DISPOSE_SURFACES };
+
+        void handleMessage(const Message& message) override;
+        wp<SpriteController> spriteController;
     };
 
     enum {
@@ -192,7 +198,7 @@ private:
         virtual ~SpriteImpl();
 
     public:
-        explicit SpriteImpl(const sp<SpriteController> controller);
+        explicit SpriteImpl(const sp<SpriteController>& controller);
 
         virtual void setIcon(const SpriteIcon& icon);
         virtual void setVisible(bool visible);
@@ -245,7 +251,7 @@ private:
 
     sp<Looper> mLooper;
     const int32_t mOverlayLayer;
-    sp<WeakMessageHandler> mHandler;
+    sp<Handler> mHandler;
     ParentSurfaceProvider mParentSurfaceProvider;
 
     sp<SurfaceComposerClient> mSurfaceComposerClient;
@@ -260,7 +266,6 @@ private:
     void invalidateSpriteLocked(const sp<SpriteImpl>& sprite);
     void disposeSurfaceLocked(const sp<SurfaceControl>& surfaceControl);
 
-    void handleMessage(const Message& message);
     void doUpdateSprites();
     void doDisposeSurfaces();
 
diff --git a/libs/input/TouchSpotController.cpp b/libs/input/TouchSpotController.cpp
index d9fe5996bcff..c212608e4ea5 100644
--- a/libs/input/TouchSpotController.cpp
+++ b/libs/input/TouchSpotController.cpp
@@ -39,15 +39,15 @@ namespace android {
 
 // --- Spot ---
 
-void TouchSpotController::Spot::updateSprite(const SpriteIcon* icon, float x, float y,
+void TouchSpotController::Spot::updateSprite(const SpriteIcon* icon, float newX, float newY,
                                              int32_t displayId) {
     sprite->setLayer(Sprite::BASE_LAYER_SPOT + id);
     sprite->setAlpha(alpha);
     sprite->setTransformationMatrix(SpriteTransformationMatrix(scale, 0.0f, 0.0f, scale));
-    sprite->setPosition(x, y);
+    sprite->setPosition(newX, newY);
     sprite->setDisplayId(displayId);
-    this->x = x;
-    this->y = y;
+    x = newX;
+    y = newY;
 
     if (icon != mLastIcon) {
         mLastIcon = icon;
diff --git a/services/core/jni/com_android_server_input_InputManagerService.cpp b/services/core/jni/com_android_server_input_InputManagerService.cpp
index 7c8c558474bb..161943a2b5bf 100644
--- a/services/core/jni/com_android_server_input_InputManagerService.cpp
+++ b/services/core/jni/com_android_server_input_InputManagerService.cpp
@@ -747,6 +747,7 @@ void NativeInputManager::ensureSpriteControllerLocked() REQUIRES(mLock) {
         mLocked.spriteController = new SpriteController(mLooper, layer, [this](int displayId) {
             return getParentSurfaceForPointers(displayId);
         });
+        mLocked.spriteController->setHandlerController(mLocked.spriteController);
     }
 }