Merge RQ2A.210305.007

Bug: 180401296
Merged-In: I2f7cee6cdae4b16c5adeb7af751a591a939948e2
Change-Id: I6c6bab18dc342c4456e8fd2db35173568fab1407

7 files changed, 79 insertions, 63 deletions

diff --git a/libs/binder/LazyServiceRegistrar.cpp b/libs/binder/LazyServiceRegistrar.cpp
index 21a9251b79..f96b6bb4eb 100644
--- a/libs/binder/LazyServiceRegistrar.cpp
+++ b/libs/binder/LazyServiceRegistrar.cpp
@@ -30,16 +30,12 @@ namespace internal {
 
 using AidlServiceManager = android::os::IServiceManager;
 
-class ClientCounterCallback : public ::android::os::BnClientCallback {
+class ClientCounterCallbackImpl : public ::android::os::BnClientCallback {
 public:
-    ClientCounterCallback() : mNumConnectedServices(0), mForcePersist(false) {}
+    ClientCounterCallbackImpl() : mNumConnectedServices(0), mForcePersist(false) {}
 
     bool registerService(const sp<IBinder>& service, const std::string& name,
                          bool allowIsolated, int dumpFlags);
-
-    /**
-     * Set a flag to prevent services from automatically shutting down
-     */
     void forcePersist(bool persist);
 
     void setActiveServicesCallback(const std::function<bool(bool)>& activeServicesCallback);
@@ -97,7 +93,29 @@ private:
     std::function<bool(bool)> mActiveServicesCallback;
 };
 
-bool ClientCounterCallback::registerService(const sp<IBinder>& service, const std::string& name,
+class ClientCounterCallback {
+public:
+    ClientCounterCallback();
+
+    bool registerService(const sp<IBinder>& service, const std::string& name,
+                                            bool allowIsolated, int dumpFlags);
+
+    /**
+     * Set a flag to prevent services from automatically shutting down
+     */
+    void forcePersist(bool persist);
+
+    void setActiveServicesCallback(const std::function<bool(bool)>& activeServicesCallback);
+
+    bool tryUnregister();
+
+    void reRegister();
+
+private:
+    sp<ClientCounterCallbackImpl> mImpl;
+};
+
+bool ClientCounterCallbackImpl::registerService(const sp<IBinder>& service, const std::string& name,
                                             bool allowIsolated, int dumpFlags) {
     auto manager = interface_cast<AidlServiceManager>(asBinder(defaultServiceManager()));
 
@@ -111,7 +129,7 @@ bool ClientCounterCallback::registerService(const sp<IBinder>& service, const st
     }
 
     if (!reRegister) {
-        if (!manager->registerClientCallback(name, service, this).isOk()) {
+        if(!manager->registerClientCallback(name, service, this).isOk()) {
             ALOGE("Failed to add client callback for service %s", name.c_str());
             return false;
         }
@@ -127,7 +145,7 @@ bool ClientCounterCallback::registerService(const sp<IBinder>& service, const st
     return true;
 }
 
-std::map<std::string, ClientCounterCallback::Service>::iterator ClientCounterCallback::assertRegisteredService(const sp<IBinder>& service) {
+std::map<std::string, ClientCounterCallbackImpl::Service>::iterator ClientCounterCallbackImpl::assertRegisteredService(const sp<IBinder>& service) {
     LOG_ALWAYS_FATAL_IF(service == nullptr, "Got onClients callback for null service");
     for (auto it = mRegisteredServices.begin(); it != mRegisteredServices.end(); ++it) {
         auto const& [name, registered] = *it;
@@ -139,7 +157,7 @@ std::map<std::string, ClientCounterCallback::Service>::iterator ClientCounterCal
     __builtin_unreachable();
 }
 
-void ClientCounterCallback::forcePersist(bool persist) {
+void ClientCounterCallbackImpl::forcePersist(bool persist) {
     mForcePersist = persist;
     if (!mForcePersist) {
         // Attempt a shutdown in case the number of clients hit 0 while the flag was on
@@ -147,7 +165,7 @@ void ClientCounterCallback::forcePersist(bool persist) {
     }
 }
 
-bool ClientCounterCallback::tryUnregister() {
+bool ClientCounterCallbackImpl::tryUnregister() {
     auto manager = interface_cast<AidlServiceManager>(asBinder(defaultServiceManager()));
 
     for (auto& [name, entry] : mRegisteredServices) {
@@ -163,7 +181,7 @@ bool ClientCounterCallback::tryUnregister() {
     return true;
 }
 
-void ClientCounterCallback::reRegister() {
+void ClientCounterCallbackImpl::reRegister() {
     for (auto& [name, entry] : mRegisteredServices) {
         // re-register entry if not already registered
         if (entry.registered) {
@@ -180,7 +198,7 @@ void ClientCounterCallback::reRegister() {
     }
 }
 
-void ClientCounterCallback::maybeTryShutdown() {
+void ClientCounterCallbackImpl::maybeTryShutdown() {
     if (mForcePersist) {
         ALOGI("Shutdown prevented by forcePersist override flag.");
         return;
@@ -207,7 +225,7 @@ void ClientCounterCallback::maybeTryShutdown() {
  * onClients is oneway, so no need to worry about multi-threading. Note that this means multiple
  * invocations could occur on different threads however.
  */
-Status ClientCounterCallback::onClients(const sp<IBinder>& service, bool clients) {
+Status ClientCounterCallbackImpl::onClients(const sp<IBinder>& service, bool clients) {
     auto & [name, registered] = *assertRegisteredService(service);
     if (registered.clients == clients) {
         LOG_ALWAYS_FATAL("Process already thought %s had clients: %d but servicemanager has "
@@ -229,24 +247,49 @@ Status ClientCounterCallback::onClients(const sp<IBinder>& service, bool clients
           mNumConnectedServices, mRegisteredServices.size(), name.c_str(), clients);
 
     maybeTryShutdown();
-
     return Status::ok();
 }
 
-void ClientCounterCallback::tryShutdown() {
-    ALOGI("Trying to shut down the service. No clients in use for any service in process.");
+ void ClientCounterCallbackImpl::tryShutdown() {
+     ALOGI("Trying to shut down the service. No clients in use for any service in process.");
 
     if (tryUnregister()) {
-        ALOGI("Unregistered all clients and exiting");
-        exit(EXIT_SUCCESS);
-    }
+         ALOGI("Unregistered all clients and exiting");
+         exit(EXIT_SUCCESS);
+     }
 
     reRegister();
 }
 
+void ClientCounterCallbackImpl::setActiveServicesCallback(const std::function<bool(bool)>&
+                                                          activeServicesCallback) {
+    mActiveServicesCallback = activeServicesCallback;
+}
+
+ClientCounterCallback::ClientCounterCallback() {
+      mImpl = sp<ClientCounterCallbackImpl>::make();
+}
+
+bool ClientCounterCallback::registerService(const sp<IBinder>& service, const std::string& name,
+                                            bool allowIsolated, int dumpFlags) {
+    return mImpl->registerService(service, name, allowIsolated, dumpFlags);
+}
+
+void ClientCounterCallback::forcePersist(bool persist) {
+    mImpl->forcePersist(persist);
+}
+
 void ClientCounterCallback::setActiveServicesCallback(const std::function<bool(bool)>&
                                                       activeServicesCallback) {
-    mActiveServicesCallback = activeServicesCallback;
+    mImpl->setActiveServicesCallback(activeServicesCallback);
+}
+
+bool ClientCounterCallback::tryUnregister() {
+    return mImpl->tryUnregister();
+}
+
+void ClientCounterCallback::reRegister() {
+    mImpl->reRegister();
 }
 
 }  // namespace internal
diff --git a/libs/binder/Parcel.cpp b/libs/binder/Parcel.cpp
index 8bed621ab4..1a4ede1940 100644
--- a/libs/binder/Parcel.cpp
+++ b/libs/binder/Parcel.cpp
@@ -1651,7 +1651,10 @@ const char* Parcel::readString8Inplace(size_t* outLen) const
         *outLen = size;
         const char* str = (const char*)readInplace(size+1);
         if (str != nullptr) {
-            return str;
+            if (str[size] == '\0') {
+                return str;
+            }
+            android_errorWriteLog(0x534e4554, "172655291");
         }
     }
     *outLen = 0;
@@ -1689,7 +1692,10 @@ const char16_t* Parcel::readString16Inplace(size_t* outLen) const
         *outLen = size;
         const char16_t* str = (const char16_t*)readInplace((size+1)*sizeof(char16_t));
         if (str != nullptr) {
-            return str;
+            if (str[size] == u'\0') {
+                return str;
+            }
+            android_errorWriteLog(0x534e4554, "172655291");
         }
     }
     *outLen = 0;
diff --git a/libs/binder/ndk/ibinder.cpp b/libs/binder/ndk/ibinder.cpp
index 3c906819ff..0f59de4309 100644
--- a/libs/binder/ndk/ibinder.cpp
+++ b/libs/binder/ndk/ibinder.cpp
@@ -181,7 +181,7 @@ status_t ABBinder::onTransact(transaction_code_t code, const Parcel& data, Parce
 
         binder_status_t status = getClass()->onTransact(this, code, &in, &out);
         return PruneStatusT(status);
-    } else if (code == SHELL_COMMAND_TRANSACTION && getClass()->handleShellCommand != nullptr) {
+    } else if (code == SHELL_COMMAND_TRANSACTION) {
         int in = data.readFileDescriptor();
         int out = data.readFileDescriptor();
         int err = data.readFileDescriptor();
diff --git a/libs/binder/ndk/ibinder_internal.h b/libs/binder/ndk/ibinder_internal.h
index 6824306fbf..22cacb4e08 100644
--- a/libs/binder/ndk/ibinder_internal.h
+++ b/libs/binder/ndk/ibinder_internal.h
@@ -116,13 +116,13 @@ struct AIBinder_Class {
     const char* getInterfaceDescriptorUtf8() const { return mInterfaceDescriptor.c_str(); }
 
     // required to be non-null, implemented for every class
-    const AIBinder_Class_onCreate onCreate = nullptr;
-    const AIBinder_Class_onDestroy onDestroy = nullptr;
-    const AIBinder_Class_onTransact onTransact = nullptr;
+    const AIBinder_Class_onCreate onCreate;
+    const AIBinder_Class_onDestroy onDestroy;
+    const AIBinder_Class_onTransact onTransact;
 
     // optional methods for a class
-    AIBinder_onDump onDump = nullptr;
-    AIBinder_handleShellCommand handleShellCommand = nullptr;
+    AIBinder_onDump onDump;
+    AIBinder_handleShellCommand handleShellCommand;
 
    private:
     // Copy of the raw char string for when we don't have to return UTF-16
diff --git a/libs/binder/ndk/tests/iface.cpp b/libs/binder/ndk/tests/iface.cpp
index 2afe5d2058..53b5c3c320 100644
--- a/libs/binder/ndk/tests/iface.cpp
+++ b/libs/binder/ndk/tests/iface.cpp
@@ -118,7 +118,7 @@ IFoo::~IFoo() {
     AIBinder_Weak_delete(mWeakBinder);
 }
 
-AIBinder* IFoo::getBinder() {
+binder_status_t IFoo::addService(const char* instance) {
     AIBinder* binder = nullptr;
 
     if (mWeakBinder != nullptr) {
@@ -132,18 +132,8 @@ AIBinder* IFoo::getBinder() {
             AIBinder_Weak_delete(mWeakBinder);
         }
         mWeakBinder = AIBinder_Weak_new(binder);
-
-        // WARNING: it is important that this class does not implement debug or
-        // shell functions because it does not use special C++ wrapper
-        // functions, and so this is how we test those functions.
     }
 
-    return binder;
-}
-
-binder_status_t IFoo::addService(const char* instance) {
-    AIBinder* binder = getBinder();
-
     binder_status_t status = AServiceManager_addService(binder, instance);
     // Strong references we care about kept by remote process
     AIBinder_decStrong(binder);
diff --git a/libs/binder/ndk/tests/include/iface/iface.h b/libs/binder/ndk/tests/include/iface/iface.h
index 7408d0c5a9..244c9857ac 100644
--- a/libs/binder/ndk/tests/include/iface/iface.h
+++ b/libs/binder/ndk/tests/include/iface/iface.h
@@ -31,9 +31,6 @@ class IFoo : public virtual ::android::RefBase {
 
     static AIBinder_Class* kClass;
 
-    // binder representing this interface with one reference count
-    AIBinder* getBinder();
-
     // Takes ownership of IFoo
     binder_status_t addService(const char* instance);
     static ::android::sp<IFoo> getService(const char* instance, AIBinder** outBinder = nullptr);
diff --git a/libs/binder/ndk/tests/libbinder_ndk_unit_test.cpp b/libs/binder/ndk/tests/libbinder_ndk_unit_test.cpp
index 6a88401962..496a915f8b 100644
--- a/libs/binder/ndk/tests/libbinder_ndk_unit_test.cpp
+++ b/libs/binder/ndk/tests/libbinder_ndk_unit_test.cpp
@@ -241,26 +241,6 @@ TEST(NdkBinder, CheckServiceThatDoesExist) {
     AIBinder_decStrong(binder);
 }
 
-TEST(NdkBinder, UnimplementedDump) {
-    sp<IFoo> foo = IFoo::getService(IFoo::kSomeInstanceName);
-    ASSERT_NE(foo, nullptr);
-    AIBinder* binder = foo->getBinder();
-    EXPECT_EQ(OK, AIBinder_dump(binder, STDOUT_FILENO, nullptr, 0));
-    AIBinder_decStrong(binder);
-}
-
-TEST(NdkBinder, UnimplementedShell) {
-    // libbinder_ndk doesn't support calling shell, so we are calling from the
-    // libbinder across processes to the NDK service which doesn't implement
-    // shell
-    static const sp<android::IServiceManager> sm(android::defaultServiceManager());
-    sp<IBinder> testService = sm->getService(String16(IFoo::kSomeInstanceName));
-
-    Vector<String16> argsVec;
-    EXPECT_EQ(OK, IBinder::shellCommand(testService, STDIN_FILENO, STDOUT_FILENO, STDERR_FILENO,
-                                        argsVec, nullptr, nullptr));
-}
-
 TEST(NdkBinder, DoubleNumber) {
     sp<IFoo> foo = IFoo::getService(IFoo::kSomeInstanceName);
     ASSERT_NE(foo, nullptr);