Flutter support #18

unsuitable001 · 2021-07-27T06:44:21Z

Add Flutter Support, covering all of the native platforms.

Platforms -

Android
Linux
Windows

MacOS & iOS will be done separately.

This PR only takes care of 64bit systems. For 32 bit systems, issue #20 should be tracked.

Reference #5

unsuitable001 · 2021-07-28T07:20:16Z

@dcharkes Remind me after you upload android binaries. I need to revert download urls.

@dcharkes @mannprerak2 Flutter support for Android, Linux and Windows is available now. I'm excluding flutter example from dart analyze as it will complain about package:flutter/material.dart.

But, we will not be able to publish this to pub.dev as I haven't mentioned the flutter version in pubspec. If I add flutter version, we will fail to do dart pub get. Only flutter pub get will work, limiting the use for non-flutter environments.

Possible approaches -

webcrypto.dart way - Add Flutter version to pubspec and use it with flutter (at least for now).
objectbox-dart way - Separate the platform specific specific code in another standalone package (with no dart code) and publish both the main package and the Flutter support package. Make the user depend on both the packages if Flutter support is required.
My Hack 😅 - Keep the platform specific code inside a new directory that doesn't get special treatment form pub publish (cheating the system :p ). Then, add a flag/option/command in the cronet:setup script to enable flutter support which will move our platform specific code into the proper location so flutter build system can use it.

Reference: dart-lang/pub#2606

unsuitable001 · 2021-07-28T09:33:31Z

To add MacOS support, maybe we can try doing this -

cd example/flutter
flutter create --platforms=macos --project-name=cronet_example .

And, this (should) meet our target 😃.

Tell me if it works.

bin/setup.dart

tool/update_tars.dart

mannprerak2 · 2021-07-29T03:34:48Z

In my opinion, the webcrypto approach is more sensible, simply because most people would be using this with flutter.

unsuitable001 · 2021-07-29T11:58:38Z

In my opinion, the webcrypto approach is more sensible, simply because most people would be using this with flutter.

@mannprerak2 I'll miss dart cli then 😢 . Hope pub.dev fixes it soon. Other changes are done too :)

pubspec.yaml

dcharkes

Just for my reference (so that I know which files to review): Which files were generated by flutter create and were not modified? And which files where either modified or not generated by flutter create?

README.md

dcharkes · 2021-08-02T10:04:20Z

android/src/main/kotlin/dev/google/cronet/CronetPlugin.kt

+  private lateinit var channel : MethodChannel
+
+  override fun onAttachedToEngine(@NonNull flutterPluginBinding: FlutterPlugin.FlutterPluginBinding) {
+    System.loadLibrary("cronet.86.0.4240.198")


Why are we loading these in Kotlin rather than using DynamicLibrary.open?

If we do not load it using Kotlin first, we get this error -

F/chromium(17045): [0418/210141.455792:FATAL:jni_android.cc(96)] Check failed: g_jvm.

Though we're not using Platform Channel from our side in our package itself. Loading the library via Kotlin first then doing DynamicLibrary.open on Dart side fixes the issue. (I'm yet to figure out why this is happening though.)

Reference: unsuitable001/dart_cronet_sample#3

It sounds similar to this: flutter/flutter#73318

Lets just keep it for now and file an issue to look at it later.

dcharkes · 2021-08-02T10:06:06Z

example/flutter/README.md

@@ -0,0 +1,16 @@
+# cronet_example


Replace this boilerplate readme with a description of what the Flutter example is doing.

I'll delete the cli example as we're going flutter only for now :). So, I'll delete this readme file also and pub will index the actual code under Example tab by it's own. :)

Why only Flutter? The Dart CLI things just keep working if you have a dart on your path correct?

When I try to do dart run, I get -

Resolving dependencies... Because example_dart depends on cronet from path which requires the Flutter SDK, version solving failed. Flutter users should run `flutter pub get` instead of `pub get`.

Oops. It does work.
If I do - dart run bin/example_dart.dart it works.
But, if I do dart run, it gives the error I mentioned above. Keep the CLI example then. :)

dcharkes · 2021-08-02T10:20:59Z

Closes #5

Only closes #5 if there is a separate issue to track iOS/MacOS Flutter support.

README.md

dcharkes · 2021-08-02T10:35:07Z

Can we add some flutter test tests as well just to make sure stuff works.

See https://github.com/google/webcrypto.dart repo.

unsuitable001 · 2021-08-02T11:25:31Z

Just for my reference (so that I know which files to review): Which files were generated by flutter create and were not modified? And which files where either modified or not generated by flutter create?

@dcharkes

Generated but modified -

android/src/main/kotlin/dev/google/cronet/CronetPlugin.kt
android/src/main/AndroidManifest.xml
example/flutter/android/app/src/main/AndroidManifest.xml

Modified (not generated) -

bin/setup.dart
example/README.md
example/flutter/lib/main.dart
lib/src/constants.dart            # I need to change this after you upload the android binaries.
lib/src/third_party/ffigen/find_resource.dart
pubspec.yaml
src/CMakeLists.txt
tool/update_tars.dart

Anything not listed here is generated boilerplate.

dcharkes · 2021-08-02T10:44:53Z

README.md

@@ -6,13 +6,14 @@ This is a [GSoC 2021 project](https://summerofcode.withgoogle.com/projects/#4757

 ## Supported Platforms

-Currently, 64 bit Desktop Platforms (Linux, Windows and MacOS) are supported.
+Currently, Android and 64 bit Desktop Platforms (Linux, Windows and MacOS) are supported.


What about x86 Android when using the Android Emulator from Android studio?

Those binaries are added too, both x86 and x86_64. (I'll try to test on the emulator).

Not working on x86 emulator :( Getting HttpException with some negative long random value as error status code. Maybe we should mention this and probably need to do some modifications in the wrapper to support 32 bit systems overall.

Please file an issue to track this.

unsuitable001 · 2021-08-02T13:32:05Z

Can we add some flutter test tests as well just to make sure stuff works.

See https://github.com/google/webcrypto.dart repo.

Tested using flutter test, everything passing. But, is there any official Github Action for flutter like dart-lang/setup-dart? If not, is there any other 3rd party action is recommend to use?

https://github.com/marketplace/actions/flutter-action This one is the most popular (if we're considering 3rd party Github Actions)

unsuitable001 · 2021-08-04T06:02:15Z

@dcharkes All done.
Checks on Windows (stable) failing due to -

LINK : fatal error LNK1181: cannot open input file 'DART_LIB-NOTFOUND.lib' [D:\a\cronet.dart\cronet.dart\src\out\windows\wrapper.vcxproj]

That implies, C:/tools/dart-sdk/bin/dart.lib is not available in that particular vm.
But, surprisingly, Windows (dev) passes.

Is there any known issue in flutter regarding this?

dcharkes · 2021-08-04T09:32:00Z

@dcharkes All done.
Checks on Windows (stable) failing due to -
LINK : fatal error LNK1181: cannot open input file 'DART_LIB-NOTFOUND.lib' [D:\a\cronet.dart\cronet.dart\src\out\windows\wrapper.vcxproj]
That implies, C:/tools/dart-sdk/bin/dart.lib is not available in that particular vm.
But, surprisingly, Windows (dev) passes.

Is there any known issue in flutter regarding this?

Why do we need to link against dart.lib?

We're using the _DL symbols in wrapper.cc, so we should not have to link against Dart/Flutter at all when building Cronet or the wrapper.

unsuitable001 · 2021-08-04T09:58:38Z

Why do we need to link against dart.lib?

We're using the _DL symbols in wrapper.cc, so we should not have to link against Dart/Flutter at all when building Cronet or the wrapper.

Oops 😅 It was already removed for Linux/Mac. Removed it. CI passed 😄

dcharkes · 2021-08-04T10:42:25Z

android/src/main/kotlin/dev/google/cronet/CronetPlugin.kt

+  private lateinit var channel : MethodChannel
+
+  override fun onAttachedToEngine(@NonNull flutterPluginBinding: FlutterPlugin.FlutterPluginBinding) {
+    System.loadLibrary("cronet.86.0.4240.198")


It sounds similar to this: flutter/flutter#73318

Lets just keep it for now and file an issue to look at it later.

dcharkes · 2021-08-04T10:44:12Z

example/flutter/android/app/src/debug/AndroidManifest.xml

@@ -0,0 +1,7 @@
+<manifest xmlns:android="http://schemas.android.com/apk/res/android"


Can you leave comments on what lines of the file were added/modified from the generated file? That way it will be easier to deal with conflicts etc. if the Flutter project needs to be updated in the future.

Oops. I mentioned this file in the above comment. It isn't modified by me. But, other paths are correct. I did a copy paste mistake with android/src/main/AndroidManifest.xml.

Anyways, I added it to the correct file :)

dcharkes · 2021-08-04T10:44:16Z

example/flutter/android/app/src/main/AndroidManifest.xml

@@ -0,0 +1,44 @@
+<manifest xmlns:android="http://schemas.android.com/apk/res/android"


Can you leave comments on what lines of the file were added/modified from the generated file? That way it will be easier to deal with conflicts etc. if the Flutter project needs to be updated in the future.

android/src/main/AndroidManifest.xml

unsuitable001 · 2021-08-04T11:10:53Z

android/src/main/AndroidManifest.xml

@@ -0,0 +1,6 @@
+<manifest xmlns:android="http://schemas.android.com/apk/res/android"
+  package="dev.google.cronet">
+  <!-- Flutter Boilerplate Modifications: Internet & Netwrok Permissions-->


This is the another modified file that I mis-"copy pasted" with another manifest file's path.

dcharkes · 2021-08-04T11:16:41Z

android/src/main/AndroidManifest.xml

@@ -1,6 +1,6 @@
 <manifest xmlns:android="http://schemas.android.com/apk/res/android"
  package="dev.google.cronet">
-  <!-- Flutter Boilerplate Modifications: Internet & Netwrok Permissions-->
+  <!-- Flutter Boilerplate Modifications: Internet & netwrok Permissions-->


netwrok -> network (it was the spelling I meant, not the capitalization 🙈 )

Oops 😓 my bad.

Update the binaries and I'll change the download url and commit both the changes together 🙈. 1 less task for our CI vm.

What do we need the jars for in the Android binaries?

We need those jar files too :) Again, I need to refer this issue: unsuitable001/dart_cronet_sample#3

I don't know the exact reason. :)

I'm re-testing it on my device and I'll paste the exact error log here.

Tried in 3 variation. Logs below.

Attempt 1: Include jar files but do not load the .so files from kotlin.
Output: Crashing as soon as it's getting launch. Only getting

http://127.0.0.1:34017/YTBP04h-Lu4=/

Attempt 2: Remove jars but load so files from kotlin

✓ Built build/app/outputs/flutter-apk/app-debug.apk. Installing build/app/outputs/flutter-apk/app.apk... 8.9s F/chromium(21501): [0804/174650.092879:FATAL:jni_android.cc(138)] Failed to find class J/N F/chromium(21501): #00 pc 0x00000000002e09bf /data/app/dev.google.cronet_example-ZSU7uPvW02OfcK8Ob4tYpQ==/lib/arm64/libcronet.86.0.4240.198.so F/chromium(21501): #01 pc 0x00000000002eb60f /data/app/dev.google.cronet_example-ZSU7uPvW02OfcK8Ob4tYpQ==/lib/arm64/libcronet.86.0.4240.198.so F/chromium(21501): #02 pc 0x00000000003418af /data/app/dev.google.cronet_example-ZSU7uPvW02OfcK8Ob4tYpQ==/lib/arm64/libcronet.86.0.4240.198.so F/chromium(21501): #03 pc 0x00000000001e1aa3 /data/app/dev.google.cronet_example-ZSU7uPvW02OfcK8Ob4tYpQ==/lib/arm64/libcronet.86.0.4240.198.so F/chromium(21501): #04 pc 0x00000000002c1a3f /system/lib64/libart.so F/chromium(21501): #05 pc 0x000000000000403b /system/lib64/libopenjdkjvm.so F/chromium(21501): #06 pc 0x0000000000000bcb /system/framework/arm64/boot-core-oj.oat F/chromium(21501): Error connecting to the service protocol: failed to connect to http://127.0.0.1:37607/pQcwEtK-emc=/

Attempt 3: Remove jars and do not load .so files from kotlin

✓ Built build/app/outputs/flutter-apk/app-debug.apk. Installing build/app/outputs/flutter-apk/app.apk... 7.4s I/OpenGLRenderer(21802): Davey! duration=906ms; Flags=1, IntendedVsync=29271908984223, Vsync=29272625650861, OldestInputEvent=9223372036854775807, NewestInputEvent=0, HandleInputStart=29272634604821, AnimationStart=29272634798155, PerformTraversalsStart=29272634807217, DrawStart=29272653090655, SyncQueued=29272657276280, SyncStart=29272657438155, IssueDrawCommandsStart=29272657732217, SwapBuffers=29272812535446, FrameCompleted=29272815417269, DequeueBufferDuration=11718000, QueueBufferDuration=765000, F/libc (21802): Fatal signal 11 (SIGSEGV), code 1 (SEGV_MAPERR), fault addr 0x0 in tid 21827 (1.ui), pid 21802 (.cronet_example) *** *** *** *** *** *** *** *** *** *** *** *** *** *** *** *** Build fingerprint: 'asus/WW_X00TD/ASUS_X00T_2:9/PKQ1/16.2017.2009.087-20200826:user/release-keys' Revision: '0' ABI: 'arm64' pid: 21802, tid: 21827, name: 1.ui >>> dev.google.cronet_example <<< signal 11 (SIGSEGV), code 1 (SEGV_MAPERR), fault addr 0x0 Cause: null pointer dereference x0 0000000000000000 x1 00000078b2bfac78 x2 0000000000010002 x3 00000078a7281588 x4 00000078a7281500 x5 0000007950719f88 x6 00000078a72814f0 x7 0000000000000000 x8 0f5c9418fa297d4e x9 0f5c9418fa297d4e x10 0000000000000001 x11 0000000000000000 x12 00000078a7281500 x13 00000078a72814f0 x14 0000000000004100 x15 aaaaaaaaaaaaaaab x16 00000078a7a6ced0 x17 000000795071b040 x18 0000000000000001 x19 00000078a7a74000 x20 0000000000000002 x21 00000078b2bfc588 x22 00000078b1108041 x23 00000078b4755b90 x24 00000078b1108041 x25 00000078b2b1e000 x26 00000078b2bfc588 x27 00000078a7387430 x28 0000000000000004 x29 00000078b2bfaca0 sp 00000078b2bfac50 lr 00000078a7717ca8 pc 00000078a78773a4 backtrace: #00 pc 00000000003413a4 /data/app/dev.google.cronet_example-1jJaFkoD6WqfgexC4ieOzA==/lib/arm64/libcronet.86.0.4240.198.so #01 pc 00000000001e1ca4 /data/app/dev.google.cronet_example-1jJaFkoD6WqfgexC4ieOzA==/lib/arm64/libcronet.86.0.4240.198.so #02 pc 00000000002baba8 /data/app/dev.google.cronet_example-1jJaFkoD6WqfgexC4ieOzA==/lib/arm64/libcronet.86.0.4240.198.so #03 pc 0000000000006694 <anonymous:00000078b1080000> Syncing files to device ASUS X00TD... ⣾ Lost connection to device. Syncing files to device ASUS X00TD... 702ms Oops; flutter has exited unexpectedly: "getIsolate: (112) Service has disappeared". ⣻Oops; flutter has exited unexpectedly: "getIsolate: (112) Service has disappeared".

Ah of course, we're compiling that Kotlin helper code to a jar. Silly me. :)

https://github.com/google/cronet.dart/releases/tag/binaries-v0.0.4

@dcharkes Links are updated!

unsuitable001 added 7 commits July 27, 2021 08:59

flutter support: initial commit

cf87c48

implement: android support

11e8c11

CI: exclude flutter sample from dart analyze

d0ea739

flutter: add linux support

1b6a8a2

update tools/setup for android

6094162

flutter: add windows support

bafdd35

update readme for android support

bd49741

unsuitable001 requested review from dcharkes and mannprerak2 July 28, 2021 07:20

mannprerak2 reviewed Jul 29, 2021

View reviewed changes

bin/setup.dart Outdated Show resolved Hide resolved

bin/setup.dart Outdated Show resolved Hide resolved

tool/update_tars.dart Outdated Show resolved Hide resolved

unsuitable001 added 2 commits July 29, 2021 17:16

str interpolate -> package:path join & adding constants

875cc3a

remove circular ref in map

a631548

unsuitable001 requested a review from mannprerak2 July 29, 2021 11:57

make androidPath final

0488e0e

mannprerak2 reviewed Aug 1, 2021

View reviewed changes

pubspec.yaml Show resolved Hide resolved

add flutter platform keys for desktops

2481e54

unsuitable001 requested a review from mannprerak2 August 1, 2021 09:07

dcharkes reviewed Aug 2, 2021

View reviewed changes

README.md Outdated Show resolved Hide resolved

dcharkes reviewed Aug 2, 2021

View reviewed changes

unsuitable001 added 2 commits August 2, 2021 19:38

readme fixes and CI from Dart -> Flutter

e0bdfb8

update ci

3870e67

unsuitable001 added 3 commits August 4, 2021 10:57

remove boilerplate widget_test

9fa8327

ci check success on each step

edb24bd

ci: remove successs check at wrong place

59da4ee

change exit code in case of build failure

3adf06f

remove linking with dart.lib (windows) and int -> size_t for strlen

3a4c1d7

unsuitable001 requested a review from dcharkes August 4, 2021 10:00

dcharkes approved these changes Aug 4, 2021

View reviewed changes

add flutter create boilerplate modification notes

3dbaf29

dcharkes reviewed Aug 4, 2021

View reviewed changes

android/src/main/AndroidManifest.xml Outdated Show resolved Hide resolved

unsuitable001 commented Aug 4, 2021

View reviewed changes

Network -> network

d26ed08

dcharkes reviewed Aug 4, 2021

View reviewed changes

unsuitable001 added 2 commits August 4, 2021 16:50

yet another typo 😓

5c58ed6

change download url

4cdcc6c

dcharkes merged commit c2fb111 into google:main Aug 4, 2021

unsuitable001 deleted the flutter_support branch August 4, 2021 14:21

		@@ -0,0 +1,7 @@
		<manifest xmlns:android="http://schemas.android.com/apk/res/android"

		@@ -0,0 +1,44 @@
		<manifest xmlns:android="http://schemas.android.com/apk/res/android"

Flutter support #18

Flutter support #18

Conversation

unsuitable001 commented Jul 27, 2021 • edited Loading

unsuitable001 commented Jul 28, 2021

unsuitable001 commented Jul 28, 2021

mannprerak2 commented Jul 29, 2021

unsuitable001 commented Jul 29, 2021 • edited Loading

dcharkes left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dcharkes commented Aug 2, 2021

dcharkes commented Aug 2, 2021

unsuitable001 commented Aug 2, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

unsuitable001 Aug 4, 2021 • edited Loading

Choose a reason for hiding this comment

unsuitable001 commented Aug 2, 2021 • edited Loading

unsuitable001 commented Aug 4, 2021

dcharkes commented Aug 4, 2021

unsuitable001 commented Aug 4, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

unsuitable001 Aug 4, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

unsuitable001 commented Jul 27, 2021 •

edited

Loading

unsuitable001 commented Jul 29, 2021 •

edited

Loading

unsuitable001 commented Aug 2, 2021 •

edited

Loading

unsuitable001 Aug 4, 2021 •

edited

Loading

unsuitable001 commented Aug 2, 2021 •

edited

Loading

unsuitable001 Aug 4, 2021 •

edited

Loading