Pass the filename directly to JNI for loading deferred component. #23824

chingjun · 2021-01-21T05:56:57Z

When .so files are in the lib/ path in the APK, it can be dlopen-ed
directly using just the filename. We don't need to search for the file.
The interface has thus been changed to accept a single path instead of a
search directory.

Also instead of hardcoding the .so basename and assets directory, read
them from FlutterApplicationInfo instead.

Pre-launch Checklist

I read the [Contributor Guide] and followed the process outlined there for submitting PRs.
I read the [Tree Hygiene] wiki page, which explains my responsibilities.
I read and followed the [Flutter Style Guide] and the [C++, Objective-C, Java style guides].
I listed at least one issue that this PR fixes in the description above.
I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test exempt. See [testing the engine] for instructions on
writing and running engine tests.
I updated/added relevant documentation (doc comments with ///).
I signed the [CLA].
All existing and new tests are passing.
The reviewer has submitted any presubmit flakes in this PR using the [engine presubmit flakes form] before re-triggering the failure.

When .so files are in the lib/ path in the APK, it can be dlopen-ed directly using just the filename. We don't need to search for the file. The interface has thus been changed to accept a single path instead of a search directory. Also instead of hardcoding the .so basename and assets directory, read them from FlutterApplicationInfo instead.

chingjun · 2021-01-21T06:03:12Z

Another note, in the .aab file that @GaryQian sent to me, the .so file in the base module is called libapp.so but the one in the feature module is called app.so-2.part.so.

The change is this PR will be looking for libapp.so-2.part.so instead, which makes it incompatible with whatever code you currently have. I believe the tooling code is not submitted yet, so it's not breaking anyone yet. Please correct me if I am wrong, I can update the tooling to keep the filenames consistent.

GaryQian · 2021-01-21T08:17:28Z

Changing to libapp.so-2.part.so is no problem. I'll give this a more thorough look tomorrow.

GaryQian

LGTM, thanks!

GaryQian · 2021-01-21T21:11:42Z

shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java

-   *     when a library is sucessfully found. When the found library is invalid, no additional paths
-   *     will be attempted.
+   * @param sharedLibraryName File name of the .so file to be loaded, or if the file is not already
+   *     in LD_LIBRARY_PATH, the full path to the file. Note: .so files in the lib/<abi> directory


Remove Note. Also, may want to replicate this info about lib/ in the DeferredComponentManager docs as well.

Updated, and added this to the DeferredComponentManager docs. Thanks for the review!

…ent. (flutter/engine#23824)

* 5b2defd Roll Fuchsia Mac SDK from MrtHftV0U... to 7nZajqutF... (flutter/engine#23827) * c8ffb20 Roll Skia from 87a055b02027 to e0d023562bd9 (1 revision) (flutter/engine#23829) * 296902b implemented GetMainContext() for opengl (flutter/engine#23634) * 5cf3eae Roll Skia from e0d023562bd9 to 982127b7d57d (4 revisions) (flutter/engine#23831) * fd9a079 [iOS] Fixes DisplayLinkManager leaks (flutter/engine#22194) * b241537 Roll Fuchsia Linux Toolchain from git_revision:2c0536b76b35fa592ac7b4a0e4bb176eaf55af75 to IJxh_9dNS... (flutter/engine#23832) * 5c2003f Roll Skia from 982127b7d57d to 6de1e52d0b12 (1 revision) (flutter/engine#23834) * 5b9cd44 Automatically download Noto fonts as backup fonts in CanvasKit mode (flutter/engine#23728) * 70a6824 Roll Dart SDK from 5e24a66b1bb8 to 704928da5702 (2 revisions) (flutter/engine#23838) * b0c46d8 Roll Skia from 6de1e52d0b12 to 8a37fb2c605b (5 revisions) (flutter/engine#23836) * d4132ea Use references when iterating over SkParagraph text boxes (flutter/engine#23837) * 87960d8 Fix typo in embedder unit tests (flutter/engine#23783) * 7f66714 iOS deeplink sends "path + query" instead of just path (flutter/engine#23562) * 1474d08 Roll Skia from 8a37fb2c605b to 37d16f135265 (4 revisions) (flutter/engine#23839) * 3da13fc Make android more lenient when it comes to out-of-order key event responses (flutter/engine#23604) * 9223073 Fix background crash when FlutterView going appear while app goes background (flutter/engine#23175) * 7c19824 Pass the filename directly to JNI for loading deferred component. (flutter/engine#23824) * 5dc2469 Reland path vol tracker (flutter/engine#23840) * e7e76f1 Roll Skia from 37d16f135265 to e89d8ea20b62 (2 revisions) (flutter/engine#23841) * 07f4861 Roll Dart SDK from 704928da5702 to 1db2d4d95562 (1 revision) (flutter/engine#23846) * 993ab78 Roll Skia from e89d8ea20b62 to c09761f57605 (1 revision) (flutter/engine#23843) * a4836a6 Call Dart plugin registrant if available (flutter/engine#23813) * 475a234 Roll Fuchsia Linux SDK from UGavhI1zv... to mODEe2CNk... (flutter/engine#23848) * b51da31 Roll Skia from c09761f57605 to 450f8565c7f3 (5 revisions) (flutter/engine#23851) * cb7106d Roll Skia from 450f8565c7f3 to 372791761157 (1 revision) (flutter/engine#23855) * 69980e5 Roll Fuchsia Mac SDK from 7nZajqutF... to tuJCioUf3... (flutter/engine#23857) * 20ff574 Roll Skia from 372791761157 to ce75036b3eaf (4 revisions) (flutter/engine#23858) * 0118b54 Implements accessibility bridge in common library (flutter/engine#23491) * ffc77f0 Search multiple paths when loading deferred component .so files. (flutter/engine#23849) * 71d264d Revert "implemented GetMainContext() for opengl (#23634)" (flutter/engine#23859) * fb48735 Roll Skia from ce75036b3eaf to cc6961b9ac5e (3 revisions) (flutter/engine#23860) * fdddf87 Roll Dart SDK from 1db2d4d95562 to 82b4c77fb17f (2 revisions) (flutter/engine#23861)

…utter#23824) When .so files are in the lib/ path in the APK, it can be dlopen-ed directly using just the filename. We don't need to search for the file. The interface has thus been changed to accept a single path instead of a search directory. Also instead of hardcoding the .so basename and assets directory, read them from FlutterApplicationInfo instead.

chingjun requested a review from GaryQian January 21, 2021 05:56

flutter-dashboard bot added the platform-android label Jan 21, 2021

google-cla bot added the cla: yes label Jan 21, 2021

GaryQian approved these changes Jan 21, 2021

View reviewed changes

GaryQian reviewed Jan 21, 2021

View reviewed changes

chingjun added 2 commits January 21, 2021 13:23

Review feedback and code formatting

75c38f7

Code formatting

eda4ee2

chingjun merged commit 7c19824 into flutter:master Jan 22, 2021

engine-flutter-autoroll mentioned this pull request Jan 22, 2021

Roll Engine from 90641fa92385 to 7c19824c6d08 (17 revisions) flutter/flutter#74461

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jan 22, 2021

7c19824 Pass the filename directly to JNI for loading deferred compon…

5957e51

…ent. (flutter/engine#23824)

engine-flutter-autoroll mentioned this pull request Jan 22, 2021

Roll Engine from 90641fa92385 to 993ab786eeff (21 revisions) flutter/flutter#74474

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jan 22, 2021

7c19824 Pass the filename directly to JNI for loading deferred compon…

9aab9eb

…ent. (flutter/engine#23824)

engine-flutter-autoroll mentioned this pull request Jan 22, 2021

Roll Engine from 90641fa92385 to 475a2349d53e (23 revisions) flutter/flutter#74478

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jan 22, 2021

7c19824 Pass the filename directly to JNI for loading deferred compon…

182e4f5

…ent. (flutter/engine#23824)

engine-flutter-autoroll mentioned this pull request Jan 22, 2021

Roll Engine from 90641fa92385 to b51da31eb93c (24 revisions) flutter/flutter#74487

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jan 22, 2021

7c19824 Pass the filename directly to JNI for loading deferred compon…

2d39f7d

…ent. (flutter/engine#23824)

engine-flutter-autoroll mentioned this pull request Jan 22, 2021

Roll Engine from 90641fa92385 to cb7106dd7c39 (25 revisions) flutter/flutter#74501

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jan 22, 2021

7c19824 Pass the filename directly to JNI for loading deferred compon…

f0e6c6e

…ent. (flutter/engine#23824)

engine-flutter-autoroll mentioned this pull request Jan 22, 2021

Roll Engine from 90641fa92385 to 69980e5b8342 (26 revisions) flutter/flutter#74506

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jan 22, 2021

7c19824 Pass the filename directly to JNI for loading deferred compon…

c871cb1

…ent. (flutter/engine#23824)

engine-flutter-autoroll mentioned this pull request Jan 22, 2021

Roll Engine from 90641fa92385 to 20ff574d871d (27 revisions) flutter/flutter#74510

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jan 22, 2021

7c19824 Pass the filename directly to JNI for loading deferred compon…

b79a5a1

…ent. (flutter/engine#23824)

engine-flutter-autoroll mentioned this pull request Jan 22, 2021

Roll Engine from 90641fa92385 to ffc77f0da35b (29 revisions) flutter/flutter#74515

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jan 22, 2021

7c19824 Pass the filename directly to JNI for loading deferred compon…

825abda

…ent. (flutter/engine#23824)

engine-flutter-autoroll mentioned this pull request Jan 22, 2021

Roll Engine from 90641fa92385 to fdddf8708039 (32 revisions) flutter/flutter#74517

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jan 22, 2021

7c19824 Pass the filename directly to JNI for loading deferred compon…

59692ab

…ent. (flutter/engine#23824)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Pass the filename directly to JNI for loading deferred component. #23824

Pass the filename directly to JNI for loading deferred component. #23824

Uh oh!

chingjun commented Jan 21, 2021

Uh oh!

chingjun commented Jan 21, 2021

Uh oh!

GaryQian commented Jan 21, 2021

Uh oh!

GaryQian left a comment

Uh oh!

GaryQian Jan 21, 2021

Uh oh!

chingjun Jan 21, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Pass the filename directly to JNI for loading deferred component. #23824

Pass the filename directly to JNI for loading deferred component. #23824

Uh oh!

Conversation

chingjun commented Jan 21, 2021

Pre-launch Checklist

Uh oh!

chingjun commented Jan 21, 2021

Uh oh!

GaryQian commented Jan 21, 2021

Uh oh!

GaryQian left a comment

Choose a reason for hiding this comment

Uh oh!

GaryQian Jan 21, 2021

Choose a reason for hiding this comment

Uh oh!

chingjun Jan 21, 2021

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants