okhttp,interop-testing: use forked OkHttp classes #3325
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ejona86
requested changes
Aug 10, 2017
| @@ -37,7 +34,10 @@ | |||
| import io.grpc.internal.KeepAliveManager; | |||
| import io.grpc.internal.SharedResourceHolder; | |||
| import io.grpc.internal.SharedResourceHolder.Resource; | |||
| import io.grpc.okhttp.internal.CipherSuite; | |||
| import io.grpc.okhttp.internal.ConnectionSpec; | |||
There was a problem hiding this comment.
ConnectionSpec is part of grpc's API in this class. We don't want to expose our internal fork, so we let users specify okhttp's ConnectionSpec and then we use Utils.convertSpec() to copy it into an internal equivalent.
Feel free to eagerly convert in connectionSpec(); that would allow proguard to remove the references if it isn't specified.
|
Superseded by #4267 |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This reduces our dependency on the OkHttp 2.5.0 library by switching imports of three classes (
CipherSuite,ConnectionSpec, andTlsVersion) fromcom.squareup.okhttp.*to our already manually copied/forked classes inio.grpc.okhttp.internal.*.For our Android Hello World app after Proguard, this change reduces the classes.dex size of our
com.squareup.okhttp.*dependencies from 12.3KB to 4.0KB, and a modest reduction in APK size of 4.3KB (from 710.8KB to 706.5KB).The following classes are still direct dependencies of
OkHttpClientTransport:None of these are forked in
io.grpc.okhttp.internal, althoughStatusLinebrings incom.squareup.okhttp.Protocol, which is already forked asio.grpc.okhttp.internal.Protocol. We could save an additional 400B out of the 4KB of thecom.squareup.okhttp.*dependencies by forkingStatusLine(and forking its dependencycom.squareup.okhttp.Response).I manually downloaded the OkHttp 2.5.0 jar and verified that the implementations of
CipherSuite,ConnectionSpec, andTlsVersionmatch our own forked copies (we do have one method with increased visibility in our copy ofConnectionSpec).