Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[cronet_http] 🏗️ Use dart-define to determine dependency #1111

Merged
merged 15 commits into from
Feb 23, 2024
18 changes: 6 additions & 12 deletions .github/workflows/cronet.yml
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,10 @@ jobs:
runs-on: macos-latest
strategy:
matrix:
package: ['cronet_http', 'cronet_http_embedded']
cronetHttpNoPlay: ['false', 'true']
defaults:
run:
working-directory: pkgs/cronet_http
steps:
- uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11
- uses: actions/setup-java@387ac29b308b003ca37ba93a6cab5eb57c8f5f93
Expand All @@ -36,23 +39,14 @@ jobs:
- uses: subosito/flutter-action@2783a3f08e1baf891508463f8c6653c258246225
with:
channel: 'stable'
- name: Make cronet_http_embedded copy
if: ${{ matrix.package == 'cronet_http_embedded' }}
run: |
mv pkgs/cronet_http pkgs/cronet_http_embedded
cd pkgs/cronet_http_embedded
flutter pub get && dart tool/prepare_for_embedded.dart
- id: install
name: Install dependencies
working-directory: 'pkgs/${{ matrix.package }}'
run: flutter pub get
- name: Check formatting
if: always() && steps.install.outcome == 'success'
working-directory: 'pkgs/${{ matrix.package }}'
run: dart format --output=none --set-exit-if-changed .
- name: Analyze code
if: always() && steps.install.outcome == 'success'
working-directory: 'pkgs/${{ matrix.package }}'
run: flutter analyze --fatal-infos
- name: Run tests
uses: reactivecircus/android-emulator-runner@6b0df4b0efb23bb0ec63d881db79aefbc976e4b2
Expand All @@ -64,6 +58,6 @@ jobs:
# - pkgs/cronet_http/example/android/app/build.gradle
api-level: 21
arch: x86_64
target: ${{ matrix.package == 'cronet_http_embedded' && 'default' || 'google_apis' }}
target: ${{ matrix.cronetHttpNoPlay == 'true' && 'default' || 'google_apis' }}
profile: pixel
script: cd pkgs/${{ matrix.package }}/example && flutter test --timeout=1200s integration_test/
script: cd pkgs/cronet_http/example && flutter test --dart-define=cronetHttpNoPlay=${{ matrix.cronetHttpNoPlay }} --timeout=1200s integration_test/
4 changes: 4 additions & 0 deletions pkgs/cronet_http/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
## 1.2.0-wip
brianquinlan marked this conversation as resolved.
Show resolved Hide resolved

* Support the Cronet embedding dependency with `--dart-define=cronetHttpNoPlay=true`.

## 1.1.1

* Make it possible to construct `CronetClient` with custom a `CronetEngine`
Expand Down
55 changes: 38 additions & 17 deletions pkgs/cronet_http/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,22 +2,20 @@
[![package publisher](https://img.shields.io/pub/publisher/cronet_http.svg)](https://pub.dev/packages/cronet_http/publisher)

An Android Flutter plugin that provides access to the
[Cronet][]
HTTP client.
[Cronet][] HTTP client.

Cronet is available as part of
[Google Play Services][].
Cronet is available as part of [Google Play Services][]
and as [a standalone embedded library][].

This package depends on [Google Play Services][] for its [Cronet][]
implementation.
[`package:cronet_http_embedded`](https://pub.dev/packages/cronet_http_embedded)
is functionally identical to this package but embeds [Cronet][] directly
instead of relying on [Google Play Services][].
This package depends on [Google Play Services][]
for its [Cronet][] implementation.
To use the embedded version of [Cronet][] without [Google Play Services][],
see [Use embedded Cronet](#use-embedded-cronet).

## Motivation

Using [Cronet][], rather than the socket-based [dart:io HttpClient][]
implemententation, has several advantages:
Using [Cronet][], rather than the socket-based
[dart:io HttpClient][] implementation, has several advantages:

1. It automatically supports Android platform features such as HTTP proxies.
2. It supports configurable caching.
Expand All @@ -40,23 +38,46 @@ void main() async {
final Client httpClient;
if (Platform.isAndroid) {
final engine = CronetEngine.build(
cacheMode: CacheMode.memory,
cacheMaxSize: 2 * 1024 * 1024,
userAgent: 'Book Agent');
cacheMode: CacheMode.memory,
cacheMaxSize: 2 * 1024 * 1024,
userAgent: 'Book Agent',
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There seems to be some random formatting changes in this file...isn't this formatting actually incorrect?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Flutter prefers trailing commas, which makes the formatting different.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This just looks misindented though. Can you change it back?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, the extra space. Would it be accepted if I only took it away?

);
httpClient = CronetClient.fromCronetEngine(engine, isOwned: true);
} else {
httpClient = IOClient(HttpClient()..userAgent = 'Book Agent');
}

final response = await client.get(Uri.https(
final response = await client.get(
Uri.https(
'www.googleapis.com',
'/books/v1/volumes',
{'q': 'HTTP', 'maxResults': '40', 'printType': 'books'}));
{'q': 'HTTP', 'maxResults': '40', 'printType': 'books'},
),
);
httpClient.close();
}
```

### Use embedded Cronet

If you want your application to work without [Google Play Services][],
you can instead depend on the `org.chromium.net:cronet-embedded` package
by using `dart-define` to set `cronetHttpNoPlay` is set to `true`.

For example:

```
flutter run --dart-define=cronetHttpNoPlay=true
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The convention seems to be to use name_with_underscore. So maybe:

cronet_http_no_play=true

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any guide suggesting this?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, I just looked at some existing defines e.g. test_runner.configuration.

But it is not consistent so feel free to leave it this way.

```

To use the embedded version in `flutter test`:

```
flutter test --dart-define=cronetHttpNoPlay=true
```

[Cronet]: https://developer.android.com/guide/topics/connectivity/cronet/reference/org/chromium/net/package-summary
[dart:io HttpClient]: https://api.dart.dev/stable/dart-io/HttpClient-class.html
[Google Play Services]: https://developers.google.com/android/guides/overview
[a standalone embedded library]: https://mvnrepository.com/artifact/org.chromium.net/cronet-embedded
[dart:io HttpClient]: https://api.dart.dev/stable/dart-io/HttpClient-class.html
[package:http Client]: https://pub.dev/documentation/http/latest/http/Client-class.html
12 changes: 0 additions & 12 deletions pkgs/cronet_http/README_EMBEDDED.md

This file was deleted.

17 changes: 16 additions & 1 deletion pkgs/cronet_http/android/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,17 @@ rootProject.allprojects {
}
}

def dartDefines = [
cronetHttpNoPlay: 'false'
]
if (project.hasProperty('dart-defines')) {
def defines = project.property('dart-defines').split(',').collectEntries { entry ->
def pair = new String(entry.decodeBase64(), 'UTF-8').split('=')
[(pair.first()): pair.last()]
}
dartDefines = dartDefines + defines
}

apply plugin: 'com.android.library'
apply plugin: 'kotlin-android'

Expand Down Expand Up @@ -65,5 +76,9 @@ android {
}

dependencies {
implementation "com.google.android.gms:play-services-cronet:18.0.1"
if (dartDefines.cronetHttpNoPlay == 'true') {
implementation 'org.chromium.net:cronet-embedded:113.5672.61'
} else {
implementation "com.google.android.gms:play-services-cronet:18.0.1"
}
}
2 changes: 1 addition & 1 deletion pkgs/cronet_http/pubspec.yaml
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
name: cronet_http
version: 1.1.1
version: 1.2.0-wip
description: >-
An Android Flutter plugin that provides access to the Cronet HTTP client.
repository: https://github.com/dart-lang/http/tree/master/pkgs/cronet_http
Expand Down
162 changes: 0 additions & 162 deletions pkgs/cronet_http/tool/prepare_for_embedded.dart

This file was deleted.