From f84f9c92b592bdada5d9ff218dc9325aa269ee37 Mon Sep 17 00:00:00 2001 From: "auto-submit[bot]" <98614782+auto-submit[bot]@users.noreply.github.com> Date: Sat, 7 Sep 2024 02:56:46 +0000 Subject: [PATCH] Reverts "[engine] reland: always post tasks for platform channel responses. (#55006)" (flutter/engine#55022) Reverts: flutter/engine#55006 Initiated by: jonahwilliams Reason for reverting: still failling mac module test Original PR Author: jonahwilliams Reviewed By: {jason-simmons} This change reverts the following previous change: 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, 18 insertions(+), 40 deletions(-) diff --git a/engine/src/flutter/shell/common/engine.cc b/engine/src/flutter/shell/common/engine.cc index 37f71f4496d..ac628eb9294 100644 --- a/engine/src/flutter/shell/common/engine.cc +++ b/engine/src/flutter/shell/common/engine.cc @@ -61,7 +61,6 @@ 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 8e0108eaccc..7a822c5dca1 100644 --- a/engine/src/flutter/shell/common/engine_unittests.cc +++ b/engine/src/flutter/shell/common/engine_unittests.cc @@ -252,27 +252,6 @@ 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 a23bc5498ef..79c532640a9 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) - // 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 { + fml::TaskRunner::RunNowOrPostTask( + task_runners_.GetUITaskRunner(), + 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 3b65f469820..b1ca5e66ae6 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,6 +602,13 @@ 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 @@ -897,13 +904,6 @@ 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 3b4702269bf..5779d4b4961 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"]; - // Initial route is set directly in the shell/engine and should not send a platform - // channel message as it will arrive too late. + // Now check that an encoded method call has been made on the binary messenger to set the + // initial route to "test". FlutterMethodCall* setInitialRouteMethodCall = [FlutterMethodCall methodCallWithMethodName:@"setInitialRoute" arguments:@"test"]; NSData* encodedSetInitialRouteMethod = [[FlutterJSONMethodCodec sharedInstance] encodeMethodCall:setInitialRouteMethodCall]; - OCMReject([mockBinaryMessenger sendOnChannel:@"flutter/navigation" + OCMVerify([mockBinaryMessenger sendOnChannel:@"flutter/navigation" message:encodedSetInitialRouteMethod]); } -- (void)testInitialRouteSettingsDoesNotSendNavigationMessage { +- (void)testInitialRouteSettingsSendsNavigationMessage { id mockBinaryMessenger = OCMClassMock([FlutterBinaryMessengerRelay class]); auto settings = FLTDefaultSettingsForBundle(); @@ -221,13 +221,13 @@ FLUTTER_ASSERT_ARC [engine setBinaryMessenger:mockBinaryMessenger]; [engine run]; - // Initial route is set directly in the shell/engine and should not send a platform - // channel message as it will arrive too late. + // Now check that an encoded method call has been made on the binary messenger to set the + // initial route to "test". FlutterMethodCall* setInitialRouteMethodCall = [FlutterMethodCall methodCallWithMethodName:@"setInitialRoute" arguments:@"test"]; NSData* encodedSetInitialRouteMethod = [[FlutterJSONMethodCodec sharedInstance] encodeMethodCall:setInitialRouteMethodCall]; - OCMReject([mockBinaryMessenger sendOnChannel:@"flutter/navigation" + OCMVerify([mockBinaryMessenger sendOnChannel:@"flutter/navigation" message:encodedSetInitialRouteMethod]); }