binder: Promote out of experimental status #9669

cbianchi-7 · 2022-11-02T22:36:40Z

Removing the @experimentalapi annotation for APIs used by the App Interaction Library to enable gRPC to be used as dependency for IPCs for Jetpack.

…teraction service depends on.

linux-foundation-easycla · 2022-11-02T22:36:44Z

The committers listed above are authorized under a signed CLA.

✅ login: cbianchi-7 (a021cb7, ee1d14c)

binder/src/main/java/io/grpc/binder/SecurityPolicies.java

binder/src/main/java/io/grpc/binder/BinderServerBuilder.java

…tion and leaving the internalOnly() without.

binder/src/main/java/io/grpc/binder/BinderServerBuilder.java

ejona86

These comments are from our API review meeting. Overall, mostly nits. I don't expect most of these to be addressed directly in this PR. The two things that are impactful that we noticed involve IBinderReceiver and non-final AndroidComponentAddress. We can accept not touching IBinderReceiver. But final-izing AndroidComponentAddress seems reasonably important.

binder/src/main/java/io/grpc/binder/SecurityPolicies.java

binder/src/main/java/io/grpc/binder/IBinderReceiver.java

binder/src/main/java/io/grpc/binder/BinderServerBuilder.java

binder/src/main/java/io/grpc/binder/AndroidComponentAddress.java

…nboundParcelablePolicy per PR discussions.

ejona86 · 2022-11-04T23:45:34Z

If the user adds interceptors, they will run before the security interceptor, because interceptors wrap services. The last one is closest to the network.

grpc-java/binder/src/main/java/io/grpc/binder/BinderServerBuilder.java

Line 111 in a82ea0c

BinderTransportSecurity.installAuthInterceptor(this);

…ass.

binder/src/main/java/io/grpc/binder/SecurityPolicies.java

binder/src/main/java/io/grpc/binder/AndroidComponentAddress.java

…om several policies.

binder/src/main/java/io/grpc/binder/BinderServerBuilder.java

…ckage check in AndroidComponentAddress. BinderInternal class to expose IBinderReceiver methods.

… case.

…ing discussion.

cbianchi-7 · 2022-11-29T17:40:36Z

Have addressed and resolved all existing comments / changes. Are there any additional thoughts from folks?
@ejona86 , @jdcormie , @markb74

ejona86

Looks good. I noticed one thing. I think I'll fix it up myself and then approve.

binder/src/main/java/io/grpc/binder/BinderInternal.java

cbianchi-7 · 2022-11-29T18:28:53Z

That sounds good to me. Thanks for running the additional checks as well. All looks good.

ejona86

I'll let markb74 merge.

jdcormie

I wonder if readers in this forum will understand the connection to "app interaction sdk" in the PR description. How about "Promote binder out of experimental status" or similar?

cbianchi-7 · 2022-11-30T18:09:59Z

@jdcormie , I think that's a good suggestion to give better change context. I updated the description accordingly using the active tense.

jdcormie · 2022-11-30T18:47:52Z

binder/src/main/java/io/grpc/binder/AndroidComponentAddress.java

@@ -103,6 +101,10 @@ public static AndroidComponentAddress forComponent(ComponentName component) {
        new Intent(ApiConstants.ACTION_BIND).setComponent(component));
  }

+  /**
+   * Returns the Authority which is the package name of the target app.
+   * See {@link android.content.ComponentName}.


between paragraphs (https://google.github.io/styleguide/javaguide.html#s7.1.2-javadoc-paragraphs)

jdcormie · 2022-11-30T18:48:26Z

binder/src/main/java/io/grpc/binder/BinderInternal.java

+public class BinderInternal {
+
+  /**
+   * Set the receiver's {@link IBinder} using {@link IBinderReceiver#set(IBinder)}.


"Sets" (https://google.github.io/styleguide/javaguide.html#s7.2-summary-fragment)

jdcormie · 2022-11-30T18:53:08Z

binder/src/main/java/io/grpc/binder/SecurityPolicies.java

  public static ServerSecurityPolicy serverInternalOnly() {
    return new ServerSecurityPolicy();
  }

+  /**
+   * Creates a default {@link SecurityPolicy} that checks authorization based on UID.


"... that allows access only to callers with the same UID as the current process."

This reverts commit d6aa0ea.

cbianchi-7 added 2 commits November 2, 2022 15:32

Remove the experimental annotation for AndroidComponentAddress.

a021cb7

Removing the additional experimental annotations for apis that the in…

ee1d14c

…teraction service depends on.

markb74 reviewed Nov 3, 2022

View reviewed changes

binder/src/main/java/io/grpc/binder/SecurityPolicies.java Outdated Show resolved Hide resolved

markb74 reviewed Nov 3, 2022

View reviewed changes

binder/src/main/java/io/grpc/binder/BinderServerBuilder.java Show resolved Hide resolved

Updating the public APIs on SecurityPolicies with experimental annota…

a0fc978

…tion and leaving the internalOnly() without.

ejona86 added the kokoro:run Add this label to a PR to tell Kokoro the code is safe and tests can be run label Nov 3, 2022

grpc-kokoro removed the kokoro:run Add this label to a PR to tell Kokoro the code is safe and tests can be run label Nov 3, 2022

ejona86 reviewed Nov 3, 2022

View reviewed changes

binder/src/main/java/io/grpc/binder/BinderServerBuilder.java Show resolved Hide resolved

Marking enableStats() and enableTracing() as experimental.

724cab7

ejona86 mentioned this pull request Nov 3, 2022

Binderchannel (io.grpc.binder.*) is experimental. #8022

Open

ejona86 reviewed Nov 3, 2022

View reviewed changes

Marking the IBinder volatile, and adding experimental annotation to i…

c163f1c

…nboundParcelablePolicy per PR discussions.

cbianchi-7 added 2 commits November 7, 2022 09:21

Removing setting the compression defaults. Related to 236232634.

095259b

Removing the synchronized from the get and set due to the volatile cl…

ff5e64c

…ass.

markb74 reviewed Nov 9, 2022

View reviewed changes

binder/src/main/java/io/grpc/binder/SecurityPolicies.java Outdated Show resolved Hide resolved

markb74 reviewed Nov 9, 2022

View reviewed changes

binder/src/main/java/io/grpc/binder/SecurityPolicies.java Outdated Show resolved Hide resolved

markb74 reviewed Nov 9, 2022

View reviewed changes

binder/src/main/java/io/grpc/binder/SecurityPolicies.java Outdated Show resolved Hide resolved

markb74 reviewed Nov 9, 2022

View reviewed changes

binder/src/main/java/io/grpc/binder/SecurityPolicies.java Outdated Show resolved Hide resolved

markb74 reviewed Nov 9, 2022

View reviewed changes

binder/src/main/java/io/grpc/binder/SecurityPolicies.java Outdated Show resolved Hide resolved

markb74 reviewed Nov 9, 2022

View reviewed changes

binder/src/main/java/io/grpc/binder/SecurityPolicies.java Outdated Show resolved Hide resolved

markb74 reviewed Nov 9, 2022

View reviewed changes

binder/src/main/java/io/grpc/binder/AndroidComponentAddress.java Show resolved Hide resolved

Final AndroidComponentAddress and removing experimental annotation fr…

7a283dd

…om several policies.

ejona86 reviewed Nov 15, 2022

View reviewed changes

binder/src/main/java/io/grpc/binder/BinderServerBuilder.java Show resolved Hide resolved

ejona86 added the kokoro:run Add this label to a PR to tell Kokoro the code is safe and tests can be run label Nov 16, 2022

grpc-kokoro removed the kokoro:run Add this label to a PR to tell Kokoro the code is safe and tests can be run label Nov 16, 2022

cbianchi-7 added 3 commits November 15, 2022 17:25

Additional SecurityPolicies and BinderServerBuilder documentation. Pa…

f1f3ee5

…ckage check in AndroidComponentAddress. BinderInternal class to expose IBinderReceiver methods.

Documentation for internalOnly().

e92f90c

Re-adding stats and tracing disabling by default due to discussed GMM…

5313582

… case.

Marking SecurityPolicies API as non-experimental, and resolving exist…

1c9806e

…ing discussion.

ejona86 added the kokoro:run Add this label to a PR to tell Kokoro the code is safe and tests can be run label Nov 29, 2022

grpc-kokoro removed the kokoro:run Add this label to a PR to tell Kokoro the code is safe and tests can be run label Nov 29, 2022

ejona86 reviewed Nov 29, 2022

View reviewed changes

binder/src/main/java/io/grpc/binder/BinderInternal.java Show resolved Hide resolved

Add missing copyright and @Internal annotation

6630c75

ejona86 added the kokoro:run Add this label to a PR to tell Kokoro the code is safe and tests can be run label Nov 29, 2022

ejona86 approved these changes Nov 29, 2022

View reviewed changes

grpc-kokoro removed the kokoro:run Add this label to a PR to tell Kokoro the code is safe and tests can be run label Nov 29, 2022

markb74 approved these changes Nov 29, 2022

View reviewed changes

markb74 added the kokoro:run Add this label to a PR to tell Kokoro the code is safe and tests can be run label Nov 30, 2022

jdcormie reviewed Nov 30, 2022

View reviewed changes

cbianchi-7 changed the title ~~Remove experimental for app interaction sdk~~ Promoting binder out of experimental status Nov 30, 2022

ejona86 merged commit d6aa0ea into grpc:master Nov 30, 2022

ejona86 changed the title ~~Promoting binder out of experimental status~~ binder: Promote out of experimental status Nov 30, 2022

jdcormie reviewed Nov 30, 2022

View reviewed changes

cbianchi-7 mentioned this pull request Dec 1, 2022

binder: Additional documentation for promoting out of experimental status #9725

Merged

YifeiZhuang added a commit to YifeiZhuang/grpc-java that referenced this pull request Dec 12, 2022

Revert "binder: Promote out of experimental status (grpc#9669)"

662c5a9

This reverts commit d6aa0ea.

github-actions bot locked as resolved and limited conversation to collaborators Mar 1, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

binder: Promote out of experimental status #9669

binder: Promote out of experimental status #9669

cbianchi-7 commented Nov 2, 2022

linux-foundation-easycla bot commented Nov 2, 2022 •

edited

Loading

ejona86 left a comment

ejona86 commented Nov 4, 2022

cbianchi-7 commented Nov 29, 2022 •

edited

Loading

ejona86 left a comment

cbianchi-7 commented Nov 29, 2022

ejona86 left a comment

jdcormie left a comment

cbianchi-7 commented Nov 30, 2022

jdcormie Nov 30, 2022

jdcormie Nov 30, 2022

jdcormie Nov 30, 2022

binder: Promote out of experimental status #9669

binder: Promote out of experimental status #9669

Conversation

cbianchi-7 commented Nov 2, 2022

linux-foundation-easycla bot commented Nov 2, 2022 • edited Loading

ejona86 left a comment

Choose a reason for hiding this comment

ejona86 commented Nov 4, 2022

cbianchi-7 commented Nov 29, 2022 • edited Loading

ejona86 left a comment

Choose a reason for hiding this comment

cbianchi-7 commented Nov 29, 2022

ejona86 left a comment

Choose a reason for hiding this comment

jdcormie left a comment

Choose a reason for hiding this comment

cbianchi-7 commented Nov 30, 2022

jdcormie Nov 30, 2022

Choose a reason for hiding this comment

jdcormie Nov 30, 2022

Choose a reason for hiding this comment

jdcormie Nov 30, 2022

Choose a reason for hiding this comment

linux-foundation-easycla bot commented Nov 2, 2022 •

edited

Loading

cbianchi-7 commented Nov 29, 2022 •

edited

Loading