Migrate Essentials v0.0.1: Implement Client with openUrl method #2

unsuitable001 · 2021-06-12T14:54:46Z

Summary

HttpClient with QUIC, HTTP2, brotli support
HttpClient with a customizable user agent string
HttpClient close method (without force close)
open, openUrl & other associated methods.
Response is consumable using dart:io style API.
Different types of Exceptions are implemented.

Notes and Breaking Changes from `dart:io`

Custom SecurityContext is no longer handled by the client. Users have to handle it in other ways. (To be documented later)
userAgent property is now read-only. Custom userAgent should be passed as a constructor argument.
Force close isn't enabled in this PR.

Partial migration from: unsuitable001/dart_cronet_sample

Added LICENSE preamble in files

…onse Partial migration from: unsuitable001/dart_cronet_sample to google/cronet.dart

mannprerak2

Round 1 of review.

.gitignore

.pubignore

CHANGELOG.md

README.md

analysis_options.yaml

lib/src/find_resource.dart

fixed un-nessesary removal of code from .gitignore refined analysis options added changelog

unsuitable001 · 2021-06-13T06:48:08Z

Round 1 of review.

Done. Please check the changes :)

mannprerak2

Round 1.1 of review.

.gitignore

CHANGELOG.md

README.md

lib/src/http_callback_handler.dart

mannprerak2 · 2021-06-13T08:08:31Z

@unsuitable001 You can remove the empty analysis_options.yaml files from your example projects, instead add this in their pubspec.yaml -

dev_dependencies:  
  ...
  lints: ^1.0.1

mannprerak2

Round 2 of review.

analysis_options.yaml

lib/src/generated_bindings.dart

test/http_client_close_test.dart

test/http_responses_exceptions_test.dart

lib/src/http_client_response.dart

behaviour change: If registerCallbacks & close api is used togather, the later called on will resolve with a RequestListenerException.

unsuitable001 · 2021-06-13T16:08:29Z

Round 1.1 of review.
Round 2 of review.

@mannprerak2 Resolved the issues. Please check :) I've also trimmed the flutter example. When you're done reviewing the existing, I'll add the actual one.

…lback's `next` callback is removed. It is now called directly. refactor: docs and cleaned api detection

unsuitable001 · 2021-06-15T05:55:13Z

@dcharkes @mannprerak2 Some changes and small fix for registerCallback api is pushed and added tests for it.

mannprerak2

Round 3 of review.

test/http_responses_exceptions_test.dart

test/http_responses_test.dart

unsuitable001 · 2021-06-15T17:21:45Z

@mannprerak2 Changes done :)

oppsie again. :) forgot to put newline between few as they cross that 80char limit. I'll push that with next review's changes.

test/test_utils.dart

dcharkes

Looking good so far! 👌

Here's my first round of comments.

Maybe we should remove flutter support from this PR and do it in a next one. Then we can focus on getting the dart standalone setup (package-wise, code-layout-wise, CI-wise) right in this PR, and focus on those aspects for Flutter in a follow up CL.

README.md

.pubignore

README.md

lib/src/native/wrapper/wrapper.cc

lib/src/native/wrapper/wrapper.h

lib/src/prepare_cronet.dart

dcharkes · 2021-06-16T08:44:06Z

lib/src/prepare_cronet.dart

+
+/// Download [cronet] library
+/// from Github Releases
+Future<void> downloadCronetBinaries(String platform) async {


Can we get a buildCronetBinaries as well? (Automating the steps you did locally to build the binary.)

That way we can just run buildCronetBinaries when we want to make the binaries for a new version.

Also, that would enable users to build instead of download if they so prefer.

Let's make this build utility to build the wrapper for that custom version and print the link to build cronet in the console. Should we also take the responsibility for building actual cronet binaries? I guess that may turn out to be a little bit hard to maintain. What's your view?

Well, you need to build the libcronet.xxxx.so/dll yourself also right? It would be useful to have the steps to do that as a Dart script, so that you don't forget.

We can either have that script in tool/update_cronet.dart so that you can update the shared libraries with a simple step, or we can have it available in bin/setup.dart as dart run cronet:build-cronet (or something similar).

lib/src/native/include/dart/dart_api_dl.h

unsuitable001 · 2021-06-19T07:18:53Z

@dcharkes Done with this round of changes. 😃

unsuitable001 · 2021-06-27T12:32:37Z

Review of Dart public API and tests. (Internal Dart code and C/C++ code review pending.)

@dcharkes Done with this round of changes. There are few queries in some of the review threads. Waiting for your thoughts on them.

Other than that, there's a change on what we discussed before.
Discussed: Expose addresses of callbacks from wrapper bindings and pass them to cronet for registration.

Change: Exposed few cronet function pointers and passed them to the wrapper.

Reason: The executor (thread) we're using is written in C/C++ and a part of the wrapper. So, we've to expose few cronet apis to make executor work. And, wrapper also handles few memory management tasks (e.g. finalizers etc.) for us. So, I decided to keep the pattern and expose the function for registering callbacks to the wrapper.

dcharkes

Next round of comments

README.md

lib/src/third_party/cronet/ffigen.yaml

lib/src/http_callback_handler.dart

third_party/cronet_impl/sample_executor.h

third_party/cronet_impl/sample_executor.cc

lib/src/http_callback_handler.dart

src/wrapper.cc

third_party/cronet_impl/sample_executor.h

third_party/cronet/cronet.idl_c.h

third_party/cronet_sample/BUILD.md

bin/setup.dart

dcharkes · 2021-07-02T11:16:26Z

README.md

+
+```bash
+dart pub get
+dart run cronet <platform> # Downloads the cronet binaries.


dart run cronet:setup

dcharkes · 2021-07-02T11:42:01Z

src/wrapper.cc

@@ -0,0 +1,201 @@
+// Copyright (c) 2021, the Dart project authors. Please see the AUTHORS file


Nice! I like how this is set up!

dcharkes · 2021-07-02T11:45:00Z

lib/src/http_client_request.dart

+        requestParams, _method.toNativeUtf8().cast<Int8>());
+    wrapper.InitSampleExecutor(_callbackHandler.executor);
+
+    final cronetCallbacks = cronet.Cronet_UrlRequestCallback_CreateWith(


Maybe add a TODO with a reference to the relevant ffigen issue with regard to the casts.

unsuitable001 · 2021-07-02T12:03:56Z

@dcharkes Typos are fixed and ffigen issue reference added. Tell Google to make a AI typo finder for me :p (Autocorrect for programmers)

unsuitable001 added 3 commits June 11, 2021 21:31

adding dependencies, generated_bindings and android support

d96f235

refactor: third_party APIs (cronet) seperated from wrapper.h

4d3bb12

Added LICENSE preamble in files

initial implementation: essential APIs of http client, request & resp…

e3ca8a8

…onse Partial migration from: unsuitable001/dart_cronet_sample to google/cronet.dart

mannprerak2 reviewed Jun 12, 2021

View reviewed changes

refactor: comments & readme

b2253a3

fixed un-nessesary removal of code from .gitignore refined analysis options added changelog

mannprerak2 reviewed Jun 13, 2021

View reviewed changes

.gitignore Show resolved Hide resolved

CHANGELOG.md Outdated Show resolved Hide resolved

README.md Outdated Show resolved Hide resolved

lib/src/http_callback_handler.dart Outdated Show resolved Hide resolved

mannprerak2 reviewed Jun 13, 2021

View reviewed changes

refactor: docs, analysis, license, tests

30cfbe6

behaviour change: If registerCallbacks & close api is used togather, the later called on will resolve with a RequestListenerException.

unsuitable001 added 2 commits June 14, 2021 10:35

api change: registerCallback's future resolves bool and readDataCal…

1ba712d

…lback's `next` callback is removed. It is now called directly. refactor: docs and cleaned api detection

tests & bug fix: http response callback api

a218ac4

mannprerak2 reviewed Jun 15, 2021

View reviewed changes

test/http_responses_exceptions_test.dart Outdated Show resolved Hide resolved

test/http_responses_test.dart Outdated Show resolved Hide resolved

tests: automatic port selection

d99c7c3

mannprerak2 reviewed Jun 15, 2021

View reviewed changes

test/test_utils.dart Outdated Show resolved Hide resolved

tests: add license and enforce 80char

c3da375

dcharkes reviewed Jun 16, 2021

View reviewed changes

unsuitable001 added 6 commits June 17, 2021 20:49

remove: flutter support boilerplate

ea156b1

hint user to get binaries and relocate prepare_cronet

382be13

refactor: native code lib/src/native -> src and 3rd_party -> third_party

82b296a

rewrite build.sh in dart

a667deb

add build_cronet executable for user customization

7df5522

tar -> package:archive and cronet binary detection improvements

9e7836a

info: display command to get cronet library when wrapper can't load it

5b921cf

unsuitable001 mentioned this pull request Jun 19, 2021

Mirror https://github.com/google/cronet.dart/pull/2 unsuitable001/cronet.dart#1

Closed

test: remove unused imports

0303b71

unsuitable001 added 3 commits June 25, 2021 22:06

initial splitting of cronet and wrapper bindings

12499c3

spliting wrapper and cronet bindings

783a6ce

locate binaries to .dart_tool

96636e8

dcharkes reviewed Jun 28, 2021

View reviewed changes

unsuitable001 added 7 commits June 28, 2021 19:25

formatting and sanity checks

d3e4d84

add licence & minor refactors

5958aad

void* -> real type def for cronet fn & test: add regex

7533e4b

setup script, cmake based build & set dylibs as global

f9a1e1e

analyzer: removed unused show from tool/build_wrapper.dart

1d51146

ci: change ci setup command

2f93cd3

CronetNativeException -> Error and enum to string mapping

31de14b

dcharkes reviewed Jun 30, 2021

View reviewed changes

src/wrapper.cc Outdated Show resolved Hide resolved

third_party/cronet_impl/sample_executor.h Show resolved Hide resolved

dcharkes reviewed Jun 30, 2021

View reviewed changes

third_party/cronet/cronet.idl_c.h Outdated Show resolved Hide resolved

dcharkes reviewed Jun 30, 2021

View reviewed changes

third_party/cronet_sample/BUILD.md Outdated Show resolved Hide resolved

dcharkes reviewed Jun 30, 2021

View reviewed changes

bin/setup.dart Outdated Show resolved Hide resolved

unsuitable001 added 3 commits June 30, 2021 23:44

add verify option to cronet:setup, fix setup halting

2ce6957

C API for SampleExecutor

0beef32

tool: update tarballs. setup verify: delete verifier after done.

4452dbe

dcharkes approved these changes Jul 2, 2021

View reviewed changes

unsuitable001 added 2 commits July 2, 2021 17:27

readme: setup command, add: ffigen issue reference

1dcb370

readme: typo fix

c50d7b3

unsuitable001 added 3 commits July 2, 2021 17:38

readme: supported platforms

f222aa4

readme

3de3374

update: binary download url

ec2efb0

dcharkes merged commit c9ef18a into google:main Jul 2, 2021

unsuitable001 deleted the essentials_v0.0.1 branch July 2, 2021 12:36

hadesss123 mentioned this pull request Jan 16, 2022

When I use 32-bit dynamic library, there is an error #36

Open

glanium mentioned this pull request Dec 10, 2022

flutter pub run cronet:setup failed with error #43

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Migrate Essentials v0.0.1: Implement Client with openUrl method #2

Migrate Essentials v0.0.1: Implement Client with openUrl method #2

unsuitable001 commented Jun 12, 2021 •

edited

mannprerak2 left a comment

unsuitable001 commented Jun 13, 2021 •

edited

mannprerak2 left a comment

mannprerak2 commented Jun 13, 2021

mannprerak2 left a comment

unsuitable001 commented Jun 13, 2021 •

edited

unsuitable001 commented Jun 15, 2021

mannprerak2 left a comment

unsuitable001 commented Jun 15, 2021 •

edited

dcharkes left a comment

dcharkes Jun 16, 2021

unsuitable001 Jun 17, 2021

dcharkes Jun 28, 2021

unsuitable001 commented Jun 19, 2021

unsuitable001 commented Jun 27, 2021 •

edited

dcharkes left a comment

dcharkes Jul 2, 2021

dcharkes Jul 2, 2021

dcharkes Jul 2, 2021

unsuitable001 commented Jul 2, 2021 •

edited

		@@ -0,0 +1,201 @@
		// Copyright (c) 2021, the Dart project authors. Please see the AUTHORS file

Migrate Essentials v0.0.1: Implement Client with openUrl method #2

Migrate Essentials v0.0.1: Implement Client with openUrl method #2

Conversation

unsuitable001 commented Jun 12, 2021 • edited

Summary

Notes and Breaking Changes from dart:io

mannprerak2 left a comment

Choose a reason for hiding this comment

unsuitable001 commented Jun 13, 2021 • edited

mannprerak2 left a comment

Choose a reason for hiding this comment

mannprerak2 commented Jun 13, 2021

mannprerak2 left a comment

Choose a reason for hiding this comment

unsuitable001 commented Jun 13, 2021 • edited

unsuitable001 commented Jun 15, 2021

mannprerak2 left a comment

Choose a reason for hiding this comment

unsuitable001 commented Jun 15, 2021 • edited

dcharkes left a comment

Choose a reason for hiding this comment

dcharkes Jun 16, 2021

Choose a reason for hiding this comment

unsuitable001 Jun 17, 2021

Choose a reason for hiding this comment

dcharkes Jun 28, 2021

Choose a reason for hiding this comment

unsuitable001 commented Jun 19, 2021

unsuitable001 commented Jun 27, 2021 • edited

dcharkes left a comment

Choose a reason for hiding this comment

dcharkes Jul 2, 2021

Choose a reason for hiding this comment

dcharkes Jul 2, 2021

Choose a reason for hiding this comment

dcharkes Jul 2, 2021

Choose a reason for hiding this comment

unsuitable001 commented Jul 2, 2021 • edited

unsuitable001 commented Jun 12, 2021 •

edited

Notes and Breaking Changes from `dart:io`

unsuitable001 commented Jun 13, 2021 •

edited

unsuitable001 commented Jun 13, 2021 •

edited

unsuitable001 commented Jun 15, 2021 •

edited

unsuitable001 commented Jun 27, 2021 •

edited

unsuitable001 commented Jul 2, 2021 •

edited