Merge "SF: Enable address sanitizer for the unittest"
diff --git a/PREUPLOAD.cfg b/PREUPLOAD.cfg
new file mode 100644
index 0000000..f8f0265
--- /dev/null
+++ b/PREUPLOAD.cfg
@@ -0,0 +1,3 @@
+[Hook Scripts]
+owners_hook = ${REPO_ROOT}/frameworks/base/tools/aosp/aosp_sha.sh ${PREUPLOAD_COMMIT} "OWNERS$"
+installd_hook = ${REPO_ROOT}/frameworks/base/tools/aosp/aosp_sha.sh ${PREUPLOAD_COMMIT} "^cmds/installd/"
diff --git a/cmds/installd/OWNERS b/cmds/installd/OWNERS
new file mode 100644
index 0000000..50440f1
--- /dev/null
+++ b/cmds/installd/OWNERS
@@ -0,0 +1,7 @@
+set noparent
+
+calin@google.com
+agampe@google.com
+jsharkey@android.com
+toddke@google.com
+ngeoffray@google.com
diff --git a/cmds/servicemanager/service_manager.c b/cmds/servicemanager/service_manager.c
index 6b340a8..d776682 100644
--- a/cmds/servicemanager/service_manager.c
+++ b/cmds/servicemanager/service_manager.c
@@ -403,7 +403,11 @@
 
     cb.func_audit = audit_callback;
     selinux_set_callback(SELINUX_CB_AUDIT, cb);
+#ifdef VENDORSERVICEMANAGER
+    cb.func_log = selinux_vendor_log_callback;
+#else
     cb.func_log = selinux_log_callback;
+#endif
     selinux_set_callback(SELINUX_CB_LOG, cb);
 
 #ifdef VENDORSERVICEMANAGER
diff --git a/libs/binder/ndk/AIBinder.cpp b/libs/binder/ndk/AIBinder.cpp
index b7f5636..c02a77a 100644
--- a/libs/binder/ndk/AIBinder.cpp
+++ b/libs/binder/ndk/AIBinder.cpp
@@ -22,6 +22,8 @@
 
 #include <android-base/logging.h>
 
+using DeathRecipient = ::android::IBinder::DeathRecipient;
+
 using ::android::IBinder;
 using ::android::Parcel;
 using ::android::sp;
@@ -32,10 +34,10 @@
 
 static const void* kId = "ABBinder";
 static void* kValue = static_cast<void*>(new bool{true});
-void cleanId(const void* /*id*/, void* /*obj*/, void* /*cookie*/){/* do nothing */};
+void clean(const void* /*id*/, void* /*obj*/, void* /*cookie*/){/* do nothing */};
 
 static void attach(const sp<IBinder>& binder) {
-    binder->attachObject(kId, kValue, nullptr /*cookie*/, cleanId);
+    binder->attachObject(kId, kValue, nullptr /*cookie*/, clean);
 }
 static bool has(const sp<IBinder>& binder) {
     return binder != nullptr && binder->findObject(kId) == kValue;
@@ -43,6 +45,21 @@
 
 } // namespace ABBinderTag
 
+namespace ABpBinderTag {
+
+static std::mutex gLock;
+static const void* kId = "ABpBinder";
+struct Value {
+    wp<ABpBinder> binder;
+};
+void clean(const void* id, void* obj, void* cookie) {
+    CHECK(id == kId) << id << " " << obj << " " << cookie;
+
+    delete static_cast<Value*>(obj);
+};
+
+} // namespace ABpBinderTag
+
 AIBinder::AIBinder(const AIBinder_Class* clazz) : mClazz(clazz) {}
 AIBinder::~AIBinder() {}
 
@@ -121,14 +138,51 @@
 }
 ABpBinder::~ABpBinder() {}
 
-sp<AIBinder> ABpBinder::fromBinder(const ::android::sp<::android::IBinder>& binder) {
+void ABpBinder::onLastStrongRef(const void* id) {
+    {
+        std::lock_guard<std::mutex> lock(ABpBinderTag::gLock);
+        // Since ABpBinder is OBJECT_LIFETIME_WEAK, we must remove this weak reference in order for
+        // the ABpBinder to be deleted. Since a strong reference to this ABpBinder object should no
+        // longer be able to exist at the time of this method call, there is no longer a need to
+        // recover it.
+
+        ABpBinderTag::Value* value =
+                static_cast<ABpBinderTag::Value*>(remote()->findObject(ABpBinderTag::kId));
+        if (value != nullptr) {
+            value->binder = nullptr;
+        }
+    }
+
+    BpRefBase::onLastStrongRef(id);
+}
+
+sp<AIBinder> ABpBinder::lookupOrCreateFromBinder(const ::android::sp<::android::IBinder>& binder) {
     if (binder == nullptr) {
         return nullptr;
     }
     if (ABBinderTag::has(binder)) {
         return static_cast<ABBinder*>(binder.get());
     }
-    return new ABpBinder(binder);
+
+    // The following code ensures that for a given binder object (remote or local), if it is not an
+    // ABBinder then at most one ABpBinder object exists in a given process representing it.
+    std::lock_guard<std::mutex> lock(ABpBinderTag::gLock);
+
+    ABpBinderTag::Value* value =
+            static_cast<ABpBinderTag::Value*>(binder->findObject(ABpBinderTag::kId));
+    if (value == nullptr) {
+        value = new ABpBinderTag::Value;
+        binder->attachObject(ABpBinderTag::kId, static_cast<void*>(value), nullptr /*cookie*/,
+                             ABpBinderTag::clean);
+    }
+
+    sp<ABpBinder> ret = value->binder.promote();
+    if (ret == nullptr) {
+        ret = new ABpBinder(binder);
+        value->binder = ret;
+    }
+
+    return ret;
 }
 
 struct AIBinder_Weak {
@@ -179,6 +233,63 @@
     return new AIBinder_Class(interfaceDescriptor, onCreate, onDestroy, onTransact);
 }
 
+void AIBinder_DeathRecipient::TransferDeathRecipient::binderDied(const wp<IBinder>& who) {
+    CHECK(who == mWho);
+
+    mOnDied(mCookie);
+    mWho = nullptr;
+}
+
+AIBinder_DeathRecipient::AIBinder_DeathRecipient(AIBinder_DeathRecipient_onBinderDied onDied)
+      : mOnDied(onDied) {
+    CHECK(onDied != nullptr);
+}
+
+binder_status_t AIBinder_DeathRecipient::linkToDeath(AIBinder* binder, void* cookie) {
+    CHECK(binder != nullptr);
+
+    std::lock_guard<std::mutex> l(mDeathRecipientsMutex);
+
+    sp<TransferDeathRecipient> recipient =
+            new TransferDeathRecipient(binder->getBinder(), cookie, mOnDied);
+
+    binder_status_t status = binder->getBinder()->linkToDeath(recipient, cookie, 0 /*flags*/);
+    if (status != EX_NONE) {
+        return status;
+    }
+
+    mDeathRecipients.push_back(recipient);
+    return EX_NONE;
+}
+
+binder_status_t AIBinder_DeathRecipient::unlinkToDeath(AIBinder* binder, void* cookie) {
+    CHECK(binder != nullptr);
+
+    std::lock_guard<std::mutex> l(mDeathRecipientsMutex);
+
+    for (auto it = mDeathRecipients.rbegin(); it != mDeathRecipients.rend(); ++it) {
+        sp<TransferDeathRecipient> recipient = *it;
+
+        if (recipient->getCookie() == cookie &&
+
+            recipient->getWho() == binder->getBinder()) {
+            mDeathRecipients.erase(it.base() - 1);
+
+            binder_status_t status =
+                    binder->getBinder()->unlinkToDeath(recipient, cookie, 0 /*flags*/);
+            if (status != EX_NONE) {
+                LOG(ERROR) << __func__
+                           << ": removed reference to death recipient but unlink failed.";
+            }
+            return status;
+        }
+    }
+
+    return -ENOENT;
+}
+
+// start of C-API methods
+
 AIBinder* AIBinder_new(const AIBinder_Class* clazz, void* args) {
     if (clazz == nullptr) {
         LOG(ERROR) << __func__ << ": Must provide class to construct local binder.";
@@ -218,6 +329,26 @@
     return binder->getBinder()->pingBinder();
 }
 
+binder_status_t AIBinder_linkToDeath(AIBinder* binder, AIBinder_DeathRecipient* recipient,
+                                     void* cookie) {
+    if (binder == nullptr || recipient == nullptr) {
+        LOG(ERROR) << __func__ << ": Must provide binder and recipient.";
+        return EX_NULL_POINTER;
+    }
+
+    return recipient->linkToDeath(binder, cookie);
+}
+
+binder_status_t AIBinder_unlinkToDeath(AIBinder* binder, AIBinder_DeathRecipient* recipient,
+                                       void* cookie) {
+    if (binder == nullptr || recipient == nullptr) {
+        LOG(ERROR) << __func__ << ": Must provide binder and recipient.";
+        return EX_NULL_POINTER;
+    }
+
+    return recipient->unlinkToDeath(binder, cookie);
+}
+
 void AIBinder_incStrong(AIBinder* binder) {
     if (binder == nullptr) {
         LOG(ERROR) << __func__ << ": on null binder";
@@ -286,7 +417,10 @@
     }
 
     if (!binder->isRemote()) {
-        LOG(WARNING) << "A binder object at " << binder << " is being transacted on, however, this object is in the same process as its proxy. Transacting with this binder is expensive compared to just calling the corresponding functionality in the same process.";
+        LOG(WARNING) << "A binder object at " << binder
+                     << " is being transacted on, however, this object is in the same process as "
+                        "its proxy. Transacting with this binder is expensive compared to just "
+                        "calling the corresponding functionality in the same process.";
     }
 
     *in = new AParcel(binder);
@@ -344,3 +478,21 @@
 
     return parcelStatus;
 }
+
+AIBinder_DeathRecipient* AIBinder_DeathRecipient_new(
+        AIBinder_DeathRecipient_onBinderDied onBinderDied) {
+    if (onBinderDied == nullptr) {
+        LOG(ERROR) << __func__ << ": requires non-null onBinderDied parameter.";
+        return nullptr;
+    }
+    return new AIBinder_DeathRecipient(onBinderDied);
+}
+
+void AIBinder_DeathRecipient_delete(AIBinder_DeathRecipient** recipient) {
+    if (recipient == nullptr) {
+        return;
+    }
+
+    delete *recipient;
+    *recipient = nullptr;
+}
diff --git a/libs/binder/ndk/AIBinder_internal.h b/libs/binder/ndk/AIBinder_internal.h
index 23949bb..30009d2 100644
--- a/libs/binder/ndk/AIBinder_internal.h
+++ b/libs/binder/ndk/AIBinder_internal.h
@@ -20,6 +20,8 @@
 #include "AIBinder_internal.h"
 
 #include <atomic>
+#include <mutex>
+#include <vector>
 
 #include <binder/Binder.h>
 #include <binder/IBinder.h>
@@ -79,13 +81,18 @@
     void* mUserData;
 };
 
-// This binder object may be remote or local (even though it is 'Bp'). It is not yet associated with
-// a class.
+// This binder object may be remote or local (even though it is 'Bp'). The implication if it is
+// local is that it is an IBinder object created outside of the domain of libbinder_ndk.
 struct ABpBinder : public AIBinder, public ::android::BpRefBase {
-    static ::android::sp<AIBinder> fromBinder(const ::android::sp<::android::IBinder>& binder);
+    // Looks up to see if this object has or is an existing ABBinder or ABpBinder object, otherwise
+    // it creates an ABpBinder object.
+    static ::android::sp<AIBinder> lookupOrCreateFromBinder(
+            const ::android::sp<::android::IBinder>& binder);
 
     virtual ~ABpBinder();
 
+    void onLastStrongRef(const void* id) override;
+
     ::android::sp<::android::IBinder> getBinder() override { return remote(); }
     ABpBinder* asABpBinder() override { return this; }
 
@@ -108,3 +115,38 @@
     // one.
     const ::android::String16 mInterfaceDescriptor;
 };
+
+// Ownership is like this (when linked to death):
+//
+//   AIBinder_DeathRecipient -sp-> TransferDeathRecipient <-wp-> IBinder
+//
+// When the AIBinder_DeathRecipient is dropped, so are the actual underlying death recipients. When
+// the IBinder dies, only a wp to it is kept.
+struct AIBinder_DeathRecipient {
+    // One of these is created for every linkToDeath. This is to be able to recover data when a
+    // binderDied receipt only gives us information about the IBinder.
+    struct TransferDeathRecipient : ::android::IBinder::DeathRecipient {
+        TransferDeathRecipient(const ::android::wp<::android::IBinder>& who, void* cookie,
+                               const AIBinder_DeathRecipient_onBinderDied& onDied)
+              : mWho(who), mCookie(cookie), mOnDied(onDied) {}
+
+        void binderDied(const ::android::wp<::android::IBinder>& who) override;
+
+        const ::android::wp<::android::IBinder>& getWho() { return mWho; }
+        void* getCookie() { return mCookie; }
+
+    private:
+        ::android::wp<::android::IBinder> mWho;
+        void* mCookie;
+        const AIBinder_DeathRecipient_onBinderDied& mOnDied;
+    };
+
+    AIBinder_DeathRecipient(AIBinder_DeathRecipient_onBinderDied onDied);
+    binder_status_t linkToDeath(AIBinder* binder, void* cookie);
+    binder_status_t unlinkToDeath(AIBinder* binder, void* cookie);
+
+private:
+    std::mutex mDeathRecipientsMutex;
+    std::vector<::android::sp<TransferDeathRecipient>> mDeathRecipients;
+    AIBinder_DeathRecipient_onBinderDied mOnDied;
+};
diff --git a/libs/binder/ndk/AParcel.cpp b/libs/binder/ndk/AParcel.cpp
index 93384d6..f090929 100644
--- a/libs/binder/ndk/AParcel.cpp
+++ b/libs/binder/ndk/AParcel.cpp
@@ -35,7 +35,8 @@
 }
 
 binder_status_t AParcel_writeStrongBinder(AParcel* parcel, AIBinder* binder) {
-    return (*parcel)->writeStrongBinder(binder->getBinder());
+    sp<IBinder> writeBinder = binder != nullptr ? binder->getBinder() : nullptr;
+    return (*parcel)->writeStrongBinder(writeBinder);
 }
 binder_status_t AParcel_readStrongBinder(const AParcel* parcel, AIBinder** binder) {
     sp<IBinder> readBinder = nullptr;
@@ -43,7 +44,7 @@
     if (status != EX_NONE) {
         return status;
     }
-    sp<AIBinder> ret = ABpBinder::fromBinder(readBinder);
+    sp<AIBinder> ret = ABpBinder::lookupOrCreateFromBinder(readBinder);
     AIBinder_incStrong(ret.get());
     *binder = ret.get();
     return status;
@@ -54,7 +55,7 @@
     if (status != EX_NONE) {
         return status;
     }
-    sp<AIBinder> ret = ABpBinder::fromBinder(readBinder);
+    sp<AIBinder> ret = ABpBinder::lookupOrCreateFromBinder(readBinder);
     AIBinder_incStrong(ret.get());
     *binder = ret.get();
     return status;
diff --git a/libs/binder/ndk/AServiceManager.cpp b/libs/binder/ndk/AServiceManager.cpp
index 3979945..90dd1c8 100644
--- a/libs/binder/ndk/AServiceManager.cpp
+++ b/libs/binder/ndk/AServiceManager.cpp
@@ -41,7 +41,7 @@
     sp<IServiceManager> sm = defaultServiceManager();
     sp<IBinder> binder = sm->getService(String16(instance));
 
-    sp<AIBinder> ret = ABpBinder::fromBinder(binder);
+    sp<AIBinder> ret = ABpBinder::lookupOrCreateFromBinder(binder);
     AIBinder_incStrong(ret.get());
     return ret.get();
 }
diff --git a/libs/binder/ndk/include_ndk/android/binder_ibinder.h b/libs/binder/ndk/include_ndk/android/binder_ibinder.h
index aa29437..7dca5a4 100644
--- a/libs/binder/ndk/include_ndk/android/binder_ibinder.h
+++ b/libs/binder/ndk/include_ndk/android/binder_ibinder.h
@@ -14,6 +14,16 @@
  * limitations under the License.
  */
 
+/**
+ * @addtogroup NdkBinder
+ * @{
+ */
+
+/**
+ * @file binder_ibinder.h
+ * @brief Object which can receive transactions and be sent across processes.
+ */
+
 #pragma once
 
 #include <stdint.h>
@@ -84,6 +94,16 @@
  * implementation (usually a callback) to transfer all ownership to a remote process and
  * automatically be deleted when the remote process is done with it or dies. Other memory models are
  * possible, but this is the standard one.
+ *
+ * If the process containing an AIBinder dies, it is possible to be holding a strong reference to
+ * an object which does not exist. In this case, transactions to this binder will return
+ * EX_DEAD_OBJECT. See also AIBinder_linkToDeath, AIBinder_unlinkToDeath, and AIBinder_isAlive.
+ *
+ * Once an AIBinder is created, anywhere it is passed (remotely or locally), there is a 1-1
+ * correspondence between the address of an AIBinder and the object it represents. This means that
+ * when two AIBinder pointers point to the same address, they represent the same object (whether
+ * that object is local or remote). This correspondance can be broken accidentally if AIBinder_new
+ * is erronesouly called to create the same object multiple times.
  */
 struct AIBinder;
 typedef struct AIBinder AIBinder;
@@ -97,6 +117,12 @@
 typedef struct AIBinder_Weak AIBinder_Weak;
 
 /**
+ * Represents a handle on a death notification. See AIBinder_linkToDeath/AIBinder_unlinkToDeath.
+ */
+struct AIBinder_DeathRecipient;
+typedef struct AIBinder_DeathRecipient AIBinder_DeathRecipient;
+
+/**
  * This is called whenever a new AIBinder object is needed of a specific class.
  *
  * These arguments are passed from AIBinder_new. The return value is stored and can be retrieved
@@ -138,6 +164,14 @@
  *
  * When this is called, the refcount is implicitly 1. So, calling decStrong exactly one time is
  * required to delete this object.
+ *
+ * Once an AIBinder object is created using this API, re-creating that AIBinder for the same
+ * instance of the same class will break pointer equality for that specific AIBinder object. For
+ * instance, if someone erroneously created two AIBinder instances representing the same callback
+ * object and passed one to a hypothetical addCallback function and then later another one to a
+ * hypothetical removeCallback function, the remote process would have no way to determine that
+ * these two objects are actually equal using the AIBinder pointer alone (which they should be able
+ * to do). Also see the suggested memory ownership model suggested above.
  */
 __attribute__((warn_unused_result)) AIBinder* AIBinder_new(const AIBinder_Class* clazz, void* args);
 
@@ -163,6 +197,26 @@
 binder_status_t AIBinder_ping(AIBinder* binder);
 
 /**
+ * Registers for notifications that the associated binder is dead. The same death recipient may be
+ * associated with multiple different binders. If the binder is local, then no death recipient will
+ * be given (since if the local process dies, then no recipient will exist to recieve a
+ * transaction). The cookie is passed to recipient in the case that this binder dies and can be
+ * null. The exact cookie must also be used to unlink this transaction (see AIBinder_linkToDeath).
+ * This function may return a binder transaction failure. The cookie can be used both for
+ * identification and holding user data.
+ */
+binder_status_t AIBinder_linkToDeath(AIBinder* binder, AIBinder_DeathRecipient* recipient,
+                                     void* cookie);
+
+/**
+ * Stops registration for the associated binder dying. Does not delete the recipient. This function
+ * may return a binder transaction failure and in case the death recipient cannot be found, it
+ * returns -ENOENT.
+ */
+binder_status_t AIBinder_unlinkToDeath(AIBinder* binder, AIBinder_DeathRecipient* recipient,
+                                       void* cookie);
+
+/**
  * This can only be called if a strong reference to this object already exists in process.
  */
 void AIBinder_incStrong(AIBinder* binder);
@@ -254,4 +308,23 @@
  */
 __attribute__((warn_unused_result)) AIBinder* AIBinder_Weak_promote(AIBinder_Weak* weakBinder);
 
+/**
+ * This function is executed on death receipt. See AIBinder_linkToDeath/AIBinder_unlinkToDeath.
+ */
+typedef void (*AIBinder_DeathRecipient_onBinderDied)(void* cookie);
+
+/**
+ * Creates a new binder death recipient. This can be attached to multiple different binder objects.
+ */
+__attribute__((warn_unused_result)) AIBinder_DeathRecipient* AIBinder_DeathRecipient_new(
+        AIBinder_DeathRecipient_onBinderDied onBinderDied);
+
+/**
+ * Deletes a binder death recipient. It is not necessary to call AIBinder_unlinkToDeath before
+ * calling this as these will all be automatically unlinked.
+ */
+void AIBinder_DeathRecipient_delete(AIBinder_DeathRecipient** recipient);
+
 __END_DECLS
+
+/** @} */
diff --git a/libs/binder/ndk/include_ndk/android/binder_parcel.h b/libs/binder/ndk/include_ndk/android/binder_parcel.h
index 19925f7..e871ed1 100644
--- a/libs/binder/ndk/include_ndk/android/binder_parcel.h
+++ b/libs/binder/ndk/include_ndk/android/binder_parcel.h
@@ -14,6 +14,16 @@
  * limitations under the License.
  */
 
+/**
+ * @addtogroup NdkBinder
+ * @{
+ */
+
+/**
+ * @file binder_parcel.h
+ * @brief A collection of data that can be sent as a single packet.
+ */
+
 #pragma once
 
 #include <sys/cdefs.h>
@@ -150,3 +160,5 @@
 // @END
 
 __END_DECLS
+
+/** @} */
diff --git a/libs/binder/ndk/include_ndk/android/binder_status.h b/libs/binder/ndk/include_ndk/android/binder_status.h
index d414c99..e97e181 100644
--- a/libs/binder/ndk/include_ndk/android/binder_status.h
+++ b/libs/binder/ndk/include_ndk/android/binder_status.h
@@ -14,6 +14,15 @@
  * limitations under the License.
  */
 
+/**
+ * @addtogroup NdkBinder
+ * @{
+ */
+
+/**
+ * @file binder_status.h
+ */
+
 #pragma once
 
 #include <stdint.h>
@@ -54,3 +63,5 @@
 typedef int32_t binder_status_t;
 
 __END_DECLS
+
+/** @} */
diff --git a/libs/binder/ndk/test/main_client.cpp b/libs/binder/ndk/test/main_client.cpp
index 967789f..b8518d7 100644
--- a/libs/binder/ndk/test/main_client.cpp
+++ b/libs/binder/ndk/test/main_client.cpp
@@ -16,6 +16,7 @@
 
 #include <android-base/logging.h>
 #include <android/binder_manager.h>
+#include <android/binder_process.h>
 #include <gtest/gtest.h>
 #include <iface/iface.h>
 
@@ -45,6 +46,28 @@
     AIBinder_decStrong(binder);
 }
 
+void OnBinderDeath(void* cookie) {
+    LOG(ERROR) << "BINDER DIED. COOKIE: " << cookie;
+}
+
+TEST(NdkBinder, LinkToDeath) {
+    ABinderProcess_setThreadPoolMaxThreadCount(1); // to recieve death notifications
+    ABinderProcess_startThreadPool();
+
+    AIBinder* binder = AServiceManager_getService(kExistingNonNdkService);
+    ASSERT_NE(nullptr, binder);
+
+    AIBinder_DeathRecipient* recipient = AIBinder_DeathRecipient_new(OnBinderDeath);
+    ASSERT_NE(nullptr, recipient);
+
+    EXPECT_EQ(EX_NONE, AIBinder_linkToDeath(binder, recipient, nullptr));
+    EXPECT_EQ(EX_NONE, AIBinder_unlinkToDeath(binder, recipient, nullptr));
+    EXPECT_EQ(-ENOENT, AIBinder_unlinkToDeath(binder, recipient, nullptr));
+
+    AIBinder_DeathRecipient_delete(&recipient);
+    AIBinder_decStrong(binder);
+}
+
 class MyTestFoo : public IFoo {
     int32_t doubleNumber(int32_t in) override {
         LOG(INFO) << "doubleNumber " << in;
@@ -64,6 +87,34 @@
     EXPECT_EQ(2, getFoo->doubleNumber(1));
 }
 
+TEST(NdkBinder, EqualityOfRemoteBinderPointer) {
+    AIBinder* binderA = AServiceManager_getService(kExistingNonNdkService);
+    ASSERT_NE(nullptr, binderA);
+
+    AIBinder* binderB = AServiceManager_getService(kExistingNonNdkService);
+    ASSERT_NE(nullptr, binderB);
+
+    EXPECT_EQ(binderA, binderB);
+
+    AIBinder_decStrong(binderA);
+    AIBinder_decStrong(binderB);
+}
+
+TEST(NdkBinder, ABpBinderRefCount) {
+    AIBinder* binder = AServiceManager_getService(kExistingNonNdkService);
+    AIBinder_Weak* wBinder = AIBinder_Weak_new(binder);
+
+    ASSERT_NE(nullptr, binder);
+    EXPECT_EQ(1, AIBinder_debugGetRefCount(binder));
+
+    AIBinder_decStrong(binder);
+
+    // assert because would need to decStrong if non-null and we shouldn't need to add a no-op here
+    ASSERT_NE(nullptr, AIBinder_Weak_promote(wBinder));
+
+    AIBinder_Weak_delete(&wBinder);
+}
+
 TEST(NdkBinder, AddServiceMultipleTimes) {
     static const char* kInstanceName1 = "test-multi-1";
     static const char* kInstanceName2 = "test-multi-2";
diff --git a/libs/gui/LayerState.cpp b/libs/gui/LayerState.cpp
index deb8ea8..5a8d8db 100644
--- a/libs/gui/LayerState.cpp
+++ b/libs/gui/LayerState.cpp
@@ -14,6 +14,8 @@
  * limitations under the License.
  */
 
+#define LOG_TAG "LayerState"
+
 #include <utils/Errors.h>
 #include <binder/Parcel.h>
 #include <gui/ISurfaceComposerClient.h>
@@ -313,8 +315,10 @@
         sidebandStream = other.sidebandStream;
     }
 
-    if (other.what != what) {
-        ALOGE("Unmerged SurfaceComposer Transaction properties. LayerState::merge needs updating?");
+    if ((other.what & what) != other.what) {
+        ALOGE("Unmerged SurfaceComposer Transaction properties. LayerState::merge needs updating? "
+              "other.what=0x%X what=0x%X",
+              other.what, what);
     }
 }
 
diff --git a/libs/vr/libbufferhubqueue/buffer_hub_queue_client.cpp b/libs/vr/libbufferhubqueue/buffer_hub_queue_client.cpp
index 1f2c517..a296d20 100644
--- a/libs/vr/libbufferhubqueue/buffer_hub_queue_client.cpp
+++ b/libs/vr/libbufferhubqueue/buffer_hub_queue_client.cpp
@@ -440,6 +440,9 @@
 Status<std::vector<size_t>> ProducerQueue::AllocateBuffers(
     uint32_t width, uint32_t height, uint32_t layer_count, uint32_t format,
     uint64_t usage, size_t buffer_count) {
+  if (buffer_count == 0) {
+    return {};
+  }
   if (capacity() + buffer_count > kMaxQueueCapacity) {
     ALOGE(
         "ProducerQueue::AllocateBuffers: queue is at capacity: %zu, cannot "
@@ -481,10 +484,13 @@
     }
   }
 
-  if (buffer_slots.size() == 0) {
-    // Error out if no buffer is allocated and improted.
-    ALOGE_IF(TRACE, "ProducerQueue::AllocateBuffers: no buffer allocated.");
-    ErrorStatus(ENOMEM);
+  if (buffer_slots.size() != buffer_count) {
+    // Error out if the count of allocated/imported buffer(s) is not correct.
+    ALOGE(
+        "ProducerQueue::AllocateBuffers: requested to import %zu "
+        "buffers, but actually imported %zu buffers.",
+        buffer_count, buffer_slots.size());
+    return ErrorStatus(ENOMEM);
   }
 
   return {std::move(buffer_slots)};
@@ -502,12 +508,6 @@
           status.GetErrorMessage().c_str());
     return status.error_status();
   }
-
-  if (status.get().size() == 0) {
-    ALOGE_IF(TRACE, "ProducerQueue::AllocateBuffer: no buffer allocated.");
-    ErrorStatus(ENOMEM);
-  }
-
   return {status.get()[0]};
 }
 
diff --git a/libs/vr/libbufferhubqueue/tests/buffer_hub_queue-test.cpp b/libs/vr/libbufferhubqueue/tests/buffer_hub_queue-test.cpp
index 2975f56..c1f322c 100644
--- a/libs/vr/libbufferhubqueue/tests/buffer_hub_queue-test.cpp
+++ b/libs/vr/libbufferhubqueue/tests/buffer_hub_queue-test.cpp
@@ -562,6 +562,28 @@
   ASSERT_EQ(cs2, ps2);
 }
 
+TEST_F(BufferHubQueueTest, TestAllocateTwoBuffers) {
+  ASSERT_TRUE(CreateQueues(config_builder_.Build(), UsagePolicy{}));
+  ASSERT_EQ(producer_queue_->capacity(), 0);
+
+  auto status = producer_queue_->AllocateBuffers(
+      kBufferWidth, kBufferHeight, kBufferLayerCount, kBufferFormat,
+      kBufferUsage, 2);
+  ASSERT_TRUE(status.ok());
+  ASSERT_EQ(producer_queue_->capacity(), 2);
+}
+
+TEST_F(BufferHubQueueTest, TestAllocateZeroBuffers) {
+  ASSERT_TRUE(CreateQueues(config_builder_.Build(), UsagePolicy{}));
+  ASSERT_EQ(producer_queue_->capacity(), 0);
+
+  auto status = producer_queue_->AllocateBuffers(
+      kBufferWidth, kBufferHeight, kBufferLayerCount, kBufferFormat,
+      kBufferUsage, 0);
+  ASSERT_TRUE(status.ok());
+  ASSERT_EQ(producer_queue_->capacity(), 0);
+}
+
 TEST_F(BufferHubQueueTest, TestUsageSetMask) {
   const uint32_t set_mask = GRALLOC_USAGE_SW_WRITE_OFTEN;
   ASSERT_TRUE(
diff --git a/services/surfaceflinger/BufferQueueLayer.cpp b/services/surfaceflinger/BufferQueueLayer.cpp
index 10974b4..4839bfb 100644
--- a/services/surfaceflinger/BufferQueueLayer.cpp
+++ b/services/surfaceflinger/BufferQueueLayer.cpp
@@ -433,6 +433,10 @@
     if (mFlinger->isLayerTripleBufferingDisabled()) {
         mProducer->setMaxDequeuedBufferCount(2);
     }
+
+    if (const auto display = mFlinger->getDefaultDisplayDevice()) {
+        updateTransformHint(display);
+    }
 }
 
 status_t BufferQueueLayer::setDefaultBufferProperties(uint32_t w, uint32_t h, PixelFormat format) {
diff --git a/services/surfaceflinger/RenderEngine/gl/GLES20RenderEngine.cpp b/services/surfaceflinger/RenderEngine/gl/GLES20RenderEngine.cpp
index 7121cc2..0f0ff62 100644
--- a/services/surfaceflinger/RenderEngine/gl/GLES20RenderEngine.cpp
+++ b/services/surfaceflinger/RenderEngine/gl/GLES20RenderEngine.cpp
@@ -539,25 +539,25 @@
     glClear(GL_COLOR_BUFFER_BIT);
 }
 
-void GLES20RenderEngine::fillRegionWithColor(const Region& region, uint32_t height, float red,
-                                             float green, float blue, float alpha) {
+void GLES20RenderEngine::fillRegionWithColor(const Region& region, float red, float green,
+                                             float blue, float alpha) {
     size_t c;
     Rect const* r = region.getArray(&c);
     Mesh mesh(Mesh::TRIANGLES, c * 6, 2);
     Mesh::VertexArray<vec2> position(mesh.getPositionArray<vec2>());
     for (size_t i = 0; i < c; i++, r++) {
         position[i * 6 + 0].x = r->left;
-        position[i * 6 + 0].y = height - r->top;
+        position[i * 6 + 0].y = r->top;
         position[i * 6 + 1].x = r->left;
-        position[i * 6 + 1].y = height - r->bottom;
+        position[i * 6 + 1].y = r->bottom;
         position[i * 6 + 2].x = r->right;
-        position[i * 6 + 2].y = height - r->bottom;
+        position[i * 6 + 2].y = r->bottom;
         position[i * 6 + 3].x = r->left;
-        position[i * 6 + 3].y = height - r->top;
+        position[i * 6 + 3].y = r->top;
         position[i * 6 + 4].x = r->right;
-        position[i * 6 + 4].y = height - r->bottom;
+        position[i * 6 + 4].y = r->bottom;
         position[i * 6 + 5].x = r->right;
-        position[i * 6 + 5].y = height - r->top;
+        position[i * 6 + 5].y = r->top;
     }
     setupFillWithColor(red, green, blue, alpha);
     drawMesh(mesh);
diff --git a/services/surfaceflinger/RenderEngine/gl/GLES20RenderEngine.h b/services/surfaceflinger/RenderEngine/gl/GLES20RenderEngine.h
index 2502cb8..1abc5ba 100644
--- a/services/surfaceflinger/RenderEngine/gl/GLES20RenderEngine.h
+++ b/services/surfaceflinger/RenderEngine/gl/GLES20RenderEngine.h
@@ -62,8 +62,8 @@
     bool finish() override;
     bool waitFence(base::unique_fd fenceFd) override;
     void clearWithColor(float red, float green, float blue, float alpha) override;
-    void fillRegionWithColor(const Region& region, uint32_t height, float red, float green,
-                             float blue, float alpha) override;
+    void fillRegionWithColor(const Region& region, float red, float green, float blue,
+                             float alpha) override;
     void setScissor(uint32_t left, uint32_t bottom, uint32_t right, uint32_t top) override;
     void disableScissor() override;
     void genTextures(size_t count, uint32_t* names) override;
diff --git a/services/surfaceflinger/RenderEngine/include/renderengine/RenderEngine.h b/services/surfaceflinger/RenderEngine/include/renderengine/RenderEngine.h
index d0ca0d9..17d8782 100644
--- a/services/surfaceflinger/RenderEngine/include/renderengine/RenderEngine.h
+++ b/services/surfaceflinger/RenderEngine/include/renderengine/RenderEngine.h
@@ -93,7 +93,7 @@
     virtual bool waitFence(base::unique_fd fenceFd) = 0;
 
     virtual void clearWithColor(float red, float green, float blue, float alpha) = 0;
-    virtual void fillRegionWithColor(const Region& region, uint32_t height, float red, float green,
+    virtual void fillRegionWithColor(const Region& region, float red, float green,
                                      float blue, float alpha) = 0;
 
     virtual void setScissor(uint32_t left, uint32_t bottom, uint32_t right, uint32_t top) = 0;
diff --git a/services/surfaceflinger/Scheduler/Scheduler.cpp b/services/surfaceflinger/Scheduler/Scheduler.cpp
index bb85f4c..54e19c7 100644
--- a/services/surfaceflinger/Scheduler/Scheduler.cpp
+++ b/services/surfaceflinger/Scheduler/Scheduler.cpp
@@ -39,7 +39,7 @@
 Scheduler::~Scheduler() = default;
 
 sp<Scheduler::ConnectionHandle> Scheduler::createConnection(
-        const char* connectionName, DispSync* dispSync, int64_t phaseOffsetNs,
+        const std::string& connectionName, DispSync* dispSync, int64_t phaseOffsetNs,
         impl::EventThread::ResyncWithRateLimitCallback resyncCallback,
         impl::EventThread::InterceptVSyncsCallback interceptCallback) {
     const int64_t id = sNextId++;
@@ -56,13 +56,15 @@
 }
 
 std::unique_ptr<EventThread> Scheduler::makeEventThread(
-        const char* connectionName, DispSync* dispSync, int64_t phaseOffsetNs,
+        const std::string& connectionName, DispSync* dispSync, int64_t phaseOffsetNs,
         impl::EventThread::ResyncWithRateLimitCallback resyncCallback,
         impl::EventThread::InterceptVSyncsCallback interceptCallback) {
+    const std::string sourceName = connectionName + "Source";
     std::unique_ptr<VSyncSource> eventThreadSource =
-            std::make_unique<DispSyncSource>(dispSync, phaseOffsetNs, true, connectionName);
+            std::make_unique<DispSyncSource>(dispSync, phaseOffsetNs, true, sourceName.c_str());
+    const std::string threadName = connectionName + "Thread";
     return std::make_unique<impl::EventThread>(std::move(eventThreadSource), resyncCallback,
-                                               interceptCallback, connectionName);
+                                               interceptCallback, threadName.c_str());
 }
 
 sp<IDisplayEventConnection> Scheduler::createDisplayEventConnection(
diff --git a/services/surfaceflinger/Scheduler/Scheduler.h b/services/surfaceflinger/Scheduler/Scheduler.h
index 8efbff4..fcb46e6 100644
--- a/services/surfaceflinger/Scheduler/Scheduler.h
+++ b/services/surfaceflinger/Scheduler/Scheduler.h
@@ -56,7 +56,7 @@
 
     /** Creates an EventThread connection. */
     sp<ConnectionHandle> createConnection(
-            const char* connectionName, DispSync* dispSync, int64_t phaseOffsetNs,
+            const std::string& connectionName, DispSync* dispSync, int64_t phaseOffsetNs,
             impl::EventThread::ResyncWithRateLimitCallback resyncCallback,
             impl::EventThread::InterceptVSyncsCallback interceptCallback);
     sp<IDisplayEventConnection> createDisplayEventConnection(const sp<ConnectionHandle>& handle);
@@ -79,7 +79,7 @@
 
 protected:
     virtual std::unique_ptr<EventThread> makeEventThread(
-            const char* connectionName, DispSync* dispSync, int64_t phaseOffsetNs,
+            const std::string& connectionName, DispSync* dispSync, int64_t phaseOffsetNs,
             impl::EventThread::ResyncWithRateLimitCallback resyncCallback,
             impl::EventThread::InterceptVSyncsCallback interceptCallback);
 
diff --git a/services/surfaceflinger/Scheduler/VSyncModulator.h b/services/surfaceflinger/Scheduler/VSyncModulator.h
index 7dfad43..ea8ca4c 100644
--- a/services/surfaceflinger/Scheduler/VSyncModulator.h
+++ b/services/surfaceflinger/Scheduler/VSyncModulator.h
@@ -67,6 +67,14 @@
         mAppEventThread = appEventThread;
     }
 
+    void setSchedulerAndHandles(Scheduler* scheduler,
+                                Scheduler::ConnectionHandle* appConnectionHandle,
+                                Scheduler::ConnectionHandle* sfConnectionHandle) {
+        mScheduler = scheduler;
+        mAppConnectionHandle = appConnectionHandle;
+        mSfConnectionHandle = sfConnectionHandle;
+    }
+
     void setTransactionStart(TransactionStart transactionStart) {
         if (transactionStart == TransactionStart::EARLY) {
             mRemainingEarlyFrameCount = MIN_EARLY_FRAME_COUNT;
@@ -108,11 +116,19 @@
 
         bool changed = false;
         if (desired.sf != current.sf) {
-            mSfEventThread->setPhaseOffset(desired.sf);
+            if (mSfConnectionHandle != nullptr) {
+                mScheduler->setPhaseOffset(mSfConnectionHandle, desired.sf);
+            } else {
+                mSfEventThread->setPhaseOffset(desired.sf);
+            }
             changed = true;
         }
         if (desired.app != current.app) {
-            mAppEventThread->setPhaseOffset(desired.app);
+            if (mSfConnectionHandle != nullptr) {
+                mScheduler->setPhaseOffset(mAppConnectionHandle, desired.app);
+            } else {
+                mAppEventThread->setPhaseOffset(desired.app);
+            }
             changed = true;
         }
 
@@ -138,6 +154,10 @@
     EventThread* mSfEventThread = nullptr;
     EventThread* mAppEventThread = nullptr;
 
+    Scheduler* mScheduler = nullptr;
+    Scheduler::ConnectionHandle* mAppConnectionHandle = nullptr;
+    Scheduler::ConnectionHandle* mSfConnectionHandle = nullptr;
+
     std::atomic<Offsets> mOffsets;
 
     std::atomic<TransactionStart> mTransactionStart = TransactionStart::NORMAL;
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 55ef909..9537ccc 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -632,8 +632,8 @@
                                              });
 
         mEventQueue->setEventConnection(mScheduler->getEventConnection(mSfConnectionHandle));
-        mVsyncModulator.setEventThreads(mScheduler->getEventThread(mSfConnectionHandle),
-                                        mScheduler->getEventThread(mAppConnectionHandle));
+        mVsyncModulator.setSchedulerAndHandles(mScheduler.get(), mAppConnectionHandle.get(),
+                                               mSfConnectionHandle.get());
     } else {
         mEventThreadSource =
                 std::make_unique<DispSyncSource>(mPrimaryDispSync.get(),
@@ -1688,9 +1688,8 @@
             doComposeSurfaces(display);
 
             // and draw the dirty region
-            const int32_t height = display->getHeight();
             auto& engine(getRenderEngine());
-            engine.fillRegionWithColor(dirtyRegion, height, 1, 0, 1, 1);
+            engine.fillRegionWithColor(dirtyRegion, 1, 0, 1, 1);
 
             display->swapBuffers(getHwComposer());
         }
@@ -3194,7 +3193,7 @@
             // screen is already cleared here
             if (!region.isEmpty()) {
                 // can happen with SurfaceView
-                drawWormhole(display, region);
+                drawWormhole(region);
             }
         }
 
@@ -3231,9 +3230,10 @@
                 case HWC2::Composition::Sideband:
                 case HWC2::Composition::SolidColor: {
                     const Layer::State& state(compositionInfo.layer->mLayer->getDrawingState());
-                    const bool opaque = compositionInfo.layer->mLayer->isOpaque(state);
-                    if (compositionInfo.hwc.clearClientTarget && !firstLayer &&
-                            opaque && (state.color.a == 1.0f) && hasClientComposition) {
+                    const bool opaque = compositionInfo.layer->mLayer->isOpaque(state) &&
+                                        compositionInfo.layer->mLayer->getAlpha() == 1.0f;
+                    if (compositionInfo.hwc.clearClientTarget && !firstLayer && opaque &&
+                            hasClientComposition) {
                         // never clear the very first layer since we're
                         // guaranteed the FB is already cleared
                         compositionInfo.layer->clear(getRenderEngine());
@@ -3262,11 +3262,9 @@
     return true;
 }
 
-void SurfaceFlinger::drawWormhole(const sp<const DisplayDevice>& display,
-                                  const Region& region) const {
-    const int32_t height = display->getHeight();
+void SurfaceFlinger::drawWormhole(const Region& region) const {
     auto& engine(getRenderEngine());
-    engine.fillRegionWithColor(region, height, 0, 0, 0, 0);
+    engine.fillRegionWithColor(region, 0, 0, 0, 0);
 }
 
 status_t SurfaceFlinger::addClientLayer(const sp<Client>& client,
diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h
index 72ede93..00ee2c4 100644
--- a/services/surfaceflinger/SurfaceFlinger.h
+++ b/services/surfaceflinger/SurfaceFlinger.h
@@ -710,7 +710,7 @@
 
     void postFramebuffer(const sp<DisplayDevice>& display);
     void postFrame();
-    void drawWormhole(const sp<const DisplayDevice>& display, const Region& region) const;
+    void drawWormhole(const Region& region) const;
 
     /* ------------------------------------------------------------------------
      * Display management
diff --git a/services/surfaceflinger/tests/unittests/SchedulerTest.cpp b/services/surfaceflinger/tests/unittests/SchedulerTest.cpp
index c809e82..df0d982 100644
--- a/services/surfaceflinger/tests/unittests/SchedulerTest.cpp
+++ b/services/surfaceflinger/tests/unittests/SchedulerTest.cpp
@@ -40,7 +40,7 @@
               : mEventThread(std::move(eventThread)) {}
 
         std::unique_ptr<EventThread> makeEventThread(
-                const char* /* connectionName */, DispSync* /* dispSync */,
+                const std::string& /* connectionName */, DispSync* /* dispSync */,
                 nsecs_t /* phaseOffsetNs */,
                 impl::EventThread::ResyncWithRateLimitCallback /* resyncCallback */,
                 impl::EventThread::InterceptVSyncsCallback /* interceptCallback */) override {
diff --git a/services/surfaceflinger/tests/unittests/mock/RenderEngine/MockRenderEngine.h b/services/surfaceflinger/tests/unittests/mock/RenderEngine/MockRenderEngine.h
index 1c5135b..84d3c63 100644
--- a/services/surfaceflinger/tests/unittests/mock/RenderEngine/MockRenderEngine.h
+++ b/services/surfaceflinger/tests/unittests/mock/RenderEngine/MockRenderEngine.h
@@ -48,7 +48,7 @@
     MOCK_METHOD1(waitFence, bool(base::unique_fd*));
     bool waitFence(base::unique_fd fd) override { return waitFence(&fd); };
     MOCK_METHOD4(clearWithColor, void(float, float, float, float));
-    MOCK_METHOD6(fillRegionWithColor, void(const Region&, uint32_t, float, float, float, float));
+    MOCK_METHOD5(fillRegionWithColor, void(const Region&, float, float, float, float));
     MOCK_METHOD4(setScissor, void(uint32_t, uint32_t, uint32_t, uint32_t));
     MOCK_METHOD0(disableScissor, void());
     MOCK_METHOD2(genTextures, void(size_t, uint32_t*));