From 8ca7e1cd9055119dc7acbeaffdc6689ae716446d Mon Sep 17 00:00:00 2001 From: Jonah Williams Date: Fri, 6 Sep 2024 16:48:09 -0700 Subject: [PATCH] [engine] reland: always post tasks for platform channel responses. (flutter/engine#55006) Reland of https://github.com/flutter/engine/pull/54975 Fixes the initial route behavior for iOS. Previously the initial route setting would _always_ be posted as a task, which after merging the threads would fire after isolate creation, thus it would not actually update the initial route setting. Fixed Engine constructor so that it reads the initial route from the settings. --- engine/src/flutter/shell/common/engine.cc | 1 + .../flutter/shell/common/engine_unittests.cc | 21 +++++++++++++++++++ engine/src/flutter/shell/common/shell.cc | 8 +++---- .../ios/framework/Source/FlutterEngine.mm | 14 ++++++------- .../ios/framework/Source/FlutterEngineTest.mm | 14 ++++++------- 5 files changed, 40 insertions(+), 18 deletions(-) diff --git a/engine/src/flutter/shell/common/engine.cc b/engine/src/flutter/shell/common/engine.cc index ac628eb9294..37f71f4496d 100644 --- a/engine/src/flutter/shell/common/engine.cc +++ b/engine/src/flutter/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/engine/src/flutter/shell/common/engine_unittests.cc b/engine/src/flutter/shell/common/engine_unittests.cc index 7a822c5dca1..8e0108eaccc 100644 --- a/engine/src/flutter/shell/common/engine_unittests.cc +++ b/engine/src/flutter/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; diff --git a/engine/src/flutter/shell/common/shell.cc b/engine/src/flutter/shell/common/shell.cc index 79c532640a9..a23bc5498ef 100644 --- a/engine/src/flutter/shell/common/shell.cc +++ b/engine/src/flutter/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)); } diff --git a/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterEngine.mm b/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterEngine.mm index b1ca5e66ae6..3b65f469820 100644 --- a/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterEngine.mm +++ b/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterEngine.mm @@ -602,13 +602,6 @@ static constexpr int kNumProfilerSamplesPerSec = 5; 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,13 @@ static void SetEntryPoint(flutter::Settings* settings, NSString* entrypoint, NSS _isGpuDisabled = [UIApplication sharedApplication].applicationState == UIApplicationStateBackground; #endif + // 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 = nil; + } // Create the shell. This is a blocking operation. std::unique_ptr shell = flutter::Shell::Create( diff --git a/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterEngineTest.mm b/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterEngineTest.mm index 5779d4b4961..3b4702269bf 100644 --- a/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterEngineTest.mm +++ b/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterEngineTest.mm @@ -201,17 +201,17 @@ FLUTTER_ASSERT_ARC // 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 @@ FLUTTER_ASSERT_ARC [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]); }