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

Implemented library uri support for FlutterFragments and FlutterActivities #30726

Merged
merged 3 commits into from
Jan 10, 2022

Conversation

gaaclarke
Copy link
Member

@gaaclarke gaaclarke commented Jan 7, 2022

issue flutter/flutter#91841

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.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

@gaaclarke gaaclarke force-pushed the android-library-uri branch 4 times, most recently from 0c3906a to 29b896f Compare January 7, 2022 18:55
@gaaclarke gaaclarke force-pushed the android-library-uri branch from 29b896f to 104dd5c Compare January 7, 2022 19:20
@gaaclarke gaaclarke marked this pull request as ready for review January 7, 2022 19:46
@gaaclarke gaaclarke requested a review from blasten January 7, 2022 19:46
*
* <p>Example value: "package:foo/bar.dart"
*
* <p>This preference can be controlled by setting a {@code <meta-data>} called {@link
Copy link

Choose a reason for hiding this comment

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

btw, the manifest can have dynamic values that are passed from the tool to Gradle, then copied into the final manifest. e.g. https://github.com/flutter/flutter/blob/master/packages/flutter_tools/templates/app_shared/android.tmpl/app/src/main/AndroidManifest.xml.tmpl#L5

cc @GaryQian

Copy link

Choose a reason for hiding this comment

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

This needs another migration of existing manifest files

@@ -824,6 +825,32 @@ public String getDartEntrypointFunctionName() {
}
}

/**
* The Dart library uri for the entrypoint that will be executed as soon as the Dart snapshot is
Copy link

Choose a reason for hiding this comment

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

nit: uppercase for URI

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

@@ -105,6 +105,8 @@

/** The Dart entrypoint method name that is executed upon initialization. */
protected static final String ARG_DART_ENTRYPOINT = "dart_entrypoint";
/** */
Copy link

Choose a reason for hiding this comment

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

nit: missing comment

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

@@ -1026,6 +1036,20 @@ public String getDartEntrypointFunctionName() {
return getArguments().getString(ARG_DART_ENTRYPOINT, "main");
}

/**
* Returns the library uri of the Dart method that this {@code FlutterFragment} should execute to
Copy link

Choose a reason for hiding this comment

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

nit: uppercase URI

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

Copy link

@blasten blasten left a comment

Choose a reason for hiding this comment

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

lgtm

@ColdPaleLight
Copy link
Member

ColdPaleLight commented Jan 8, 2022

I guess that we also need to add getDartEntrypointLibraryUri to FlutterFragmentActivity, just like getDartEntrypointFunctionName.

public String getDartEntrypointFunctionName() {
try {
Bundle metaData = getMetaData();
String desiredDartEntrypoint =
metaData != null ? metaData.getString(DART_ENTRYPOINT_META_DATA_KEY) : null;
return desiredDartEntrypoint != null ? desiredDartEntrypoint : DEFAULT_DART_ENTRYPOINT;
} catch (PackageManager.NameNotFoundException e) {
return DEFAULT_DART_ENTRYPOINT;
}
}

return FlutterFragment.withNewEngine()
.dartEntrypoint(getDartEntrypointFunctionName())

@gaaclarke
Copy link
Member Author

I guess that we also need to add getDartEntrypointLibraryUri to FlutterFragmentActivity, just like getDartEntrypointFunctionName.

It should already exist by virtue of FlutterFragmentActivity inheriting from FlutterActivity. It looks like FlutterFragmentActivity's getDartEntrypointFunctionName is the same as FlutterActivity's. I'm not sure why it overloads it with the exact same code. Probably a quirk of changing inheritance.

@gaaclarke gaaclarke merged commit 279e3af into flutter:main Jan 10, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jan 10, 2022
zanderso pushed a commit to flutter/flutter that referenced this pull request Jan 11, 2022
* 58305d1 Roll Skia from dd575bc0f1f5 to 0056f3f006de (2 revisions) (flutter/engine#30765)

* 92a4f99 Allow additional expose_dirs in flutter_runner (flutter/engine#30749)

* 358605c [web] Remove EngineParagraph and ParagraphGeometricStyle (flutter/engine#30766)

* 8b9f625 Remove glitch when displaying platform views (flutter/engine#30724)

* bd65330 Add missing dependencies to the background image app (flutter/engine#30769)

* 65dfc9e [android] Remove the FlutterView casting, add a @nonnull and fix code style (flutter/engine#30734)

* 00e2a47 Roll Skia from 0056f3f006de to 3e1354a592bc (3 revisions) (flutter/engine#30771)

* 279e3af Implemented library uri support for FlutterFragments and FlutterActivities (flutter/engine#30726)

* 36ad9f1 Roll Skia from 3e1354a592bc to 55b4dc3f7a1c (1 revision) (flutter/engine#30773)
@ColdPaleLight
Copy link
Member

ColdPaleLight commented Jan 11, 2022

It should already exist by virtue of FlutterFragmentActivity inheriting from FlutterActivity. It looks like FlutterFragmentActivity's getDartEntrypointFunctionName is the same as FlutterActivity's. I'm not sure why it overloads it with the exact same code. Probably a quirk of changing inheritance.

Sorry, I didn't express clearly. Since the superclass of FlutterFragmentActivity is FragmentActivity instead of FlutterActivity, I think we still need to add this feature to FlutterFragmentActivity :-)
Here is a PR I filed to do this #30790

JsouLiang pushed a commit to JsouLiang/engine that referenced this pull request Jan 14, 2022
…ities (flutter#30726)

* Implemented library uri support for FlutterFragments and FlutterActivities.

* added docstrings

* emmanuel feedback
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants