Move PointerController from sp to shared_ptr
Bug: 160010896
Test: atest PointerController_test, manual usage
Change-Id: I4e665d00c56b44c9c1a4ea8cb27ffd10ade3315b
Merged-In: I4e665d00c56b44c9c1a4ea8cb27ffd10ade3315b
diff --git a/libs/input/PointerController.cpp b/libs/input/PointerController.cpp
index 5c2ef15..f749894 100644
--- a/libs/input/PointerController.cpp
+++ b/libs/input/PointerController.cpp
@@ -24,37 +24,10 @@
#include <log/log.h>
-#include <SkBitmap.h>
-#include <SkCanvas.h>
-#include <SkColor.h>
-#include <SkPaint.h>
-#include <SkBlendMode.h>
+#include <memory>
namespace android {
-// --- WeakLooperCallback ---
-
-class WeakLooperCallback: public LooperCallback {
-protected:
- virtual ~WeakLooperCallback() { }
-
-public:
- explicit WeakLooperCallback(const wp<LooperCallback>& callback) :
- mCallback(callback) {
- }
-
- virtual int handleEvent(int fd, int events, void* data) {
- sp<LooperCallback> callback = mCallback.promote();
- if (callback != NULL) {
- return callback->handleEvent(fd, events, data);
- }
- return 0; // the client is gone, remove the callback
- }
-
-private:
- wp<LooperCallback> mCallback;
-};
-
// --- PointerController ---
// Time to wait before starting the fade when the pointer is inactive.
@@ -70,21 +43,42 @@
// The number of events to be read at once for DisplayEventReceiver.
static const int EVENT_BUFFER_SIZE = 100;
-// --- PointerController ---
+std::shared_ptr<PointerController> PointerController::create(
+ const sp<PointerControllerPolicyInterface>& policy, const sp<Looper>& looper,
+ const sp<SpriteController>& spriteController) {
+ std::shared_ptr<PointerController> controller = std::shared_ptr<PointerController>(
+ new PointerController(policy, looper, spriteController));
-PointerController::PointerController(const sp<PointerControllerPolicyInterface>& policy,
- const sp<Looper>& looper, const sp<SpriteController>& spriteController) :
- mPolicy(policy), mLooper(looper), mSpriteController(spriteController) {
- mHandler = new WeakMessageHandler(this);
- mCallback = new WeakLooperCallback(this);
+ /*
+ * Now we need to hook up the constructed PointerController object to its callbacks.
+ *
+ * This must be executed after the constructor but before any other methods on PointerController
+ * in order to ensure that the fully constructed object is visible on the Looper thread, since
+ * that may be a different thread than where the PointerController is initially constructed.
+ *
+ * Unfortunately, this cannot be done as part of the constructor since we need to hand out
+ * weak_ptr's which themselves cannot be constructed until there's at least one shared_ptr.
+ */
- if (mDisplayEventReceiver.initCheck() == NO_ERROR) {
- mLooper->addFd(mDisplayEventReceiver.getFd(), Looper::POLL_CALLBACK,
- Looper::EVENT_INPUT, mCallback, nullptr);
+ controller->mHandler->pointerController = controller;
+ controller->mCallback->pointerController = controller;
+ if (controller->mDisplayEventReceiver.initCheck() == NO_ERROR) {
+ controller->mLooper->addFd(controller->mDisplayEventReceiver.getFd(), Looper::POLL_CALLBACK,
+ Looper::EVENT_INPUT, controller->mCallback, nullptr);
} else {
ALOGE("Failed to initialize DisplayEventReceiver.");
}
+ return controller;
+}
+PointerController::PointerController(const sp<PointerControllerPolicyInterface>& policy,
+ const sp<Looper>& looper,
+ const sp<SpriteController>& spriteController)
+ : mPolicy(policy),
+ mLooper(looper),
+ mSpriteController(spriteController),
+ mHandler(new MessageHandler()),
+ mCallback(new LooperCallback()) {
AutoMutex _l(mLock);
mLocked.animationPending = false;
@@ -486,24 +480,35 @@
updatePointerLocked();
}
-void PointerController::handleMessage(const Message& message) {
+void PointerController::MessageHandler::handleMessage(const Message& message) {
+ std::shared_ptr<PointerController> controller = pointerController.lock();
+
+ if (controller == nullptr) {
+ ALOGE("PointerController instance was released before processing message: what=%d",
+ message.what);
+ return;
+ }
switch (message.what) {
case MSG_INACTIVITY_TIMEOUT:
- doInactivityTimeout();
+ controller->doInactivityTimeout();
break;
}
}
-int PointerController::handleEvent(int /* fd */, int events, void* /* data */) {
+int PointerController::LooperCallback::handleEvent(int /* fd */, int events, void* /* data */) {
+ std::shared_ptr<PointerController> controller = pointerController.lock();
+ if (controller == nullptr) {
+ ALOGW("PointerController instance was released with pending callbacks. events=0x%x",
+ events);
+ return 0; // Remove the callback, the PointerController is gone anyways
+ }
if (events & (Looper::EVENT_ERROR | Looper::EVENT_HANGUP)) {
- ALOGE("Display event receiver pipe was closed or an error occurred. "
- "events=0x%x", events);
+ ALOGE("Display event receiver pipe was closed or an error occurred. events=0x%x", events);
return 0; // remove the callback
}
if (!(events & Looper::EVENT_INPUT)) {
- ALOGW("Received spurious callback for unhandled poll event. "
- "events=0x%x", events);
+ ALOGW("Received spurious callback for unhandled poll event. events=0x%x", events);
return 1; // keep the callback
}
@@ -511,7 +516,7 @@
ssize_t n;
nsecs_t timestamp;
DisplayEventReceiver::Event buf[EVENT_BUFFER_SIZE];
- while ((n = mDisplayEventReceiver.getEvents(buf, EVENT_BUFFER_SIZE)) > 0) {
+ while ((n = controller->mDisplayEventReceiver.getEvents(buf, EVENT_BUFFER_SIZE)) > 0) {
for (size_t i = 0; i < static_cast<size_t>(n); ++i) {
if (buf[i].header.type == DisplayEventReceiver::DISPLAY_EVENT_VSYNC) {
timestamp = buf[i].header.timestamp;
@@ -520,7 +525,7 @@
}
}
if (gotVsync) {
- doAnimate(timestamp);
+ controller->doAnimate(timestamp);
}
return 1; // keep the callback
}
@@ -737,7 +742,7 @@
return spot;
}
}
- return NULL;
+ return nullptr;
}
void PointerController::releaseSpotLocked(Spot* spot) {
diff --git a/libs/input/PointerController.h b/libs/input/PointerController.h
index ebc622b..ff5631b 100644
--- a/libs/input/PointerController.h
+++ b/libs/input/PointerController.h
@@ -17,19 +17,20 @@
#ifndef _UI_POINTER_CONTROLLER_H
#define _UI_POINTER_CONTROLLER_H
-#include "SpriteController.h"
-
-#include <map>
-#include <vector>
-
-#include <ui/DisplayInfo.h>
+#include <PointerControllerInterface.h>
+#include <gui/DisplayEventReceiver.h>
#include <input/DisplayViewport.h>
#include <input/Input.h>
-#include <PointerControllerInterface.h>
+#include <ui/DisplayInfo.h>
#include <utils/BitSet.h>
-#include <utils/RefBase.h>
#include <utils/Looper.h>
-#include <gui/DisplayEventReceiver.h>
+#include <utils/RefBase.h>
+
+#include <map>
+#include <memory>
+#include <vector>
+
+#include "SpriteController.h"
namespace android {
@@ -70,25 +71,22 @@
virtual int32_t getCustomPointerIconId() = 0;
};
-
/*
* Tracks pointer movements and draws the pointer sprite to a surface.
*
* Handles pointer acceleration and animation.
*/
-class PointerController : public PointerControllerInterface, public MessageHandler,
- public LooperCallback {
-protected:
- virtual ~PointerController();
-
+class PointerController : public PointerControllerInterface {
public:
+ static std::shared_ptr<PointerController> create(
+ const sp<PointerControllerPolicyInterface>& policy, const sp<Looper>& looper,
+ const sp<SpriteController>& spriteController);
enum InactivityTimeout {
INACTIVITY_TIMEOUT_NORMAL = 0,
INACTIVITY_TIMEOUT_SHORT = 1,
};
- PointerController(const sp<PointerControllerPolicyInterface>& policy,
- const sp<Looper>& looper, const sp<SpriteController>& spriteController);
+ virtual ~PointerController();
virtual bool getBounds(float* outMinX, float* outMinY,
float* outMaxX, float* outMaxY) const;
@@ -113,8 +111,8 @@
void reloadPointerResources();
private:
- static const size_t MAX_RECYCLED_SPRITES = 12;
- static const size_t MAX_SPOTS = 12;
+ static constexpr size_t MAX_RECYCLED_SPRITES = 12;
+ static constexpr size_t MAX_SPOTS = 12;
enum {
MSG_INACTIVITY_TIMEOUT,
@@ -130,8 +128,13 @@
float x, y;
inline Spot(uint32_t id, const sp<Sprite>& sprite)
- : id(id), sprite(sprite), alpha(1.0f), scale(1.0f),
- x(0.0f), y(0.0f), lastIcon(NULL) { }
+ : id(id),
+ sprite(sprite),
+ alpha(1.0f),
+ scale(1.0f),
+ x(0.0f),
+ y(0.0f),
+ lastIcon(nullptr) {}
void updateSprite(const SpriteIcon* icon, float x, float y, int32_t displayId);
@@ -139,12 +142,24 @@
const SpriteIcon* lastIcon;
};
+ class MessageHandler : public virtual android::MessageHandler {
+ public:
+ void handleMessage(const Message& message) override;
+ std::weak_ptr<PointerController> pointerController;
+ };
+
+ class LooperCallback : public virtual android::LooperCallback {
+ public:
+ int handleEvent(int fd, int events, void* data) override;
+ std::weak_ptr<PointerController> pointerController;
+ };
+
mutable Mutex mLock;
sp<PointerControllerPolicyInterface> mPolicy;
sp<Looper> mLooper;
sp<SpriteController> mSpriteController;
- sp<WeakMessageHandler> mHandler;
+ sp<MessageHandler> mHandler;
sp<LooperCallback> mCallback;
DisplayEventReceiver mDisplayEventReceiver;
@@ -181,14 +196,15 @@
int32_t buttonState;
std::map<int32_t /* displayId */, std::vector<Spot*>> spotsByDisplay;
- std::vector<sp<Sprite> > recycledSprites;
+ std::vector<sp<Sprite>> recycledSprites;
} mLocked GUARDED_BY(mLock);
+ PointerController(const sp<PointerControllerPolicyInterface>& policy, const sp<Looper>& looper,
+ const sp<SpriteController>& spriteController);
+
bool getBoundsLocked(float* outMinX, float* outMinY, float* outMaxX, float* outMaxY) const;
void setPositionLocked(float x, float y);
- void handleMessage(const Message& message);
- int handleEvent(int fd, int events, void* data);
void doAnimate(nsecs_t timestamp);
bool doFadingAnimationLocked(nsecs_t timestamp);
bool doBitmapAnimationLocked(nsecs_t timestamp);
diff --git a/libs/input/tests/PointerController_test.cpp b/libs/input/tests/PointerController_test.cpp
index a157426..1704436 100644
--- a/libs/input/tests/PointerController_test.cpp
+++ b/libs/input/tests/PointerController_test.cpp
@@ -136,7 +136,7 @@
sp<MockSprite> mPointerSprite;
sp<MockPointerControllerPolicyInterface> mPolicy;
sp<MockSpriteController> mSpriteController;
- sp<PointerController> mPointerController;
+ std::shared_ptr<PointerController> mPointerController;
private:
void loopThread();
@@ -160,7 +160,7 @@
EXPECT_CALL(*mSpriteController, createSprite())
.WillOnce(Return(mPointerSprite));
- mPointerController = new PointerController(mPolicy, mLooper, mSpriteController);
+ mPointerController = PointerController::create(mPolicy, mLooper, mSpriteController);
}
PointerControllerTest::~PointerControllerTest() {
diff --git a/services/core/jni/com_android_server_input_InputManagerService.cpp b/services/core/jni/com_android_server_input_InputManagerService.cpp
index 7aaa074..b7536b1 100644
--- a/services/core/jni/com_android_server_input_InputManagerService.cpp
+++ b/services/core/jni/com_android_server_input_InputManagerService.cpp
@@ -233,7 +233,7 @@
/* --- InputReaderPolicyInterface implementation --- */
virtual void getReaderConfiguration(InputReaderConfiguration* outConfig);
- virtual sp<PointerControllerInterface> obtainPointerController(int32_t deviceId);
+ virtual std::shared_ptr<PointerControllerInterface> obtainPointerController(int32_t deviceId);
virtual void notifyInputDevicesChanged(const std::vector<InputDeviceInfo>& inputDevices);
virtual sp<KeyCharacterMap> getKeyboardLayoutOverlay(const InputDeviceIdentifier& identifier);
virtual std::string getDeviceAlias(const InputDeviceIdentifier& identifier);
@@ -306,7 +306,7 @@
sp<SpriteController> spriteController;
// Pointer controller singleton, created and destroyed as needed.
- wp<PointerController> pointerController;
+ std::weak_ptr<PointerController> pointerController;
// Input devices to be disabled
SortedVector<int32_t> disabledInputDevices;
@@ -551,15 +551,16 @@
} // release lock
}
-sp<PointerControllerInterface> NativeInputManager::obtainPointerController(int32_t /* deviceId */) {
+std::shared_ptr<PointerControllerInterface> NativeInputManager::obtainPointerController(
+ int32_t /* deviceId */) {
ATRACE_CALL();
AutoMutex _l(mLock);
- sp<PointerController> controller = mLocked.pointerController.promote();
+ std::shared_ptr<PointerController> controller = mLocked.pointerController.lock();
if (controller == nullptr) {
ensureSpriteControllerLocked();
- controller = new PointerController(this, mLooper, mLocked.spriteController);
+ controller = PointerController::create(this, mLooper, mLocked.spriteController);
mLocked.pointerController = controller;
updateInactivityTimeoutLocked();
}
@@ -840,7 +841,7 @@
}
void NativeInputManager::updateInactivityTimeoutLocked() REQUIRES(mLock) {
- sp<PointerController> controller = mLocked.pointerController.promote();
+ std::shared_ptr<PointerController> controller = mLocked.pointerController.lock();
if (controller == nullptr) {
return;
}
@@ -928,7 +929,7 @@
void NativeInputManager::setPointerIconType(int32_t iconId) {
AutoMutex _l(mLock);
- sp<PointerController> controller = mLocked.pointerController.promote();
+ std::shared_ptr<PointerController> controller = mLocked.pointerController.lock();
if (controller != nullptr) {
controller->updatePointerIcon(iconId);
}
@@ -936,7 +937,7 @@
void NativeInputManager::reloadPointerIcons() {
AutoMutex _l(mLock);
- sp<PointerController> controller = mLocked.pointerController.promote();
+ std::shared_ptr<PointerController> controller = mLocked.pointerController.lock();
if (controller != nullptr) {
controller->reloadPointerResources();
}
@@ -944,7 +945,7 @@
void NativeInputManager::setCustomPointerIcon(const SpriteIcon& icon) {
AutoMutex _l(mLock);
- sp<PointerController> controller = mLocked.pointerController.promote();
+ std::shared_ptr<PointerController> controller = mLocked.pointerController.lock();
if (controller != nullptr) {
controller->setCustomPointerIcon(icon);
}