diff --git a/pkgs/cronet_http/CHANGELOG.md b/pkgs/cronet_http/CHANGELOG.md index bc9255116c..f8b134047d 100644 --- a/pkgs/cronet_http/CHANGELOG.md +++ b/pkgs/cronet_http/CHANGELOG.md @@ -1,3 +1,8 @@ +## 1.1.1 + +* Make it possible to construct `CronetClient` with custom a `CronetEngine` + while still allowing `CronetClient` to close the `CronetEngine`. + ## 1.1.0 * Use `package:http_image_provider` in the example application. diff --git a/pkgs/cronet_http/README.md b/pkgs/cronet_http/README.md index 47ebd8bac1..e53e54d19e 100644 --- a/pkgs/cronet_http/README.md +++ b/pkgs/cronet_http/README.md @@ -43,7 +43,7 @@ void main() async { cacheMode: CacheMode.memory, cacheMaxSize: 2 * 1024 * 1024, userAgent: 'Book Agent'); - httpClient = CronetClient.fromCronetEngine(engine); + httpClient = CronetClient.fromCronetEngine(engine, isOwned: true); } else { httpClient = IOClient(HttpClient()..userAgent = 'Book Agent'); } @@ -52,6 +52,7 @@ void main() async { 'www.googleapis.com', '/books/v1/volumes', {'q': 'HTTP', 'maxResults': '40', 'printType': 'books'})); + httpClient.close(); } ``` diff --git a/pkgs/cronet_http/example/integration_test/cronet_configuration_test.dart b/pkgs/cronet_http/example/integration_test/cronet_configuration_test.dart index f787ebc39a..63ce8eba0f 100644 --- a/pkgs/cronet_http/example/integration_test/cronet_configuration_test.dart +++ b/pkgs/cronet_http/example/integration_test/cronet_configuration_test.dart @@ -5,6 +5,7 @@ import 'dart:io'; import 'package:cronet_http/cronet_http.dart'; +import 'package:http/http.dart'; import 'package:integration_test/integration_test.dart'; import 'package:test/test.dart'; @@ -120,10 +121,39 @@ void testUserAgent() { }); } +void testEngineClose() { + group('engine close', () { + test('multiple close', () { + CronetEngine.build() + ..close() + ..close(); + }); + + test('request after close', () async { + final closedEngine = CronetEngine.build()..close(); + final client = CronetClient.fromCronetEngine(closedEngine); + await expectLater(() => client.get(Uri.https('example.com', '/')), + throwsA(isA())); + }); + + test('engine owned close', () { + final engine = CronetEngine.build(); + CronetClient.fromCronetEngine(engine, closeEngine: true).close(); + }); + + test('engine not owned close', () { + final engine = CronetEngine.build(); + CronetClient.fromCronetEngine(engine, closeEngine: false).close(); + engine.close(); + }); + }); +} + void main() { IntegrationTestWidgetsFlutterBinding.ensureInitialized(); testCache(); testInvalidConfigurations(); testUserAgent(); + testEngineClose(); } diff --git a/pkgs/cronet_http/example/lib/main.dart b/pkgs/cronet_http/example/lib/main.dart index 6e1bad22c4..02c56d6581 100644 --- a/pkgs/cronet_http/example/lib/main.dart +++ b/pkgs/cronet_http/example/lib/main.dart @@ -21,7 +21,7 @@ void main() { cacheMode: CacheMode.memory, cacheMaxSize: 2 * 1024 * 1024, userAgent: 'Book Agent'); - httpClient = CronetClient.fromCronetEngine(engine); + httpClient = CronetClient.fromCronetEngine(engine, closeEngine: true); } else { httpClient = IOClient(HttpClient()..userAgent = 'Book Agent'); } diff --git a/pkgs/cronet_http/lib/src/cronet_client.dart b/pkgs/cronet_http/lib/src/cronet_client.dart index 3c2a4264d4..c9573826f3 100644 --- a/pkgs/cronet_http/lib/src/cronet_client.dart +++ b/pkgs/cronet_http/lib/src/cronet_client.dart @@ -49,6 +49,7 @@ enum CacheMode { /// An environment that can be used to make HTTP requests. class CronetEngine { late final jb.CronetEngine _engine; + bool _isClosed = false; CronetEngine._(this._engine); @@ -140,7 +141,12 @@ class CronetEngine { } void close() { - _engine.shutdown(); + if (!_isClosed) { + _engine + ..shutdown() + ..release(); + } + _isClosed = true; } } @@ -275,12 +281,10 @@ class CronetClient extends BaseClient { CronetEngine? _engine; bool _isClosed = false; - /// Indicates that [_engine] was constructed as an implementation detail for - /// this [CronetClient] (i.e. was not provided as a constructor argument) and - /// should be closed when this [CronetClient] is closed. - final bool _ownedEngine; + /// Indicates that [CronetClient] is responsible for closing [_engine]. + final bool _closeEngine; - CronetClient._(this._engine, this._ownedEngine) { + CronetClient._(this._engine, this._closeEngine) { Jni.initDLApi(); } @@ -288,8 +292,13 @@ class CronetClient extends BaseClient { factory CronetClient.defaultCronetEngine() => CronetClient._(null, true); /// A [CronetClient] configured with a [CronetEngine]. - factory CronetClient.fromCronetEngine(CronetEngine engine) => - CronetClient._(engine, false); + /// + /// If [closeEngine] is `true`, then [engine] will be closed when [close] is + /// called on this [CronetClient]. This can simplify lifetime management if + /// [engine] is only used in one [CronetClient]. + factory CronetClient.fromCronetEngine(CronetEngine engine, + {bool closeEngine = false}) => + CronetClient._(engine, closeEngine); /// A [CronetClient] configured with a [Future] containing a [CronetEngine]. /// @@ -309,7 +318,7 @@ class CronetClient extends BaseClient { /// ``` @override void close() { - if (!_isClosed && _ownedEngine) { + if (!_isClosed && _closeEngine) { _engine?.close(); } _isClosed = true; @@ -322,14 +331,19 @@ class CronetClient extends BaseClient { 'HTTP request failed. Client is already closed.', request.url); } - _engine ??= CronetEngine.build(); + final engine = _engine ?? CronetEngine.build(); + _engine = engine; + + if (engine._isClosed) { + throw ClientException( + 'HTTP request failed. CronetEngine is already closed.', request.url); + } final stream = request.finalize(); final body = await stream.toBytes(); final responseCompleter = Completer(); - final engine = _engine!._engine; - final builder = engine.newUrlRequestBuilder( + final builder = engine._engine.newUrlRequestBuilder( request.url.toString().toJString(), jb.UrlRequestCallbackProxy.new1( _urlRequestCallbacks(request, responseCompleter)), diff --git a/pkgs/cronet_http/pubspec.yaml b/pkgs/cronet_http/pubspec.yaml index f1a642bbb3..f3cb350ebe 100644 --- a/pkgs/cronet_http/pubspec.yaml +++ b/pkgs/cronet_http/pubspec.yaml @@ -1,5 +1,5 @@ name: cronet_http -version: 1.1.0 +version: 1.1.1 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