Multiple flutters sample android #677

gaaclarke · 2021-01-26T22:30:33Z

xster

nice! kotlin looks pretty clean!

add_to_app/multiple_flutters/multiple_flutters_android/README.md

xster · 2021-01-29T21:46:39Z

add_to_app/multiple_flutters/multiple_flutters_android/app/build.gradle

+
+android {
+    signingConfigs {
+        self {


what does this do?

No idea. Android studio added it, maybe when I was trying to solve my signing issue for release?

I think we can remove it

...ple_flutters/multiple_flutters_android/app/src/main/java/dev/flutter/multipleflutters/App.kt

...utters/multiple_flutters_android/app/src/main/java/dev/flutter/multipleflutters/DataModel.kt

xster · 2021-01-29T22:09:13Z

...utters/multiple_flutters_android/app/src/main/java/dev/flutter/multipleflutters/DataModel.kt

+        val instance = DataModel()
+    }
+
+    private val observers =  mutableListOf<WeakReference<DataModelObserver>>()


Same comment as swift. Should this be a set? Since on Android it's not happening on an SDK class where the lifecycle is very clear, alternatively add some state assertions to the consuming class?

It isn't necessary for this example to function correctly. I think a designer of a system for production code would have to make the semantics very clear about what happens when you double subscribe the same object. Another option is just to throw an exception if someone double subscribes.

Either way is ok. I'd just add some comments or docs saying we're taking precautions with the engine bindings or this is a simple demo and you might run into trouble in these circumstances.

...ers/multiple_flutters_android/app/src/main/java/dev/flutter/multipleflutters/MainActivity.kt

xster · 2021-01-29T22:38:10Z

...ers/multiple_flutters_android/app/src/main/java/dev/flutter/multipleflutters/MainActivity.kt

+        DataModel.instance.removeObserver(this)
+    }
+
+    fun onClickNext(view : View) {


add some docs to describe where this would visually come from

...ple_flutters_android/app/src/main/java/dev/flutter/multipleflutters/SingleFlutterActivity.kt

...to_app/multiple_flutters/multiple_flutters_android/app/src/main/res/layout/activity_main.xml

xster · 2021-02-02T20:10:42Z

Didn't go through the complete code again yet. I think my only remaining comment was the MainActivity name. Otherwise I think this is ready to move out of draft after your folder move.

gaaclarke · 2021-02-08T22:08:26Z

@xster Do you know how the CI works? Looks like the version of Flutter that the tests are being run against don't have access to the new API. It's choking on "FlutterEngineGroup".

edit: Looks like the intent is that things are in experimental if they use API's that haven't landed on stable.

xster · 2021-02-08T22:12:33Z

Oh wait, right. @RedBrogdon I'm somewhat surprised the iOS PR passed the CI.
The idea was that the samples must be buildable using the latest stable branch unless the samples are in the experimental folder. But I'm not sure why that test didn't fail with the iOS PR (which is using FlutterEngineGroup, not in 1.22).

As a solution, I think it'd be ok if we waited until next month to click the merge button once the stable is pushed. @RedBrogdon I don't know if there's otherwise a cheaper solution than moving everything to experimental then moving back. i.e. some flag we can maybe set in the test file?

RedBrogdon · 2021-02-09T02:25:28Z

Looks like the intent is that things are in experimental if they use API's that haven't landed on stable.

That's correct. One of the invariants, so to speak, of the repo is that every root-level project should build/test with the stable channel, and everything that can't should be in /experimental.

Oh wait, right. @RedBrogdon I'm somewhat surprised the iOS PR passed the CI.

Y'all just uncovered a bug in our CI setup. Looks like xcpretty is swallowing error codes returned by xcodebuild. We should be configuring it to return the error code instead:

https://www.rubydoc.info/gems/xcpretty/0.2.8#usage

I'll create a separate PR to fix it.

As a solution, I think it'd be ok if we waited until next month to click the merge button once the stable is pushed. @RedBrogdon I don't know if there's otherwise a cheaper solution than moving everything to experimental then moving back. i.e. some flag we can maybe set in the test file?

You can certainly leave a PR open, since it's unlikely to run into merge conflicts. That said, the iOS version of the app has landed, and I can't fix the hole you just identified in our CI without dealing with that fact. 😄

That leaves a few options:

Move multiple_flutters to experimental and land the Android version there as well.
Remove multiple_flutters entirely, add the iOS and Flutter versions to this PR (unwieldy, but doable), and leave this PR open for now.
Leave multiple_flutters in place, remove it from the Flutter and iOS CI project lists, and leave this PR open for now.
Remove multiple_flutters, add it to this PR, then land this PR in the beta branch of this repo, which is where we're getting samples for the next release ready.

WDYT? I hadn't realized y'all were targeting anything other than stable, or I would have mentioned this earlier.

gaaclarke · 2021-02-09T18:06:14Z

I don't have a good idea about when the API is hitting stable, but a 5th option is that we pause the train until the API hits stable. That means this doesn't land and your iOS change doesn't land. That's the least amount of work for everyone but might not be ideal depending on the timeline for the next stable release. Looks like the last one was 1/25/21 so it should't be long now I imagine.

xster · 2021-02-11T21:47:42Z

Waiting to merge sounds ok to me. That would be the simplest thing to do.

domesticmouse · 2021-03-08T03:26:39Z

I'd suggest pull from master to fix the Master Branch CI stable channel failures

RedBrogdon

Looks like Flutter tests are failing because they're pulling in older master code that's not touched by this PR. I'm going to call that a quirk of how GHA works and not a problem with this code, and then do the merge anyway.

xster · 2021-03-09T17:12:35Z

...ple_flutters_android/app/src/main/java/dev/flutter/multipleflutters/DoubleFlutterActivity.kt

+                1.0f
+            )
+            val engine = if (i == 0) topBindings.engine else bottomBindings.engine
+            FlutterEngineCache.getInstance().put(i.toString(), engine)


#762 is probably this. The i needs to account for the number of occurrences of DoubleFlutterActivity too. Otherwise, the engine cache index 0 and 1 get overwritten over and over again (and can't pop back).

saurabhchalke · 2021-03-11T14:01:48Z

@gaaclarke Is there a way to start with a Flutter screen at the beginning and bypassing any Kotlin screens at all?
I am trying to tweak this example with Flutter -> Native -> Flutter flow.

Also, the Kotlin to Flutter navigation seems to be slow on my device (Mi A2, Android 10), although Flutter to Kotlin seems to be quick.
Do you see any optimizations possible in here?

Raised issue #765 to discuss this.

gaaclarke · 2021-03-11T18:10:07Z

I Is there a way to start with a Flutter screen at the beginning and bypassing any Kotlin screens at all?

Yep, I don't see any reason why it shouldn't work.

Also, the Kotlin to Flutter navigation seems to be slow on my device (Mi A2, Android 10), although Flutter to Kotlin seems to be quick.
Do you see any optimizations possible in here?

It is slow on debug builds and on emulators, on release builds on real devices it is fast. If you found a scenario where the transition isn't fast on release builds / real devices, please file an issue with the supporting evidence and reproduction steps.

@johnshajiang

Bumps [actions/setup-java](https://github.com/actions/setup-java) from 4.2.2 to 4.3.0. <details> <summary>Release notes</summary> Sourced from <a href="https://github.com/actions/setup-java/releases">actions/setup-java's releases</a>. <blockquote> <h2>v4.3.0</h2> What's Changed <ul> <li>Add support for SapMachine JDK/JRE by <a href="https://github.com/Shegox"><code>@â��Shegox</code></a> in <a href="https://redirect.github.com/actions/setup-java/issues/614">#614</a></li> </ul> <pre lang="yaml"><code>steps: - name: Checkout uses: actions/checkout@v4 - name: Setup-java uses: actions/setup-java@v4 with: distribution: â��sapmachineâ�� java-version: â��21â�� </code></pre> Bug fixes : <ul> <li> <pre><code>Fix typos on Corretto by @johnshajiang in [#666](actions/setup-java#666) </code></pre> </li> <li> <pre><code>IBM Semeru Enhancement on arm64 byÂ @mahabaleshwars in [#677](actions/setup-java#677) </code></pre> </li> <li> <pre><code>Resolve Basic Validation Check Failures byÂ @aparnajyothi-yâ�¨Â in [#682](actions/setup-java#682) </code></pre> </li> </ul> New Contributors : <ul> <li> <pre><code>@johnshajiang made their first contribution in [#666](actions/setup-java#666) </code></pre> </li> <li> <pre><code>@Shegox made their first contribution in [#614](actions/setup-java#614) </code></pre> </li> </ul> Full Changelog: <a href="https://github.com/actions/setup-java/compare/v4...v4.3.0">https://github.com/actions/setup-java/compare/v4...v4.3.0</a> </blockquote> </details> <details> <summary>Commits</summary> <ul> <li><a href="https://github.com/actions/setup-java/commit/2dfa2011c5b2a0f1489bf9e433881c92c1631f88"><code>2dfa201</code></a> basic validation failure fix (<a href="https://redirect.github.com/actions/setup-java/issues/682">#682</a>)</li> <li><a href="https://github.com/actions/setup-java/commit/7467385c615a13cecd14d5768e738332968d0792"><code>7467385</code></a> feat: add support for SapMachine JDK/JRE (<a href="https://redirect.github.com/actions/setup-java/issues/614">#614</a>)</li> <li><a href="https://github.com/actions/setup-java/commit/8e04ddff28554375a9a1096c888a2ef2c9803cd7"><code>8e04ddf</code></a> Update Error Messages and Fix Architecture Detection for IBM Semeru (<a href="https://redirect.github.com/actions/setup-java/issues/677">#677</a>)</li> <li><a href="https://github.com/actions/setup-java/commit/67fbd726daaf08212a7b021c1c4d117f94a81dd3"><code>67fbd72</code></a> Fix typos on Corretto (<a href="https://redirect.github.com/actions/setup-java/issues/665">#665</a>) (<a href="https://redirect.github.com/actions/setup-java/issues/666">#666</a>)</li> <li>See full diff in <a href="https://github.com/actions/setup-java/compare/6a0805fcefea3d4657a47ac4c165951e33482018...2dfa2011c5b2a0f1489bf9e433881c92c1631f88">compare view</a></li> </ul> </details> [![Dependabot compatibility score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=actions/setup-java&package-manager=github_actions&previous-version=4.2.2&new-version=4.3.0)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. --- <details> <summary>Dependabot commands and options</summary> You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot merge` will merge this PR after your CI passes on it - `@dependabot squash and merge` will squash and merge this PR after your CI passes on it - `@dependabot cancel merge` will cancel a previously requested merge and block automerging - `@dependabot reopen` will reopen this PR if it is closed - `@dependabot close` will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually - `@dependabot show <dependency name> ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself) </details>

gaaclarke force-pushed the multiple-flutters-sample-android branch 8 times, most recently from 24853ca to ef975b7 Compare January 28, 2021 21:53

xster reviewed Jan 29, 2021

View reviewed changes

gaaclarke force-pushed the multiple-flutters-sample-android branch 13 times, most recently from 1a2dcd0 to 25f82c1 Compare February 2, 2021 00:51

gaaclarke force-pushed the multiple-flutters-sample-android branch 2 times, most recently from 9339cd3 to cf6b063 Compare February 8, 2021 21:50

gaaclarke marked this pull request as ready for review February 8, 2021 22:07

RedBrogdon mentioned this pull request Feb 9, 2021

Unmasking CI error codes from iOS #710

Merged

gaaclarke mentioned this pull request Feb 23, 2021

Multiple Flutter Engine: destroy flutter engine making live flutter stop response user interaction flutter/flutter#76597

Closed

gaaclarke force-pushed the multiple-flutters-sample-android branch from cf6b063 to 62a9f19 Compare February 23, 2021 21:21

gaaclarke requested a review from RedBrogdon March 3, 2021 18:09

gaaclarke added 2 commits March 3, 2021 10:10

Added sample for multiple flutters for android

3aeb60e

added missing FlutterEngine.destroy as a comment

be28a34

gaaclarke force-pushed the multiple-flutters-sample-android branch from 3b7dece to be28a34 Compare March 3, 2021 18:11

google-cla bot added the cla: yes Contributor license agreement signed by all authors label Mar 3, 2021

xster mentioned this pull request Mar 5, 2021

multiple_flutters_android not found #755

Closed

RedBrogdon approved these changes Mar 8, 2021

View reviewed changes

RedBrogdon merged commit cf913f6 into flutter:master Mar 8, 2021

xster reviewed Mar 9, 2021

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Multiple flutters sample android #677

Multiple flutters sample android #677

gaaclarke commented Jan 26, 2021

xster left a comment

xster Jan 29, 2021

gaaclarke Feb 1, 2021

xster Feb 2, 2021

xster Jan 29, 2021

gaaclarke Feb 1, 2021

xster Feb 2, 2021

xster Jan 29, 2021

xster commented Feb 2, 2021

gaaclarke commented Feb 8, 2021 •

edited

Loading

xster commented Feb 8, 2021

RedBrogdon commented Feb 9, 2021

gaaclarke commented Feb 9, 2021

xster commented Feb 11, 2021

domesticmouse commented Mar 8, 2021

RedBrogdon left a comment

xster Mar 9, 2021

saurabhchalke commented Mar 11, 2021 •

edited

Loading

gaaclarke commented Mar 11, 2021

Multiple flutters sample android #677

Multiple flutters sample android #677

Conversation

gaaclarke commented Jan 26, 2021

xster 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

xster commented Feb 2, 2021

gaaclarke commented Feb 8, 2021 • edited Loading

xster commented Feb 8, 2021

RedBrogdon commented Feb 9, 2021

gaaclarke commented Feb 9, 2021

xster commented Feb 11, 2021

domesticmouse commented Mar 8, 2021

RedBrogdon left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

saurabhchalke commented Mar 11, 2021 • edited Loading

gaaclarke commented Mar 11, 2021

gaaclarke commented Feb 8, 2021 •

edited

Loading

saurabhchalke commented Mar 11, 2021 •

edited

Loading