Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Android Embedding PR22: Polish - FlutterActivity Intent factories, FlutterFragment control of render modes, FlutterSurfaceView transparent until rendering is ready. #8317

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -9,17 +9,21 @@
import android.content.pm.ActivityInfo;
import android.content.pm.ApplicationInfo;
import android.content.pm.PackageManager;
import android.os.Build;
import android.os.Bundle;
import android.support.annotation.NonNull;
import android.support.annotation.Nullable;
import android.support.v4.app.FragmentActivity;
import android.support.v4.app.FragmentManager;
import android.view.View;
import android.view.ViewGroup;
import android.view.Window;
import android.view.WindowManager;
import android.widget.FrameLayout;

import io.flutter.embedding.engine.FlutterEngine;
import io.flutter.embedding.engine.FlutterShellArgs;
import io.flutter.plugin.platform.PlatformPlugin;
import io.flutter.view.FlutterMain;

/**
Expand Down Expand Up @@ -60,26 +64,75 @@ public class FlutterActivity extends FragmentActivity {
private static final String TAG = "FlutterActivity";

// Meta-data arguments, processed from manifest XML.
private static final String DART_ENTRYPOINT_META_DATA_KEY = "io.flutter.Entrypoint";
private static final String INITIAL_ROUTE_META_DATA_KEY = "io.flutter.InitialRoute";
protected static final String DART_ENTRYPOINT_META_DATA_KEY = "io.flutter.Entrypoint";
protected static final String INITIAL_ROUTE_META_DATA_KEY = "io.flutter.InitialRoute";

// Intent extra arguments.
public static final String EXTRA_DART_ENTRYPOINT = "dart_entrypoint";
public static final String EXTRA_INITIAL_ROUTE = "initial_route";
protected static final String EXTRA_DART_ENTRYPOINT = "dart_entrypoint";
protected static final String EXTRA_INITIAL_ROUTE = "initial_route";

// Default configuration.
protected static final String DEFAULT_DART_ENTRYPOINT = "main";
protected static final String DEFAULT_INITIAL_ROUTE = "/";

// FlutterFragment management.
private static final String TAG_FLUTTER_FRAGMENT = "flutter_fragment";
// TODO(mattcarroll): replace ID with R.id when build system supports R.java
private static final int FRAGMENT_CONTAINER_ID = 609893468; // random number
private FlutterFragment flutterFragment;

/**
* Builder to create an {@code Intent} that launches a {@code FlutterActivity} with the
* desired configuration.
*/
public static class IntentBuilder {
Copy link
Contributor

Choose a reason for hiding this comment

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

final

(Google's [internal] "Effective Java" guide, Item 19: "Design and document for inheritance or else prohibit it")

private String dartEntrypoint = DEFAULT_DART_ENTRYPOINT;
private String initialRoute = DEFAULT_INITIAL_ROUTE;

/**
* The name of the initial Dart method to invoke, defaults to "main".
*/
@NonNull
public IntentBuilder dartEntrypoint(@NonNull String dartEntrypoint) {
this.dartEntrypoint = dartEntrypoint;
return this;
}

/**
* The initial route that a Flutter app will render in this {@link FlutterFragment},
* defaults to "/".
*/
@NonNull
public IntentBuilder initialRoute(@NonNull String initialRoute) {
this.initialRoute = initialRoute;
return this;
}

@NonNull
public Intent build(@NonNull Context context) {
return new Intent(context, FlutterActivity.class)
.putExtra(EXTRA_DART_ENTRYPOINT, dartEntrypoint)
.putExtra(EXTRA_INITIAL_ROUTE, initialRoute);
}
}

@Override
public void onCreate(Bundle savedInstanceState) {
super.onCreate(savedInstanceState);
setContentView(createFragmentContainer());
configureStatusBarForFullscreenFlutterExperience();
ensureFlutterFragmentCreated();
}

private void configureStatusBarForFullscreenFlutterExperience() {
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.LOLLIPOP) {
Copy link
Contributor

Choose a reason for hiding this comment

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

optional, subjective: Consider using guard clauses to reduce nesting.

https://testing.googleblog.com/2017/06/code-health-reduce-nesting-reduce.html

On another note though is it OK that we're not executing anything on older SDKs? Is the UI going to still render correctly?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think older Android versions always have a black status bar.

Window window = getWindow();
window.addFlags(WindowManager.LayoutParams.FLAG_DRAWS_SYSTEM_BAR_BACKGROUNDS);
window.setStatusBarColor(0x40000000);
window.getDecorView().setSystemUiVisibility(PlatformPlugin.DEFAULT_SYSTEM_UI);
}
}

/**
* Creates a {@link FrameLayout} with an ID of {@code #FRAGMENT_CONTAINER_ID} that will contain
* the {@link FlutterFragment} displayed by this {@code FlutterActivity}.
Expand Down Expand Up @@ -125,12 +178,13 @@ private void ensureFlutterFragmentCreated() {
*/
@NonNull
protected FlutterFragment createFlutterFragment() {
return FlutterFragment.newInstance(
getDartEntrypoint(),
getInitialRoute(),
getAppBundlePath(),
FlutterShellArgs.fromIntent(getIntent())
);
return new FlutterFragment.Builder()
.dartEntrypoint(getDartEntrypoint())
.initialRoute(getInitialRoute())
.appBundlePath(getAppBundlePath())
.flutterShellArgs(FlutterShellArgs.fromIntent(getIntent()))
.renderMode(FlutterView.RenderMode.surface)
.build();
}

@Override
Expand Down Expand Up @@ -216,7 +270,7 @@ protected String getAppBundlePath() {
* <p>
* Subclasses may override this method to directly control the Dart entrypoint.
*/
@Nullable
@NonNull
protected String getDartEntrypoint() {
if (getIntent().hasExtra(EXTRA_DART_ENTRYPOINT)) {
return getIntent().getStringExtra(EXTRA_DART_ENTRYPOINT);
Expand All @@ -228,9 +282,10 @@ protected String getDartEntrypoint() {
PackageManager.GET_META_DATA|PackageManager.GET_ACTIVITIES
);
Bundle metadata = activityInfo.metaData;
return metadata != null ? metadata.getString(DART_ENTRYPOINT_META_DATA_KEY) : null;
String desiredDartEntrypoint = metadata != null ? metadata.getString(DART_ENTRYPOINT_META_DATA_KEY) : null;
return desiredDartEntrypoint != null ? desiredDartEntrypoint : DEFAULT_DART_ENTRYPOINT;
} catch (PackageManager.NameNotFoundException e) {
return null;
return DEFAULT_DART_ENTRYPOINT;
}
}

Expand All @@ -251,7 +306,7 @@ protected String getDartEntrypoint() {
* <p>
* Subclasses may override this method to directly control the initial route.
*/
@Nullable
@NonNull
protected String getInitialRoute() {
if (getIntent().hasExtra(EXTRA_INITIAL_ROUTE)) {
return getIntent().getStringExtra(EXTRA_INITIAL_ROUTE);
Expand All @@ -263,9 +318,10 @@ protected String getInitialRoute() {
PackageManager.GET_META_DATA|PackageManager.GET_ACTIVITIES
);
Bundle metadata = activityInfo.metaData;
return metadata != null ? metadata.getString(INITIAL_ROUTE_META_DATA_KEY) : null;
String desiredInitialRoute = metadata != null ? metadata.getString(INITIAL_ROUTE_META_DATA_KEY) : null;
return desiredInitialRoute != null ? desiredInitialRoute : DEFAULT_INITIAL_ROUTE;
} catch (PackageManager.NameNotFoundException e) {
return null;
return DEFAULT_INITIAL_ROUTE;
}
}

Expand Down
153 changes: 106 additions & 47 deletions shell/platform/android/io/flutter/embedding/android/FlutterFragment.java
Original file line number Diff line number Diff line change
Expand Up @@ -61,53 +61,92 @@ public class FlutterFragment extends Fragment {
private static final String ARG_INITIAL_ROUTE = "initial_route";
private static final String ARG_APP_BUNDLE_PATH = "app_bundle_path";
private static final String ARG_FLUTTER_INITIALIZATION_ARGS = "initialization_args";
private static final String ARG_FLUTTERVIEW_RENDER_MODE = "flutterview_render_mode";

/**
* Factory method that creates a new {@link FlutterFragment} with a default configuration.
* <ul>
* <li>default Dart entrypoint of "main"</li>
* <li>initial route of "/"</li>
* <li>default app bundle location</li>
* <li>no special engine arguments</li>
* </ul>
* @return new {@link FlutterFragment}
*/
public static FlutterFragment newInstance() {
return newInstance(
null,
null,
null,
null
);
}

/**
* Factory method that creates a new {@link FlutterFragment} with the given configuration.
* Builder that creates a new {@code FlutterFragment} with {@code arguments} that correspond
* to the values set on this {@code Builder}.
* <p>
* @param dartEntrypoint the name of the initial Dart method to invoke, defaults to "main"
* @param initialRoute the first route that a Flutter app will render in this {@link FlutterFragment},
* defaults to "/"
* @param appBundlePath the path to the app bundle which contains the Dart app to execute, defaults
* to {@link FlutterMain#findAppBundlePath(Context)}
* @param flutterShellArgs any special configuration arguments for the Flutter engine
*
* @return a new {@link FlutterFragment}
* To create a {@code FlutterFragment} with default {@code arguments}, invoke {@code build()}
* immeidately:
Copy link
Contributor

Choose a reason for hiding this comment

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

Spelling

* {@code
* FlutterFragment fragment = new FlutterFragment.Builder().build();
* }
*/
public static FlutterFragment newInstance(@Nullable String dartEntrypoint,
@Nullable String initialRoute,
@Nullable String appBundlePath,
@Nullable FlutterShellArgs flutterShellArgs) {
FlutterFragment frag = new FlutterFragment();

Bundle args = createArgsBundle(
dartEntrypoint,
initialRoute,
appBundlePath,
flutterShellArgs
);
frag.setArguments(args);
public static class Builder {
Copy link
Contributor

Choose a reason for hiding this comment

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

final

private String dartEntrypoint = "main";
private String initialRoute = "/";
private String appBundlePath = null;
private FlutterShellArgs shellArgs = null;
private FlutterView.RenderMode renderMode = FlutterView.RenderMode.surface;

/**
* The name of the initial Dart method to invoke, defaults to "main".
*/
@NonNull
public Builder dartEntrypoint(@NonNull String dartEntrypoint) {
this.dartEntrypoint = dartEntrypoint;
return this;
}

/**
* The initial route that a Flutter app will render in this {@link FlutterFragment},
* defaults to "/".
*/
@NonNull
public Builder initialRoute(@NonNull String initialRoute) {
this.initialRoute = initialRoute;
return this;
}

/**
* The path to the app bundle which contains the Dart app to execute, defaults
* to {@link FlutterMain#findAppBundlePath(Context)}
*/
@NonNull
public Builder appBundlePath(@NonNull String appBundlePath) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Any reason not to say this is @Nullable?

I don't feel strongly either way - the argument in favor of @NonNull is that since it defaults to null, there's no reason to call this with a null value. But otoh, it wouldn't hurt either...

this.appBundlePath = appBundlePath;
return this;
}

/**
* Any special configuration arguments for the Flutter engine
*/
@NonNull
public Builder flutterShellArgs(@NonNull FlutterShellArgs shellArgs) {
Copy link
Contributor

Choose a reason for hiding this comment

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

And here too

Copy link
Contributor

Choose a reason for hiding this comment

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

Side note: since the values we allow for shell args are well defined, I'd consider making the supported args in FlutterShellArgs an enum rather than Strings. Something like:

public final class FlutterShellArgs {
  public static final enum Argument {
    /**
     * <documentation>
     */
    TRACE_STARTUP("trace-startup"),

    /**
     * <documentation>
     */
    START_PAUSED("start-paused"),

    // More supported values here...

    /**
     * <documentation>
     */
    VERBOSE_LOGGING("verbose-logging");

    /**
     * The intent key. (more documentation...)
     */
    public final String key;

    /**
     * The flag that's passed to the Flutter shell.
     */
    public final String value;

    private Argument(String key) {
      this.key = key;
      value = "--" + key;
    }
  }

  @NonNull
  public static FlutterShelArgs fromIntent(@NonNull Intent intent) {
    ...

    if (intent.getBooleanExtra(Argument.TRACE_STARTUP.key, false)) {
      args.add(Argument.TRACE_STARTUP.value);
    }

    ...
  }
}

Also, is there any reason for FlutterShellArgs to be mutable?

this.shellArgs = shellArgs;
return this;
}

/**
* Render Flutter either as a {@link FlutterView.RenderMode#surface} or a
* {@link FlutterView.RenderMode#texture}. You should use {@code surface} unless
* you have a specific reason to use {@code texture}. {@code texture} comes with
* a significant performance impact, but {@code texture} can be displayed
* beneath other Android {@code View}s and animated, whereas {@code surface}
* cannot.
*/
@NonNull
public Builder renderMode(@NonNull FlutterView.RenderMode renderMode) {
this.renderMode = renderMode;
return this;
}

@NonNull
public FlutterFragment build() {
FlutterFragment frag = new FlutterFragment();

Bundle args = createArgsBundle(
dartEntrypoint,
initialRoute,
appBundlePath,
shellArgs,
renderMode
);
frag.setArguments(args);

return frag;
return frag;
}
}

/**
Expand All @@ -118,16 +157,16 @@ public static FlutterFragment newInstance(@Nullable String dartEntrypoint,
* wants to this {@link Bundle}. Example:
* <pre>{@code
* public static MyFlutterFragment newInstance(String myNewArg) {
* // Create an instance of our subclass Fragment.
* // Create an instance of your subclass Fragment.
* MyFlutterFragment myFrag = new MyFlutterFragment();
*
* // Create the Bundle or args that FlutterFragment understands.
* Bundle args = FlutterFragment.createArgsBundle(...);
*
* // Add our new args to the bundle.
* // Add your new args to the bundle.
* args.putString(ARG_MY_NEW_ARG, myNewArg);
*
* // Give the args to our subclass Fragment.
* // Give the args to your subclass Fragment.
* myFrag.setArguments(args);
*
* // Return the newly created subclass Fragment.
Expand All @@ -139,13 +178,20 @@ public static FlutterFragment newInstance(@Nullable String dartEntrypoint,
* @param initialRoute the first route that a Flutter app will render in this {@link FlutterFragment}, defaults to "/"
* @param appBundlePath the path to the app bundle which contains the Dart app to execute
* @param flutterShellArgs any special configuration arguments for the Flutter engine
* @param renderMode render Flutter either as a {@link FlutterView.RenderMode#surface} or a
* {@link FlutterView.RenderMode#texture}. You should use {@code surface} unless
* you have a specific reason to use {@code texture}. {@code texture} comes with
* a significant performance impact, but {@code texture} can be displayed
* beneath other Android {@code View}s and animated, whereas {@code surface}
* cannot.
*
* @return Bundle of arguments that configure a {@link FlutterFragment}
*/
protected static Bundle createArgsBundle(@Nullable String dartEntrypoint,
@Nullable String initialRoute,
@Nullable String appBundlePath,
@Nullable FlutterShellArgs flutterShellArgs) {
@Nullable FlutterShellArgs flutterShellArgs,
@Nullable FlutterView.RenderMode renderMode) {
Bundle args = new Bundle();
args.putString(ARG_INITIAL_ROUTE, initialRoute);
args.putString(ARG_APP_BUNDLE_PATH, appBundlePath);
Expand All @@ -154,6 +200,7 @@ protected static Bundle createArgsBundle(@Nullable String dartEntrypoint,
if (null != flutterShellArgs) {
args.putStringArray(ARG_FLUTTER_INITIALIZATION_ARGS, flutterShellArgs.toArray());
}
args.putString(ARG_FLUTTERVIEW_RENDER_MODE, renderMode != null ? renderMode.name() : FlutterView.RenderMode.surface.name());
return args;
}

Expand Down Expand Up @@ -252,7 +299,7 @@ protected void onFlutterEngineCreated(@NonNull FlutterEngine flutterEngine) {
@Nullable
@Override
public View onCreateView(LayoutInflater inflater, @Nullable ViewGroup container, @Nullable Bundle savedInstanceState) {
flutterView = new FlutterView(getContext());
flutterView = new FlutterView(getContext(), getRenderMode());
flutterView.attachToFlutterEngine(flutterEngine);

// TODO(mattcarroll): the following call should exist here, but the plugin system needs to be revamped.
Expand Down Expand Up @@ -326,6 +373,18 @@ protected String getDartEntrypointFunctionName() {
return getArguments().getString(ARG_DART_ENTRYPOINT, "main");
}

/**
* Returns the desired {@link FlutterView.RenderMode} for the {@link FlutterView} displayed in
* this {@code FlutterFragment}.
*
* Defaults to {@link FlutterView.RenderMode#surface}.
*/
@NonNull
protected FlutterView.RenderMode getRenderMode() {
String renderModeName = getArguments().getString(ARG_FLUTTERVIEW_RENDER_MODE, FlutterView.RenderMode.surface.name());
return FlutterView.RenderMode.valueOf(renderModeName);
}

// TODO(mattcarroll): determine why this can't be in onResume(). Comment reason, or move if possible.
public void onPostResume() {
Log.d(TAG, "onPostResume()");
Expand Down
Loading