Merge "libbinder: extension mechanism for all binders"

am: f8fb40c0cf

Change-Id: I5bc6fd00b120c1edc87a8e61c238897fddceb724

4 files changed, 106 insertions, 0 deletions

diff --git a/libs/binder/Binder.cpp b/libs/binder/Binder.cpp
index f3483ca8c3..693045e7f3 100644
--- a/libs/binder/Binder.cpp
+++ b/libs/binder/Binder.cpp
@@ -81,6 +81,24 @@ status_t IBinder::shellCommand(const sp<IBinder>& target, int in, int out, int e
     return target->transact(SHELL_COMMAND_TRANSACTION, send, &reply);
 }
 
+status_t IBinder::getExtension(sp<IBinder>* out) {
+    BBinder* local = this->localBinder();
+    if (local != nullptr) {
+        *out = local->getExtension();
+        return OK;
+    }
+
+    BpBinder* proxy = this->remoteBinder();
+    LOG_ALWAYS_FATAL_IF(proxy == nullptr);
+
+    Parcel data;
+    Parcel reply;
+    status_t status = transact(EXTENSION_TRANSACTION, data, &reply);
+    if (status != OK) return status;
+
+    return reply.readNullableStrongBinder(out);
+}
+
 // ---------------------------------------------------------------------------
 
 class BBinder::Extras
@@ -88,6 +106,7 @@ class BBinder::Extras
 public:
     // unlocked objects
     bool mRequestingSid = false;
+    sp<IBinder> mExtension;
 
     // for below objects
     Mutex mLock;
@@ -130,6 +149,9 @@ status_t BBinder::transact(
         case PING_TRANSACTION:
             err = pingBinder();
             break;
+        case EXTENSION_TRANSACTION:
+            err = reply->writeStrongBinder(getExtension());
+            break;
         default:
             err = onTransact(code, data, reply, flags);
             break;
@@ -222,6 +244,17 @@ void BBinder::setRequestingSid(bool requestingSid)
     e->mRequestingSid = requestingSid;
 }
 
+sp<IBinder> BBinder::getExtension() {
+    Extras* e = mExtras.load(std::memory_order_acquire);
+    if (e == nullptr) return nullptr;
+    return e->mExtension;
+}
+
+void BBinder::setExtension(const sp<IBinder>& extension) {
+    Extras* e = getOrCreateExtras();
+    e->mExtension = extension;
+}
+
 BBinder::~BBinder()
 {
     Extras* e = mExtras.load(std::memory_order_relaxed);
diff --git a/libs/binder/include/binder/Binder.h b/libs/binder/include/binder/Binder.h
index dec75f5a64..1095c7f28c 100644
--- a/libs/binder/include/binder/Binder.h
+++ b/libs/binder/include/binder/Binder.h
@@ -64,6 +64,10 @@ public:
     // This must be called before the object is sent to another process. Not thread safe.
     void                setRequestingSid(bool requestSid);
 
+    sp<IBinder>         getExtension();
+    // This must be called before the object is sent to another process. Not thread safe.
+    void                setExtension(const sp<IBinder>& extension);
+
 protected:
     virtual             ~BBinder();
 
diff --git a/libs/binder/include/binder/IBinder.h b/libs/binder/include/binder/IBinder.h
index aa44285b6e..408037e711 100644
--- a/libs/binder/include/binder/IBinder.h
+++ b/libs/binder/include/binder/IBinder.h
@@ -59,6 +59,7 @@ public:
         SHELL_COMMAND_TRANSACTION = B_PACK_CHARS('_','C','M','D'),
         INTERFACE_TRANSACTION   = B_PACK_CHARS('_', 'N', 'T', 'F'),
         SYSPROPS_TRANSACTION    = B_PACK_CHARS('_', 'S', 'P', 'R'),
+        EXTENSION_TRANSACTION   = B_PACK_CHARS('_', 'E', 'X', 'T'),
 
         // Corresponds to TF_ONE_WAY -- an asynchronous call.
         FLAG_ONEWAY             = 0x00000001
@@ -86,6 +87,49 @@ public:
                                          Vector<String16>& args, const sp<IShellCallback>& callback,
                                          const sp<IResultReceiver>& resultReceiver);
 
+    /**
+     * This allows someone to add their own additions to an interface without
+     * having to modify the original interface.
+     *
+     * For instance, imagine if we have this interface:
+     *     interface IFoo { void doFoo(); }
+     *
+     * If an unrelated owner (perhaps in a downstream codebase) wants to make a
+     * change to the interface, they have two options:
+     *
+     * A). Historical option that has proven to be BAD! Only the original
+     *     author of an interface should change an interface. If someone
+     *     downstream wants additional functionality, they should not ever
+     *     change the interface or use this method.
+     *
+     *    BAD TO DO:  interface IFoo {                       BAD TO DO
+     *    BAD TO DO:      void doFoo();                      BAD TO DO
+     *    BAD TO DO: +    void doBar(); // adding a method   BAD TO DO
+     *    BAD TO DO:  }                                      BAD TO DO
+     *
+     * B). Option that this method enables!
+     *     Leave the original interface unchanged (do not change IFoo!).
+     *     Instead, create a new interface in a downstream package:
+     *
+     *         package com.<name>; // new functionality in a new package
+     *         interface IBar { void doBar(); }
+     *
+     *     When registering the interface, add:
+     *         sp<MyFoo> foo = new MyFoo; // class in AOSP codebase
+     *         sp<MyBar> bar = new MyBar; // custom extension class
+     *         foo->setExtension(bar);    // use method in BBinder
+     *
+     *     Then, clients of IFoo can get this extension:
+     *         sp<IBinder> binder = ...;
+     *         sp<IFoo> foo = interface_cast<IFoo>(binder); // handle if null
+     *         sp<IBinder> barBinder;
+     *         ... handle error ... = binder->getExtension(&barBinder);
+     *         sp<IBar> bar = interface_cast<IBar>(barBinder);
+     *         // if bar is null, then there is no extension or a different
+     *         // type of extension
+     */
+    status_t                getExtension(sp<IBinder>* out);
+
     // NOLINTNEXTLINE(google-default-arguments)
     virtual status_t        transact(   uint32_t code,
                                         const Parcel& data,
diff --git a/libs/binder/tests/binderLibTest.cpp b/libs/binder/tests/binderLibTest.cpp
index 9de346054d..495b2f9d9a 100644
--- a/libs/binder/tests/binderLibTest.cpp
+++ b/libs/binder/tests/binderLibTest.cpp
@@ -769,6 +769,24 @@ TEST_F(BinderLibTest, PromoteLocal) {
     EXPECT_TRUE(strong_from_weak == nullptr);
 }
 
+TEST_F(BinderLibTest, LocalGetExtension) {
+    sp<BBinder> binder = new BBinder();
+    sp<IBinder> ext = new BBinder();
+    binder->setExtension(ext);
+    EXPECT_EQ(ext, binder->getExtension());
+}
+
+TEST_F(BinderLibTest, RemoteGetExtension) {
+    sp<IBinder> server = addServer();
+    ASSERT_TRUE(server != nullptr);
+
+    sp<IBinder> extension;
+    EXPECT_EQ(NO_ERROR, server->getExtension(&extension));
+    ASSERT_NE(nullptr, extension.get());
+
+    EXPECT_EQ(NO_ERROR, extension->pingBinder());
+}
+
 TEST_F(BinderLibTest, CheckHandleZeroBinderHighBitsZeroCookie) {
     status_t ret;
     Parcel data, reply;
@@ -1312,6 +1330,13 @@ int run_server(int index, int readypipefd, bool usePoll)
     BinderLibTestService* testServicePtr;
     {
         sp<BinderLibTestService> testService = new BinderLibTestService(index);
+
+        /*
+         * Normally would also contain functionality as well, but we are only
+         * testing the extension mechanism.
+         */
+        testService->setExtension(new BBinder());
+
         /*
          * We need this below, but can't hold a sp<> because it prevents the
          * node from being cleaned up automatically. It's safe in this case