stagefright: use handler instead of handler-id in AMessage

This avoids locking gLooperRoster mutex on post() and deliver().

Bug: 19607784
Change-Id: If6d9d7884dbb08fc390983bda896d223803476ba

4 files changed, 82 insertions, 93 deletions

diff --git a/media/libstagefright/foundation/AHandler.cpp b/media/libstagefright/foundation/AHandler.cpp
index bd5f7e9..7dbbe54 100644
--- a/media/libstagefright/foundation/AHandler.cpp
+++ b/media/libstagefright/foundation/AHandler.cpp
@@ -19,15 +19,23 @@
 #include <utils/Log.h>
 
 #include <media/stagefright/foundation/AHandler.h>
-
-#include <media/stagefright/foundation/ALooperRoster.h>
+#include <media/stagefright/foundation/AMessage.h>
 
 namespace android {
 
-sp<ALooper> AHandler::looper() {
-    extern ALooperRoster gLooperRoster;
+void AHandler::deliverMessage(const sp<AMessage> &msg) {
+    onMessageReceived(msg);
+    mMessageCounter++;
 
-    return gLooperRoster.findLooper(id());
+    if (mVerboseStats) {
+        uint32_t what = msg->what();
+        ssize_t idx = mMessages.indexOfKey(what);
+        if (idx < 0) {
+            mMessages.add(what, 1);
+        } else {
+            mMessages.editValueAt(idx)++;
+        }
+    }
 }
 
 }  // namespace android
diff --git a/media/libstagefright/foundation/ALooper.cpp b/media/libstagefright/foundation/ALooper.cpp
index 88b1c92..617d32b 100644
--- a/media/libstagefright/foundation/ALooper.cpp
+++ b/media/libstagefright/foundation/ALooper.cpp
@@ -210,7 +210,7 @@ bool ALooper::loop() {
         mEventQueue.erase(mEventQueue.begin());
     }
 
-    gLooperRoster.deliverMessage(event.mMessage);
+    event.mMessage->deliver();
 
     // NOTE: It's important to note that at this point our "ALooper" object
     // may no longer exist (its final reference may have gone away while
diff --git a/media/libstagefright/foundation/ALooperRoster.cpp b/media/libstagefright/foundation/ALooperRoster.cpp
index 2d57aee..3484579 100644
--- a/media/libstagefright/foundation/ALooperRoster.cpp
+++ b/media/libstagefright/foundation/ALooperRoster.cpp
@@ -49,7 +49,7 @@ ALooper::handler_id ALooperRoster::registerHandler(
     ALooper::handler_id handlerID = mNextHandlerID++;
     mHandlers.add(handlerID, info);
 
-    handler->setID(handlerID);
+    handler->setID(handlerID, looper);
 
     return handlerID;
 }
@@ -68,7 +68,7 @@ void ALooperRoster::unregisterHandler(ALooper::handler_id handlerID) {
     sp<AHandler> handler = info.mHandler.promote();
 
     if (handler != NULL) {
-        handler->setID(0);
+        handler->setID(0, NULL);
     }
 
     mHandlers.removeItemsAt(index);
@@ -100,96 +100,35 @@ void ALooperRoster::unregisterStaleHandlers() {
     }
 }
 
-status_t ALooperRoster::postMessage(
-        const sp<AMessage> &msg, int64_t delayUs) {
-
-    sp<ALooper> looper = findLooper(msg->target());
-
-    if (looper == NULL) {
-        return -ENOENT;
-    }
-    looper->post(msg, delayUs);
-    return OK;
-}
-
-void ALooperRoster::deliverMessage(const sp<AMessage> &msg) {
-    sp<AHandler> handler;
-
-    {
-        Mutex::Autolock autoLock(mLock);
-
-        ssize_t index = mHandlers.indexOfKey(msg->target());
-
-        if (index < 0) {
-            ALOGW("failed to deliver message. Target handler not registered.");
-            return;
-        }
-
-        const HandlerInfo &info = mHandlers.valueAt(index);
-        handler = info.mHandler.promote();
-
-        if (handler == NULL) {
-            ALOGW("failed to deliver message. "
-                 "Target handler %d registered, but object gone.",
-                 msg->target());
-
-            mHandlers.removeItemsAt(index);
-            return;
-        }
-    }
-
-    handler->onMessageReceived(msg);
-    handler->mMessageCounter++;
-
-    if (verboseStats) {
-        uint32_t what = msg->what();
-        ssize_t idx = handler->mMessages.indexOfKey(what);
-        if (idx < 0) {
-            handler->mMessages.add(what, 1);
-        } else {
-            handler->mMessages.editValueAt(idx)++;
-        }
-    }
-}
-
-sp<ALooper> ALooperRoster::findLooper(ALooper::handler_id handlerID) {
+void ALooperRoster::getHandlerAndLooper(
+        ALooper::handler_id handlerID, wp<AHandler> *handler, wp<ALooper> *looper) {
     Mutex::Autolock autoLock(mLock);
 
     ssize_t index = mHandlers.indexOfKey(handlerID);
 
     if (index < 0) {
-        return NULL;
-    }
-
-    sp<ALooper> looper = mHandlers.valueAt(index).mLooper.promote();
-
-    if (looper == NULL) {
-        mHandlers.removeItemsAt(index);
-        return NULL;
+        handler->clear();
+        looper->clear();
+        return;
     }
 
-    return looper;
+    *handler = mHandlers.valueAt(index).mHandler;
+    *looper = mHandlers.valueAt(index).mLooper;
 }
 
 status_t ALooperRoster::postAndAwaitResponse(
         const sp<AMessage> &msg, sp<AMessage> *response) {
-    sp<ALooper> looper = findLooper(msg->target());
-
-    if (looper == NULL) {
-        ALOGW("failed to post message. "
-                "Target handler %d still registered, but object gone.",
-                msg->target());
-        response->clear();
-        return -ENOENT;
-    }
-
     Mutex::Autolock autoLock(mLock);
 
     uint32_t replyID = mNextReplyID++;
 
     msg->setInt32("replyID", replyID);
 
-    looper->post(msg, 0 /* delayUs */);
+    status_t err = msg->post(0 /* delayUs */);
+    if (err != OK) {
+        response->clear();
+        return err;
+    }
 
     ssize_t index;
     while ((index = mReplies.indexOfKey(replyID)) < 0) {
@@ -225,7 +164,7 @@ static void makeFourCC(uint32_t fourcc, char *s) {
 void ALooperRoster::dump(int fd, const Vector<String16>& args) {
     bool clear = false;
     bool oldVerbose = verboseStats;
-    for (size_t i = 0;i < args.size(); i++) {
+    for (size_t i = 0; i < args.size(); i++) {
         if (args[i] == String16("-c")) {
             clear = true;
         } else if (args[i] == String16("-von")) {
@@ -241,22 +180,23 @@ void ALooperRoster::dump(int fd, const Vector<String16>& args) {
 
     Mutex::Autolock autoLock(mLock);
     size_t n = mHandlers.size();
-    s.appendFormat(" %zd registered handlers:\n", n);
+    s.appendFormat(" %zu registered handlers:\n", n);
 
     for (size_t i = 0; i < n; i++) {
-        s.appendFormat("  %zd: ", i);
+        s.appendFormat("  %d: ", mHandlers.keyAt(i));
         HandlerInfo &info = mHandlers.editValueAt(i);
         sp<ALooper> looper = info.mLooper.promote();
         if (looper != NULL) {
-            s.append(looper->mName.c_str());
+            s.append(looper->getName());
             sp<AHandler> handler = info.mHandler.promote();
             if (handler != NULL) {
+                handler->mVerboseStats = verboseStats;
                 s.appendFormat(": %u messages processed", handler->mMessageCounter);
                 if (verboseStats) {
                     for (size_t j = 0; j < handler->mMessages.size(); j++) {
                         char fourcc[15];
                         makeFourCC(handler->mMessages.keyAt(j), fourcc);
-                        s.appendFormat("\n    %s: %d",
+                        s.appendFormat("\n    %s: %u",
                                 fourcc,
                                 handler->mMessages.valueAt(j));
                     }
diff --git a/media/libstagefright/foundation/AMessage.cpp b/media/libstagefright/foundation/AMessage.cpp
index 1f46bc9..fbc321f 100644
--- a/media/libstagefright/foundation/AMessage.cpp
+++ b/media/libstagefright/foundation/AMessage.cpp
@@ -27,6 +27,7 @@
 #include "ABuffer.h"
 #include "ADebug.h"
 #include "ALooperRoster.h"
+#include "AHandler.h"
 #include "AString.h"
 
 #include <binder/Parcel.h>
@@ -36,10 +37,23 @@ namespace android {
 
 extern ALooperRoster gLooperRoster;
 
+AMessage::AMessage(void)
+    : mWhat(0),
+      mTarget(0),
+      mNumItems(0) {
+}
+
 AMessage::AMessage(uint32_t what, ALooper::handler_id target)
     : mWhat(what),
-      mTarget(target),
+      mTarget(0),
+      mNumItems(0) {
+    setTarget(target);
+}
+
+AMessage::AMessage(uint32_t what, const sp<const AHandler> &handler)
+    : mWhat(what),
       mNumItems(0) {
+    setTarget(handler);
 }
 
 AMessage::~AMessage() {
@@ -56,10 +70,19 @@ uint32_t AMessage::what() const {
 
 void AMessage::setTarget(ALooper::handler_id handlerID) {
     mTarget = handlerID;
+    gLooperRoster.getHandlerAndLooper(handlerID, &mHandler, &mLooper);
 }
 
-ALooper::handler_id AMessage::target() const {
-    return mTarget;
+void AMessage::setTarget(const sp<const AHandler> &handler) {
+    if (handler == NULL) {
+        mTarget = 0;
+        mHandler.clear();
+        mLooper.clear();
+    } else {
+        mTarget = handler->id();
+        mHandler = handler->getHandler();
+        mLooper = handler->getLooper();
+    }
 }
 
 void AMessage::clear() {
@@ -322,8 +345,25 @@ bool AMessage::findRect(
     return true;
 }
 
-void AMessage::post(int64_t delayUs) {
-    gLooperRoster.postMessage(this, delayUs);
+void AMessage::deliver() {
+    sp<AHandler> handler = mHandler.promote();
+    if (handler == NULL) {
+        ALOGW("failed to deliver message as target handler %d is gone.", mTarget);
+        return;
+    }
+
+    handler->deliverMessage(this);
+}
+
+status_t AMessage::post(int64_t delayUs) {
+    sp<ALooper> looper = mLooper.promote();
+    if (looper == NULL) {
+        ALOGW("failed to post message as target looper for handler %d is gone.", mTarget);
+        return -ENOENT;
+    }
+
+    looper->post(this, delayUs);
+    return OK;
 }
 
 status_t AMessage::postAndAwaitResponse(sp<AMessage> *response) {
@@ -348,7 +388,7 @@ bool AMessage::senderAwaitsResponse(uint32_t *replyID) const {
 }
 
 sp<AMessage> AMessage::dup() const {
-    sp<AMessage> msg = new AMessage(mWhat, mTarget);
+    sp<AMessage> msg = new AMessage(mWhat, mHandler.promote());
     msg->mNumItems = mNumItems;
 
 #ifdef DUMP_STATS
@@ -532,7 +572,8 @@ AString AMessage::debugString(int32_t indent) const {
 // static
 sp<AMessage> AMessage::FromParcel(const Parcel &parcel) {
     int32_t what = parcel.readInt32();
-    sp<AMessage> msg = new AMessage(what);
+    sp<AMessage> msg = new AMessage();
+    msg->setWhat(what);
 
     msg->mNumItems = static_cast<size_t>(parcel.readInt32());
     for (size_t i = 0; i < msg->mNumItems; ++i) {