From f4d4aa7b657135e2eb838d6d1a6a1248229bb1ac Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Thu, 5 Sep 2024 13:02:18 -0700 Subject: [PATCH 1/6] [engine] always force platform channel responses to schedule a task. --- shell/common/shell.cc | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/shell/common/shell.cc b/shell/common/shell.cc index 79c532640a969..a23bc5498efe6 100644 --- a/shell/common/shell.cc +++ b/shell/common/shell.cc @@ -1076,10 +1076,10 @@ void Shell::OnPlatformViewDispatchPlatformMessage( // The static leak checker gets confused by the use of fml::MakeCopyable. // NOLINTNEXTLINE(clang-analyzer-cplusplus.NewDeleteLeaks) - fml::TaskRunner::RunNowOrPostTask( - task_runners_.GetUITaskRunner(), - fml::MakeCopyable([engine = engine_->GetWeakPtr(), - message = std::move(message)]() mutable { + // This logic must always explicitly post a task so that we are guaranteed + // to wake up the UI message loop to flush tasks. + task_runners_.GetUITaskRunner()->PostTask(fml::MakeCopyable( + [engine = engine_->GetWeakPtr(), message = std::move(message)]() mutable { if (engine) { engine->DispatchPlatformMessage(std::move(message)); } From 58ea52407bfb7446d44beb0ba818f14ffdbd1f6b Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Fri, 6 Sep 2024 10:47:02 -0700 Subject: [PATCH 2/6] make initial route not confusing as all heck. --- shell/common/engine.cc | 1 + .../darwin/ios/framework/Source/FlutterEngine.mm | 10 +++------- 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/shell/common/engine.cc b/shell/common/engine.cc index ac628eb929465..37f71f4496d87 100644 --- a/shell/common/engine.cc +++ b/shell/common/engine.cc @@ -61,6 +61,7 @@ Engine::Engine( task_runners_(task_runners), weak_factory_(this) { pointer_data_dispatcher_ = dispatcher_maker(*this); + initial_route_ = settings_.route; } Engine::Engine(Delegate& delegate, diff --git a/shell/platform/darwin/ios/framework/Source/FlutterEngine.mm b/shell/platform/darwin/ios/framework/Source/FlutterEngine.mm index b1ca5e66ae6d1..ccf24fa0f5c3b 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterEngine.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterEngine.mm @@ -602,13 +602,6 @@ - (void)setUpChannels { binaryMessenger:self.binaryMessenger codec:[FlutterJSONMethodCodec sharedInstance]]); - if ([_initialRoute length] > 0) { - // Flutter isn't ready to receive this method call yet but the channel buffer will cache this. - [_navigationChannel invokeMethod:@"setInitialRoute" arguments:_initialRoute]; - [_initialRoute release]; - _initialRoute = nil; - } - _restorationChannel.reset([[FlutterMethodChannel alloc] initWithName:@"flutter/restoration" binaryMessenger:self.binaryMessenger @@ -904,6 +897,9 @@ - (BOOL)createShell:(NSString*)entrypoint _isGpuDisabled = [UIApplication sharedApplication].applicationState == UIApplicationStateBackground; #endif + settings.route = [self.initialRoute UTF8String]; + [self.initialRoute release]; + self.initialRoute = nil; // Create the shell. This is a blocking operation. std::unique_ptr shell = flutter::Shell::Create( From 7cfdde5ed39eff3d4960f11538d0bb9db64418c4 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Fri, 6 Sep 2024 10:48:59 -0700 Subject: [PATCH 3/6] set initial route. --- shell/common/engine_unittests.cc | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/shell/common/engine_unittests.cc b/shell/common/engine_unittests.cc index 7a822c5dca1b7..8e0108eaccc03 100644 --- a/shell/common/engine_unittests.cc +++ b/shell/common/engine_unittests.cc @@ -252,6 +252,27 @@ TEST_F(EngineTest, Create) { }); } +TEST_F(EngineTest, CreateWithRoute) { + PostUITaskSync([this] { + auto settings = settings_; + settings.route = "/testo"; + auto engine = std::make_unique( + /*delegate=*/delegate_, + /*dispatcher_maker=*/dispatcher_maker_, + /*image_decoder_task_runner=*/image_decoder_task_runner_, + /*task_runners=*/task_runners_, + /*settings=*/settings, + /*animator=*/std::move(animator_), + /*io_manager=*/io_manager_, + /*font_collection=*/std::make_shared(), + /*runtime_controller=*/std::move(runtime_controller_), + /*gpu_disabled_switch=*/std::make_shared()); + + EXPECT_TRUE(engine); + EXPECT_EQ(engine->InitialRoute(), "/testo"); + }); +} + TEST_F(EngineTest, DispatchPlatformMessageUnknown) { PostUITaskSync([this] { MockRuntimeDelegate client; From d1a36269f4f01efa9f12c2bd36f94d50cbde89da Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Fri, 6 Sep 2024 11:05:18 -0700 Subject: [PATCH 4/6] add guard and comment. --- .../darwin/ios/framework/Source/FlutterEngine.mm | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/shell/platform/darwin/ios/framework/Source/FlutterEngine.mm b/shell/platform/darwin/ios/framework/Source/FlutterEngine.mm index ccf24fa0f5c3b..b620737fa1936 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterEngine.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterEngine.mm @@ -897,9 +897,14 @@ - (BOOL)createShell:(NSString*)entrypoint _isGpuDisabled = [UIApplication sharedApplication].applicationState == UIApplicationStateBackground; #endif - settings.route = [self.initialRoute UTF8String]; - [self.initialRoute release]; - self.initialRoute = nil; + // Override the setting route, as the dart project or function may have specified + // different values. During construction, the Engine constuctor will read the + // value of settings.route to determine the initial route value. + if (self.initialRoute) { + settings.route = [self.initialRoute UTF8String]; + [self.initialRoute release]; + self.initialRoute = nil; + } // Create the shell. This is a blocking operation. std::unique_ptr shell = flutter::Shell::Create( From 4277653581ce521151ecb3a67c02d311d1c018c3 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Fri, 6 Sep 2024 12:26:44 -0700 Subject: [PATCH 5/6] try this. --- shell/platform/darwin/ios/framework/Source/FlutterEngine.mm | 1 - 1 file changed, 1 deletion(-) diff --git a/shell/platform/darwin/ios/framework/Source/FlutterEngine.mm b/shell/platform/darwin/ios/framework/Source/FlutterEngine.mm index b620737fa1936..3b65f469820a8 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterEngine.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterEngine.mm @@ -902,7 +902,6 @@ - (BOOL)createShell:(NSString*)entrypoint // value of settings.route to determine the initial route value. if (self.initialRoute) { settings.route = [self.initialRoute UTF8String]; - [self.initialRoute release]; self.initialRoute = nil; } From ba3ecbe4ac89d6300c129dbb2581ffcecc16ee6a Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Fri, 6 Sep 2024 13:49:42 -0700 Subject: [PATCH 6/6] maybe this works. --- .../ios/framework/Source/FlutterEngineTest.mm | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/shell/platform/darwin/ios/framework/Source/FlutterEngineTest.mm b/shell/platform/darwin/ios/framework/Source/FlutterEngineTest.mm index 5779d4b496126..3b4702269bf85 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterEngineTest.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterEngineTest.mm @@ -201,17 +201,17 @@ - (void)testRunningInitialRouteSendsNavigationMessage { // Run with an initial route. [engine runWithEntrypoint:FlutterDefaultDartEntrypoint initialRoute:@"test"]; - // Now check that an encoded method call has been made on the binary messenger to set the - // initial route to "test". + // Initial route is set directly in the shell/engine and should not send a platform + // channel message as it will arrive too late. FlutterMethodCall* setInitialRouteMethodCall = [FlutterMethodCall methodCallWithMethodName:@"setInitialRoute" arguments:@"test"]; NSData* encodedSetInitialRouteMethod = [[FlutterJSONMethodCodec sharedInstance] encodeMethodCall:setInitialRouteMethodCall]; - OCMVerify([mockBinaryMessenger sendOnChannel:@"flutter/navigation" + OCMReject([mockBinaryMessenger sendOnChannel:@"flutter/navigation" message:encodedSetInitialRouteMethod]); } -- (void)testInitialRouteSettingsSendsNavigationMessage { +- (void)testInitialRouteSettingsDoesNotSendNavigationMessage { id mockBinaryMessenger = OCMClassMock([FlutterBinaryMessengerRelay class]); auto settings = FLTDefaultSettingsForBundle(); @@ -221,13 +221,13 @@ - (void)testInitialRouteSettingsSendsNavigationMessage { [engine setBinaryMessenger:mockBinaryMessenger]; [engine run]; - // Now check that an encoded method call has been made on the binary messenger to set the - // initial route to "test". + // Initial route is set directly in the shell/engine and should not send a platform + // channel message as it will arrive too late. FlutterMethodCall* setInitialRouteMethodCall = [FlutterMethodCall methodCallWithMethodName:@"setInitialRoute" arguments:@"test"]; NSData* encodedSetInitialRouteMethod = [[FlutterJSONMethodCodec sharedInstance] encodeMethodCall:setInitialRouteMethodCall]; - OCMVerify([mockBinaryMessenger sendOnChannel:@"flutter/navigation" + OCMReject([mockBinaryMessenger sendOnChannel:@"flutter/navigation" message:encodedSetInitialRouteMethod]); }