Merge "Add DeathRecipient and IBinder to public_api." am: 2a78ffe757 am: 15010434fd

Original change: https://android-review.googlesource.com/c/platform/frameworks/native/+/1619403

Change-Id: I2ab1b808e5c9d3ca677c209854099ca82054fd7f

4 files changed, 80 insertions, 51 deletions

diff --git a/libs/binder/rust/src/binder.rs b/libs/binder/rust/src/binder.rs
index d53a88f726..3899d47bd6 100644
--- a/libs/binder/rust/src/binder.rs
+++ b/libs/binder/rust/src/binder.rs
@@ -33,13 +33,13 @@ use std::ptr;
 
 /// Binder action to perform.
 ///
-/// This must be a number between [`IBinder::FIRST_CALL_TRANSACTION`] and
-/// [`IBinder::LAST_CALL_TRANSACTION`].
+/// This must be a number between [`FIRST_CALL_TRANSACTION`] and
+/// [`LAST_CALL_TRANSACTION`].
 pub type TransactionCode = u32;
 
 /// Additional operation flags.
 ///
-/// `IBinder::FLAG_*` values.
+/// `FLAG_*` values.
 pub type TransactionFlags = u32;
 
 /// Super-trait for Binder interfaces.
@@ -85,20 +85,22 @@ pub trait Remotable: Send + Sync {
     fn get_class() -> InterfaceClass;
 }
 
-/// Interface of binder local or remote objects.
-///
-/// This trait corresponds to the interface of the C++ `IBinder` class.
-pub trait IBinder {
-    /// First transaction code available for user commands (inclusive)
-    const FIRST_CALL_TRANSACTION: TransactionCode = sys::FIRST_CALL_TRANSACTION;
-    /// Last transaction code available for user commands (inclusive)
-    const LAST_CALL_TRANSACTION: TransactionCode = sys::LAST_CALL_TRANSACTION;
+/// First transaction code available for user commands (inclusive)
+pub const FIRST_CALL_TRANSACTION: TransactionCode = sys::FIRST_CALL_TRANSACTION;
+/// Last transaction code available for user commands (inclusive)
+pub const LAST_CALL_TRANSACTION: TransactionCode = sys::LAST_CALL_TRANSACTION;
 
-    /// Corresponds to TF_ONE_WAY -- an asynchronous call.
-    const FLAG_ONEWAY: TransactionFlags = sys::FLAG_ONEWAY;
-    /// Corresponds to TF_CLEAR_BUF -- clear transaction buffers after call is made.
-    const FLAG_CLEAR_BUF: TransactionFlags = sys::FLAG_CLEAR_BUF;
+/// Corresponds to TF_ONE_WAY -- an asynchronous call.
+pub const FLAG_ONEWAY: TransactionFlags = sys::FLAG_ONEWAY;
+/// Corresponds to TF_CLEAR_BUF -- clear transaction buffers after call is made.
+pub const FLAG_CLEAR_BUF: TransactionFlags = sys::FLAG_CLEAR_BUF;
 
+/// Internal interface of binder local or remote objects for making
+/// transactions.
+///
+/// This trait corresponds to the parts of the interface of the C++ `IBinder`
+/// class which are internal implementation details.
+pub trait IBinderInternal: IBinder {
     /// Is this object still alive?
     fn is_binder_alive(&self) -> bool;
 
@@ -122,19 +124,24 @@ pub trait IBinder {
     /// * `data` - [`Parcel`] with input data
     /// * `reply` - Optional [`Parcel`] for reply data
     /// * `flags` - Transaction flags, e.g. marking the transaction as
-    /// asynchronous ([`FLAG_ONEWAY`](IBinder::FLAG_ONEWAY))
+    ///   asynchronous ([`FLAG_ONEWAY`](FLAG_ONEWAY))
     fn transact<F: FnOnce(&mut Parcel) -> Result<()>>(
         &self,
         code: TransactionCode,
         flags: TransactionFlags,
         input_callback: F,
     ) -> Result<Parcel>;
+}
 
+/// Interface of binder local or remote objects.
+///
+/// This trait corresponds to the parts of the interface of the C++ `IBinder`
+/// class which are public.
+pub trait IBinder {
     /// Register the recipient for a notification if this binder
     /// goes away. If this binder object unexpectedly goes away
     /// (typically because its hosting process has been killed),
-    /// then DeathRecipient::binder_died() will be called with a reference
-    /// to this.
+    /// then the `DeathRecipient`'s callback will be called.
     ///
     /// You will only receive death notifications for remote binders,
     /// as local binders by definition can't die without you dying as well.
@@ -142,11 +149,6 @@ pub trait IBinder {
     /// INVALID_OPERATION code being returned and nothing happening.
     ///
     /// This link always holds a weak reference to its recipient.
-    ///
-    /// You will only receive a weak reference to the dead
-    /// binder. You should not try to promote this to a strong reference.
-    /// (Nor should you need to, as there is nothing useful you can
-    /// directly do with it now that it has passed on.)
     fn link_to_death(&mut self, recipient: &mut DeathRecipient) -> Result<()>;
 
     /// Remove a previously registered death notification.
@@ -222,7 +224,8 @@ impl InterfaceClass {
             // the number of u16 elements before the null terminator.
 
             let raw_descriptor: *const c_char = sys::AIBinder_Class_getDescriptor(self.0);
-            CStr::from_ptr(raw_descriptor).to_str()
+            CStr::from_ptr(raw_descriptor)
+                .to_str()
                 .expect("Expected valid UTF-8 string from AIBinder_Class_getDescriptor")
                 .into()
         }
diff --git a/libs/binder/rust/src/lib.rs b/libs/binder/rust/src/lib.rs
index 43a237abf9..5bbd2a3870 100644
--- a/libs/binder/rust/src/lib.rs
+++ b/libs/binder/rust/src/lib.rs
@@ -107,8 +107,9 @@ use binder_ndk_sys as sys;
 pub mod parcel;
 
 pub use crate::binder::{
-    FromIBinder, IBinder, Interface, InterfaceClass, Remotable, Strong, TransactionCode,
-    TransactionFlags, Weak,
+    FromIBinder, IBinder, IBinderInternal, Interface, InterfaceClass, Remotable, Strong,
+    TransactionCode, TransactionFlags, Weak, FIRST_CALL_TRANSACTION, FLAG_CLEAR_BUF, FLAG_ONEWAY,
+    LAST_CALL_TRANSACTION,
 };
 pub use error::{status_t, ExceptionCode, Result, Status, StatusCode};
 pub use native::add_service;
@@ -123,8 +124,8 @@ pub mod public_api {
     pub use super::parcel::ParcelFileDescriptor;
     pub use super::{add_service, get_interface};
     pub use super::{
-        ExceptionCode, Interface, ProcessState, SpIBinder, Status, StatusCode, Strong, ThreadState,
-        Weak, WpIBinder,
+        DeathRecipient, ExceptionCode, IBinder, Interface, ProcessState, SpIBinder, Status,
+        StatusCode, Strong, ThreadState, Weak, WpIBinder,
     };
 
     /// Binder result containing a [`Status`] on error.
diff --git a/libs/binder/rust/src/proxy.rs b/libs/binder/rust/src/proxy.rs
index 132e0758e1..52036f5312 100644
--- a/libs/binder/rust/src/proxy.rs
+++ b/libs/binder/rust/src/proxy.rs
@@ -17,7 +17,8 @@
 //! Rust API for interacting with a remote binder service.
 
 use crate::binder::{
-    AsNative, FromIBinder, IBinder, Interface, InterfaceClass, Strong, TransactionCode, TransactionFlags,
+    AsNative, FromIBinder, IBinder, IBinderInternal, Interface, InterfaceClass, Strong,
+    TransactionCode, TransactionFlags,
 };
 use crate::error::{status_result, Result, StatusCode};
 use crate::parcel::{
@@ -26,8 +27,8 @@ use crate::parcel::{
 };
 use crate::sys;
 
-use std::convert::TryInto;
 use std::cmp::Ordering;
+use std::convert::TryInto;
 use std::ffi::{c_void, CString};
 use std::fmt;
 use std::os::unix::io::AsRawFd;
@@ -211,7 +212,7 @@ impl Drop for SpIBinder {
     }
 }
 
-impl<T: AsNative<sys::AIBinder>> IBinder for T {
+impl<T: AsNative<sys::AIBinder>> IBinderInternal for T {
     /// Perform a binder transaction
     fn transact<F: FnOnce(&mut Parcel) -> Result<()>>(
         &self,
@@ -300,9 +301,7 @@ impl<T: AsNative<sys::AIBinder>> IBinder for T {
     }
 
     fn set_requesting_sid(&mut self, enable: bool) {
-        unsafe {
-            sys::AIBinder_setRequestingSid(self.as_native_mut(), enable)
-        };
+        unsafe { sys::AIBinder_setRequestingSid(self.as_native_mut(), enable) };
     }
 
     fn dump<F: AsRawFd>(&mut self, fp: &F, args: &[&str]) -> Result<()> {
@@ -351,7 +350,9 @@ impl<T: AsNative<sys::AIBinder>> IBinder for T {
         status_result(status)?;
         Ok(ibinder)
     }
+}
 
+impl<T: AsNative<sys::AIBinder>> IBinder for T {
     fn link_to_death(&mut self, recipient: &mut DeathRecipient) -> Result<()> {
         status_result(unsafe {
             // Safety: `SpIBinder` guarantees that `self` always contains a
@@ -472,7 +473,10 @@ impl Clone for WpIBinder {
             // WpIBinder object from it.
             sys::AIBinder_Weak_clone(self.0)
         };
-        assert!(!ptr.is_null(), "Unexpected null pointer from AIBinder_Weak_clone");
+        assert!(
+            !ptr.is_null(),
+            "Unexpected null pointer from AIBinder_Weak_clone"
+        );
         Self(ptr)
     }
 }
diff --git a/libs/binder/rust/tests/integration.rs b/libs/binder/rust/tests/integration.rs
index 719229c9b7..60e3502759 100644
--- a/libs/binder/rust/tests/integration.rs
+++ b/libs/binder/rust/tests/integration.rs
@@ -18,7 +18,10 @@
 
 use binder::declare_binder_interface;
 use binder::parcel::Parcel;
-use binder::{Binder, IBinder, Interface, SpIBinder, StatusCode, ThreadState, TransactionCode};
+use binder::{
+    Binder, IBinderInternal, Interface, StatusCode, ThreadState, TransactionCode,
+    FIRST_CALL_TRANSACTION,
+};
 use std::convert::{TryFrom, TryInto};
 
 /// Name of service runner.
@@ -83,7 +86,7 @@ struct TestService {
 
 #[repr(u32)]
 enum TestTransactionCode {
-    Test = SpIBinder::FIRST_CALL_TRANSACTION,
+    Test = FIRST_CALL_TRANSACTION,
     GetSelinuxContext,
 }
 
@@ -196,7 +199,6 @@ impl ITestSameDescriptor for BpTestSameDescriptor {}
 
 impl ITestSameDescriptor for Binder<BnTestSameDescriptor> {}
 
-
 #[cfg(test)]
 mod tests {
     use selinux_bindgen as selinux_sys;
@@ -209,9 +211,12 @@ mod tests {
     use std::thread;
     use std::time::Duration;
 
-    use binder::{Binder, DeathRecipient, FromIBinder, IBinder, Interface, SpIBinder, StatusCode, Strong};
+    use binder::{
+        Binder, DeathRecipient, FromIBinder, IBinder, IBinderInternal, Interface, SpIBinder,
+        StatusCode, Strong,
+    };
 
-    use super::{BnTest, ITest, ITestSameDescriptor, RUST_SERVICE_BINARY, TestService};
+    use super::{BnTest, ITest, ITestSameDescriptor, TestService, RUST_SERVICE_BINARY};
 
     pub struct ScopedServiceProcess(Child);
 
@@ -290,7 +295,9 @@ mod tests {
         };
         assert_eq!(
             test_client.get_selinux_context().unwrap(),
-            expected_context.to_str().expect("context was invalid UTF-8"),
+            expected_context
+                .to_str()
+                .expect("context was invalid UTF-8"),
         );
     }
 
@@ -479,18 +486,22 @@ mod tests {
 
         // This should succeed although we will have to treat the service as
         // remote.
-        let _interface: Strong<dyn ITestSameDescriptor> = FromIBinder::try_from(service.as_binder())
-            .expect("Could not re-interpret service as the ITestSameDescriptor interface");
+        let _interface: Strong<dyn ITestSameDescriptor> =
+            FromIBinder::try_from(service.as_binder())
+                .expect("Could not re-interpret service as the ITestSameDescriptor interface");
     }
 
     /// Test that we can round-trip a rust service through a generic IBinder
     #[test]
     fn reassociate_rust_binder() {
         let service_name = "testing_service";
-        let service_ibinder = BnTest::new_binder(TestService { s: service_name.to_string() })
-            .as_binder();
+        let service_ibinder = BnTest::new_binder(TestService {
+            s: service_name.to_string(),
+        })
+        .as_binder();
 
-        let service: Strong<dyn ITest> = service_ibinder.into_interface()
+        let service: Strong<dyn ITest> = service_ibinder
+            .into_interface()
             .expect("Could not reassociate the generic ibinder");
 
         assert_eq!(service.test().unwrap(), service_name);
@@ -499,7 +510,9 @@ mod tests {
     #[test]
     fn weak_binder_upgrade() {
         let service_name = "testing_service";
-        let service = BnTest::new_binder(TestService { s: service_name.to_string() });
+        let service = BnTest::new_binder(TestService {
+            s: service_name.to_string(),
+        });
 
         let weak = Strong::downgrade(&service);
 
@@ -512,7 +525,9 @@ mod tests {
     fn weak_binder_upgrade_dead() {
         let service_name = "testing_service";
         let weak = {
-            let service = BnTest::new_binder(TestService { s: service_name.to_string() });
+            let service = BnTest::new_binder(TestService {
+                s: service_name.to_string(),
+            });
 
             Strong::downgrade(&service)
         };
@@ -523,7 +538,9 @@ mod tests {
     #[test]
     fn weak_binder_clone() {
         let service_name = "testing_service";
-        let service = BnTest::new_binder(TestService { s: service_name.to_string() });
+        let service = BnTest::new_binder(TestService {
+            s: service_name.to_string(),
+        });
 
         let weak = Strong::downgrade(&service);
         let cloned = weak.clone();
@@ -539,8 +556,12 @@ mod tests {
     #[test]
     #[allow(clippy::eq_op)]
     fn binder_ord() {
-        let service1 = BnTest::new_binder(TestService { s: "testing_service1".to_string() });
-        let service2 = BnTest::new_binder(TestService { s: "testing_service2".to_string() });
+        let service1 = BnTest::new_binder(TestService {
+            s: "testing_service1".to_string(),
+        });
+        let service2 = BnTest::new_binder(TestService {
+            s: "testing_service2".to_string(),
+        });
 
         assert!(!(service1 < service1));
         assert!(!(service1 > service1));