Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Conversation

@jason-simmons
Copy link
Member

@auto-assign auto-assign bot requested a review from gw280 May 8, 2020 21:21
@jason-simmons jason-simmons requested review from xster and removed request for gw280 May 8, 2020 21:21
Copy link
Member

@xster xster left a comment

Choose a reason for hiding this comment

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

Thanks Jason for handling this!


System.loadLibrary("flutter");

return new InitResult(
Copy link
Member

Choose a reason for hiding this comment

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

love that we finally stopped using the resource extractor as a unit of transaction now since it was really confusing. Though this is close to getting us all the way. i.e. upon initialization task complete, initialization isn't quite done yet. We still need to wait for the resource extractor to finish. Perhaps just chain the resource extractor completion inside this async task?

Copy link
Member Author

Choose a reason for hiding this comment

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

done

return;
}
new Thread(
new Runnable() {
Copy link
Member

Choose a reason for hiding this comment

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

should we use an executor here too for consistency?

Copy link
Member Author

Choose a reason for hiding this comment

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

done

result.resourceExtractor.waitForCompletion();
}
new Handler(Looper.getMainLooper())
.post(
Copy link
Member

Choose a reason for hiding this comment

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

this is a comment for line 288. Is FlutterJNI.nativeInit the only line from ensureInitializationComplete that really needs to be run on the main thread? Could we potentially move everything into the init task to maximize the things not running on the main thread?

Copy link
Member Author

Choose a reason for hiding this comment

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

The rest of ensureInitializationComplete is basically doing string manipulation to build the arguments passed to nativeInit. The time consumed is negligible.

@jason-simmons jason-simmons force-pushed the android_strict_mode branch from 9bcf2c1 to 5a05f85 Compare May 11, 2020 22:48
Copy link
Member

@xster xster left a comment

Choose a reason for hiding this comment

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

Thanks Jason

import android.os.StrictMode;
import io.flutter.app.FlutterApplication;

public class ScenarioApplication extends FlutterApplication {
Copy link
Member

Choose a reason for hiding this comment

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

just double checking. Our CI doesn't run this yet so I'm assuming you ran it manually to check.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes - I've run this manually on Firebase and on local devices.

The LUCI script is attempting to run the scenario app test on Firebase (https://flutter.googlesource.com/recipes/+/refs/heads/master/recipes/engine.py#560). But it looks like this is currently broken on the LUCI bots.

@jason-simmons jason-simmons added the waiting for tree to go green This PR is approved and tested, but waiting for the tree to be green to land. label May 13, 2020
@fluttergithubbot fluttergithubbot merged commit dc93db5 into flutter:master May 13, 2020
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 14, 2020
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 14, 2020
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 14, 2020
iskakaushik pushed a commit to flutter/flutter that referenced this pull request May 14, 2020
* 9e3e3ba [web] Represent CSS identity transforms as 'none' instead of null (flutter/engine#18288)

* 80fa77e Roll src/third_party/skia 3ebadcc98eab..056d543c91e0 (8 commits) (flutter/engine#18344)

* 480d8e4 Fix scenario platform view tests on Android (flutter/engine#18332)

* 0385664 Roll src/third_party/dart d6fed1f62444..29c00e28f350 (16 commits) (flutter/engine#18347)

* 8371b44 Roll src/third_party/skia 056d543c91e0..71903997254f (7 commits) (flutter/engine#18350)

* f321613 Add guards on FlValue methods to check for NULL values (flutter/engine#18226)

* dc93db5 Move FlutterLoader disk I/O to a background thread to comply with Android strict mode (flutter/engine#18241)

* bf1287c null-annotate lerp.dart, annotations.dart, channel_buffers.dart, hash_codes.dart (flutter/engine#18348)

* 9600354 Add FlBasicMessageChannel (flutter/engine#18189)

* df2dfac Roll src/third_party/skia 71903997254f..6c3db04c8b03 (9 commits) (flutter/engine#18361)

* 5ad4f9e Add FlJsonMessageCodec (flutter/engine#18221)

* 84ea892 Roll src/third_party/skia 6c3db04c8b03..7156db260239 (4 commits) (flutter/engine#18370)

* f848069 Roll src/third_party/dart 29c00e28f350..f99631b12c4a (29 commits) (flutter/engine#18373)

* c791ce9 Roll src/fuchsia/sdk/mac from gOhJW... to Vepm4... (flutter/engine#18377)

* 5b62a63 Roll src/third_party/dart f99631b12c4a..e0257265d34e (2 commits) (flutter/engine#18378)

* 0b41009 Roll src/third_party/skia 7156db260239..5b2ede3d0d44 (8 commits) (flutter/engine#18380)

* 08b61ce Delete unused decode UTF-8, JSON functions (flutter/engine#18360)

* 73d835c Roll src/third_party/dart e0257265d34e..2676764792b2 (4 commits) (flutter/engine#18383)

* 9e166fb Roll src/third_party/skia 5b2ede3d0d44..39ec60aa8348 (5 commits) (flutter/engine#18384)

* 2cdbc7f Remove pipeline in favor of layer tree holder (flutter/engine#18285)

* 47513a7 Roll src/third_party/skia 39ec60aa8348..79c5674a4ca1 (3 commits) (flutter/engine#18389)
wandyers pushed a commit to wandyers/engine that referenced this pull request May 23, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

cla: yes waiting for tree to go green This PR is approved and tested, but waiting for the tree to be green to land.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

FlutterLoader needs to respect StrictMode non-main thread IO access

4 participants