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

fix setInitialRoute #6774

Merged
merged 5 commits into from
Nov 7, 2018
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -20,3 +20,4 @@ tags
Thumbs.db
.idea
pubspec.lock
.vscode/
4 changes: 2 additions & 2 deletions shell/platform/darwin/ios/framework/Headers/FlutterEngine.h
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ FLUTTER_EXPORT
* tree-shaken by the Dart compiler.
* @return YES if the call succeeds in creating and running a Flutter Engine instance; NO otherwise.
*/
- (bool)runWithEntrypoint:(NSString*)entrypoint;
- (BOOL)runWithEntrypoint:(NSString*)entrypoint;

/**
* Runs a Dart program on an Isolate using the specified entrypoint and Dart library,
Expand All @@ -95,7 +95,7 @@ FLUTTER_EXPORT
* this will default to the same library as the `main()` function in the Dart program.
* @return YES if the call succeeds in creating and running a Flutter Engine instance; NO otherwise.
*/
- (bool)runWithEntrypoint:(NSString*)entrypoint libraryURI:(NSString*)uri;
- (BOOL)runWithEntrypoint:(NSString*)entrypoint libraryURI:(NSString*)uri;

/**
* Sets the `FlutterViewController` for this instance. The FlutterEngine must be
Expand Down
13 changes: 10 additions & 3 deletions shell/platform/darwin/ios/framework/Source/FlutterEngine.mm
Original file line number Diff line number Diff line change
Expand Up @@ -264,10 +264,10 @@ - (void)launchEngine:(NSString*)entrypoint libraryURI:(NSString*)libraryOrNil {
}));
}

- (bool)runWithEntrypoint:(NSString*)entrypoint libraryURI:(NSString*)libraryURI {
- (BOOL)createShell:(NSString*)entrypoint libraryURI:(NSString*)libraryURI {
if (_shell != nullptr) {
FML_LOG(WARNING) << "This FlutterEngine was already invoked.";
return false;
return NO;
}

static size_t shellCount = 1;
Expand Down Expand Up @@ -351,13 +351,20 @@ - (bool)runWithEntrypoint:(NSString*)entrypoint libraryURI:(NSString*)libraryURI
<< entrypoint.UTF8String;
} else {
[self maybeSetupPlatformViewChannels];
}

return _shell != nullptr;
}

- (BOOL)runWithEntrypoint:(NSString*)entrypoint libraryURI:(NSString*)libraryURI {
if ([self createShell:entrypoint libraryURI:libraryURI]) {
Copy link
Contributor

Choose a reason for hiding this comment

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

What happens if someone calls launchEngine with a different set of arguments than what was passed to an earlier call to createShell? Does it make sense to provide an API that allows them to be different?

Copy link
Contributor Author

@dnfield dnfield Nov 7, 2018

Choose a reason for hiding this comment

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

That's existing API. launchEngine is part of our internal API, not available in public headers.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In other words, we could only ever do this in the embedder implementation - the API to do so is not public. But it would likely result in bad things.

[self launchEngine:entrypoint libraryURI:libraryURI];
}

return _shell != nullptr;
}

- (bool)runWithEntrypoint:(NSString*)entrypoint {
- (BOOL)runWithEntrypoint:(NSString*)entrypoint {
return [self runWithEntrypoint:entrypoint libraryURI:nil];
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
- (shell::FlutterPlatformViewsController*)platformViewsController;
- (FlutterTextInputPlugin*)textInputPlugin;
- (void)launchEngine:(NSString*)entrypoint libraryURI:(NSString*)libraryOrNil;
- (BOOL)createShell:(NSString*)entrypoint libraryURI:(NSString*)libraryOrNil;

@end

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ @implementation FlutterViewController {
blink::ViewportMetrics _viewportMetrics;
BOOL _initialized;
BOOL _viewOpaque;
BOOL _engineNeedsLaunch;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@chinmaygarde do you have any thoughts on whether we can avoid this?

}

#pragma mark - Manage and override all designated initializers
Expand All @@ -49,6 +50,7 @@ - (instancetype)initWithEngine:(FlutterEngine*)engine
if (self) {
_viewOpaque = YES;
_engine.reset([engine retain]);
_engineNeedsLaunch = NO;
_flutterView.reset([[FlutterView alloc] initWithDelegate:_engine opaque:self.isViewOpaque]);
_weakFactory = std::make_unique<fml::WeakPtrFactory<FlutterViewController>>(self);

Expand All @@ -68,8 +70,8 @@ - (instancetype)initWithProject:(FlutterDartProject*)projectOrNil
_weakFactory = std::make_unique<fml::WeakPtrFactory<FlutterViewController>>(self);
_engine.reset([[FlutterEngine alloc] initWithName:@"io.flutter" project:projectOrNil]);
_flutterView.reset([[FlutterView alloc] initWithDelegate:_engine opaque:self.isViewOpaque]);
[_engine.get() runWithEntrypoint:nil];
[_engine.get() setViewController:self];
[_engine.get() createShell:nil libraryURI:nil];
_engineNeedsLaunch = YES;

[self performCommonViewControllerInitialization];
}
Expand Down Expand Up @@ -371,6 +373,12 @@ - (void)surfaceUpdated:(BOOL)appeared {
- (void)viewWillAppear:(BOOL)animated {
TRACE_EVENT0("flutter", "viewWillAppear");

if (_engineNeedsLaunch) {
[_engine.get() launchEngine:nil libraryURI:nil];
[_engine.get() setViewController:self];
_engineNeedsLaunch = NO;
}

// Only recreate surface on subsequent appearances when viewport metrics are known.
// First time surface creation is done on viewDidLayoutSubviews.
if (_viewportMetrics.physical_width)
Expand Down