From 79249cf134a08b150d11bdfe1498fae9f326a0e0 Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Wed, 27 Oct 2021 17:03:41 -0700 Subject: [PATCH] [macOS] Reset keyboard states on engine restart (flutter/engine#29283) With this PR, the state of the keyboard system will be reset when the engine is reset (typically during a hot restart.) --- .../io/flutter/view/FlutterNativeView.java | 2 +- .../framework/Headers/FlutterViewController.h | 8 +++ .../macos/framework/Source/FlutterEngine.mm | 19 ++++++ .../Source/FlutterTextInputPlugin.mm | 7 ++- .../framework/Source/FlutterViewController.mm | 19 +++++- .../Source/FlutterViewControllerTest.mm | 59 +++++++++++++++++++ .../shell/platform/embedder/embedder.h | 2 +- .../flutter/shell/platform/linux/fl_engine.cc | 4 +- .../shell/platform/linux/fl_engine_private.h | 2 +- .../flutter/shell/platform/linux/fl_view.cc | 2 +- .../platform/windows/flutter_windows_view.h | 2 +- 11 files changed, 116 insertions(+), 10 deletions(-) diff --git a/engine/src/flutter/shell/platform/android/io/flutter/view/FlutterNativeView.java b/engine/src/flutter/shell/platform/android/io/flutter/view/FlutterNativeView.java index c44a23c998..2cf589cf7e 100644 --- a/engine/src/flutter/shell/platform/android/io/flutter/view/FlutterNativeView.java +++ b/engine/src/flutter/shell/platform/android/io/flutter/view/FlutterNativeView.java @@ -169,7 +169,7 @@ public class FlutterNativeView implements BinaryMessenger { } private final class EngineLifecycleListenerImpl implements EngineLifecycleListener { - // Called by native to notify when the engine is restarted (cold reload). + // Called by native to notify right before the engine is restarted (cold reload). @SuppressWarnings("unused") public void onPreEngineRestart() { if (mFlutterView != null) { diff --git a/engine/src/flutter/shell/platform/darwin/macos/framework/Headers/FlutterViewController.h b/engine/src/flutter/shell/platform/darwin/macos/framework/Headers/FlutterViewController.h index be85081563..2349dad3be 100644 --- a/engine/src/flutter/shell/platform/darwin/macos/framework/Headers/FlutterViewController.h +++ b/engine/src/flutter/shell/platform/darwin/macos/framework/Headers/FlutterViewController.h @@ -53,4 +53,12 @@ FLUTTER_DARWIN_EXPORT NS_DESIGNATED_INITIALIZER; - (nonnull instancetype)initWithCoder:(nonnull NSCoder*)nibNameOrNil NS_DESIGNATED_INITIALIZER; +/** + * Invoked by the engine right before the engine is restarted. + * + * This should reset states to as if the application has just started. It + * usually indicates a hot restart (Shift-R in Flutter CLI.) + */ +- (void)onPreEngineRestart; + @end diff --git a/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterEngine.mm b/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterEngine.mm index 1206a1ce05..c62353b10d 100644 --- a/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterEngine.mm +++ b/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterEngine.mm @@ -75,6 +75,14 @@ static FlutterLocale FlutterLocaleFromNSLocale(NSLocale* locale) { */ - (void)engineCallbackOnPlatformMessage:(const FlutterPlatformMessage*)message; +/** + * Invoked right before the engine is restarted. + * + * This should reset states to as if the application has just started. It + * usually indicates a hot restart (Shift-R in Flutter CLI.) + */ +- (void)engineCallbackOnPreEngineRestart; + /** * Requests that the task be posted back the to the Flutter engine at the target time. The target * time is in the clock used by the Flutter engine. @@ -296,6 +304,11 @@ static void OnPlatformMessage(const FlutterPlatformMessage* message, FlutterEngi flutterArguments.compositor = [self createFlutterCompositor]; + flutterArguments.on_pre_engine_restart_callback = [](void* user_data) { + FlutterEngine* engine = (__bridge FlutterEngine*)user_data; + [engine engineCallbackOnPreEngineRestart]; + }; + FlutterRendererConfig rendererConfig = [_renderer createRendererConfig]; FlutterEngineResult result = _embedderAPI.Initialize( FLUTTER_ENGINE_VERSION, &rendererConfig, &flutterArguments, (__bridge void*)(self), &_engine); @@ -584,6 +597,12 @@ static void OnPlatformMessage(const FlutterPlatformMessage* message, FlutterEngi } } +- (void)engineCallbackOnPreEngineRestart { + if (_viewController) { + [_viewController onPreEngineRestart]; + } +} + /** * Note: Called from dealloc. Should not use accessors or other methods. */ diff --git a/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterTextInputPlugin.mm b/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterTextInputPlugin.mm index 57544a7700..8bdfa8e813 100644 --- a/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterTextInputPlugin.mm +++ b/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterTextInputPlugin.mm @@ -186,7 +186,7 @@ static flutter::TextRange RangeFromBaseExtent(NSNumber* base, [_channel setMethodCallHandler:^(FlutterMethodCall* call, FlutterResult result) { [unsafeSelf handleMethodCall:call result:result]; }]; - _textInputContext = [[NSTextInputContext alloc] initWithClient:self]; + _textInputContext = [[NSTextInputContext alloc] initWithClient:unsafeSelf]; _previouslyPressedFlags = 0; _flutterViewController = viewController; @@ -208,6 +208,11 @@ static flutter::TextRange RangeFromBaseExtent(NSNumber* base, - (void)dealloc { [_channel setMethodCallHandler:nil]; + if (_textInputContext) { + [_textInputContext deactivate]; + [_textInputContext discardMarkedText]; + _textInputContext = nil; + } } #pragma mark - Private diff --git a/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterViewController.mm b/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterViewController.mm index ab9d8833bf..87b41b9e44 100644 --- a/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterViewController.mm +++ b/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterViewController.mm @@ -138,6 +138,11 @@ struct MouseState { */ - (void)addInternalPlugins; +/** + * Creates and registers keyboard related components. + */ +- (void)initializeKeyboard; + /** * Calls dispatchMouseEvent:phase: with a phase determined by self.mouseState. * @@ -358,6 +363,10 @@ static void CommonInit(FlutterViewController* controller) { [self configureTrackingArea]; } +- (void)onPreEngineRestart { + [self initializeKeyboard]; +} + #pragma mark - Private methods - (BOOL)launchEngine { @@ -431,9 +440,9 @@ static void CommonInit(FlutterViewController* controller) { } } -- (void)addInternalPlugins { +- (void)initializeKeyboard { __weak FlutterViewController* weakSelf = self; - [FlutterMouseCursorPlugin registerWithRegistrar:[self registrarForPlugin:@"mousecursor"]]; + _textInputPlugin = [[FlutterTextInputPlugin alloc] initWithViewController:weakSelf]; _keyboardManager = [[FlutterKeyboardManager alloc] initWithOwner:weakSelf]; [_keyboardManager addPrimaryResponder:[[FlutterEmbedderKeyResponder alloc] initWithSendEvent:^(const FlutterKeyEvent& event, @@ -451,6 +460,12 @@ static void CommonInit(FlutterViewController* controller) { codec:[FlutterJSONMessageCodec sharedInstance]]]]; [_keyboardManager addSecondaryResponder:_textInputPlugin]; +} + +- (void)addInternalPlugins { + __weak FlutterViewController* weakSelf = self; + [FlutterMouseCursorPlugin registerWithRegistrar:[self registrarForPlugin:@"mousecursor"]]; + [self initializeKeyboard]; _settingsChannel = [FlutterBasicMessageChannel messageChannelWithName:@"flutter/settings" binaryMessenger:_engine.binaryMessenger diff --git a/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterViewControllerTest.mm b/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterViewControllerTest.mm index 9ba9466178..1726f79f84 100644 --- a/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterViewControllerTest.mm +++ b/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterViewControllerTest.mm @@ -20,6 +20,7 @@ - (bool)testKeyEventsAreNotPropagatedIfHandled; - (bool)testFlagsChangedEventsArePropagatedIfNotHandled; - (bool)testPerformKeyEquivalentSynthesizesKeyUp; +- (bool)testKeyboardIsRestartedOnEngineRestart; + (void)respondFalseForSendEvent:(const FlutterKeyEvent&)event callback:(nullable FlutterKeyEventCallback)callback @@ -171,6 +172,10 @@ TEST(FlutterViewControllerTest, TestPerformKeyEquivalentSynthesizesKeyUp) { ASSERT_TRUE([[FlutterViewControllerTestObjC alloc] testPerformKeyEquivalentSynthesizesKeyUp]); } +TEST(FlutterViewControllerTest, TestKeyboardIsRestartedOnEngineRestart) { + ASSERT_TRUE([[FlutterViewControllerTestObjC alloc] testKeyboardIsRestartedOnEngineRestart]); +} + } // namespace flutter::testing @implementation FlutterViewControllerTestObjC @@ -456,6 +461,60 @@ TEST(FlutterViewControllerTest, TestPerformKeyEquivalentSynthesizesKeyUp) { return true; } +- (bool)testKeyboardIsRestartedOnEngineRestart { + id engineMock = OCMClassMock([FlutterEngine class]); + id binaryMessengerMock = OCMProtocolMock(@protocol(FlutterBinaryMessenger)); + OCMStub( // NOLINT(google-objc-avoid-throwing-exception) + [engineMock binaryMessenger]) + .andReturn(binaryMessengerMock); + __block bool called = false; + __block FlutterKeyEvent last_event; + OCMStub([[engineMock ignoringNonObjectArgs] sendKeyEvent:FlutterKeyEvent {} + callback:nil + userData:nil]) + .andDo((^(NSInvocation* invocation) { + FlutterKeyEvent* event; + [invocation getArgument:&event atIndex:2]; + called = true; + last_event = *event; + })); + + FlutterViewController* viewController = [[FlutterViewController alloc] initWithEngine:engineMock + nibName:@"" + bundle:nil]; + [viewController viewWillAppear]; + NSEvent* keyADown = [NSEvent keyEventWithType:NSEventTypeKeyDown + location:NSZeroPoint + modifierFlags:0x100 + timestamp:0 + windowNumber:0 + context:nil + characters:@"a" + charactersIgnoringModifiers:@"a" + isARepeat:FALSE + keyCode:0]; + const uint64_t kPhysicalKeyA = 0x70004; + + // Send KeyA key down event twice. Without restarting the keyboard during + // onPreEngineRestart, the second event received will be an empty event with + // physical key 0x0 because duplicate key down events are ignored. + + called = false; + [viewController keyDown:keyADown]; + EXPECT_TRUE(called); + EXPECT_EQ(last_event.type, kFlutterKeyEventTypeDown); + EXPECT_EQ(last_event.physical, kPhysicalKeyA); + + [viewController onPreEngineRestart]; + + called = false; + [viewController keyDown:keyADown]; + EXPECT_TRUE(called); + EXPECT_EQ(last_event.type, kFlutterKeyEventTypeDown); + EXPECT_EQ(last_event.physical, kPhysicalKeyA); + return true; +} + + (void)respondFalseForSendEvent:(const FlutterKeyEvent&)event callback:(nullable FlutterKeyEventCallback)callback userData:(nullable void*)userData { diff --git a/engine/src/flutter/shell/platform/embedder/embedder.h b/engine/src/flutter/shell/platform/embedder/embedder.h index 473724144a..bbd58e40f0 100644 --- a/engine/src/flutter/shell/platform/embedder/embedder.h +++ b/engine/src/flutter/shell/platform/embedder/embedder.h @@ -1580,7 +1580,7 @@ typedef struct { // callbacks on `log_message_callback`. Defaults to "flutter" if unspecified. const char* log_tag; - // A callback that is invoked when the engine is restarted. + // A callback that is invoked right before the engine is restarted. // // This optional callback is typically used to reset states to as if the // engine has just been started, and usually indicates the user has requested diff --git a/engine/src/flutter/shell/platform/linux/fl_engine.cc b/engine/src/flutter/shell/platform/linux/fl_engine.cc index bcd6749609..9f42b6bd07 100644 --- a/engine/src/flutter/shell/platform/linux/fl_engine.cc +++ b/engine/src/flutter/shell/platform/linux/fl_engine.cc @@ -48,7 +48,7 @@ struct _FlEngine { gpointer update_semantics_node_handler_data; GDestroyNotify update_semantics_node_handler_destroy_notify; - // Function to call when the engine is restarted. + // Function to call right before the engine is restarted. FlEngineOnPreEngineRestartHandler on_pre_engine_restart_handler; gpointer on_pre_engine_restart_handler_data; GDestroyNotify on_pre_engine_restart_handler_destroy_notify; @@ -284,7 +284,7 @@ static void fl_engine_update_semantics_node_cb(const FlutterSemanticsNode* node, } } -// Called when the engine is restarted. +// Called right before the engine is restarted. // // This method should reset states to as if the engine has just been started, // which usually indicates the user has requested a hot restart (Shift-R in the diff --git a/engine/src/flutter/shell/platform/linux/fl_engine_private.h b/engine/src/flutter/shell/platform/linux/fl_engine_private.h index a576a64e96..15197ad836 100644 --- a/engine/src/flutter/shell/platform/linux/fl_engine_private.h +++ b/engine/src/flutter/shell/platform/linux/fl_engine_private.h @@ -133,7 +133,7 @@ void fl_engine_set_update_semantics_node_handler( * @destroy_notify: (allow-none): a function which gets called to free * @user_data, or %NULL. * - * Registers the function called when the engine is restarted. + * Registers the function called right before the engine is restarted. */ void fl_engine_set_on_pre_engine_restart_handler( FlEngine* engine, diff --git a/engine/src/flutter/shell/platform/linux/fl_view.cc b/engine/src/flutter/shell/platform/linux/fl_view.cc index 1fbdd63334..dca1a21079 100644 --- a/engine/src/flutter/shell/platform/linux/fl_view.cc +++ b/engine/src/flutter/shell/platform/linux/fl_view.cc @@ -102,7 +102,7 @@ static void fl_view_init_keyboard(FlView* self) { FL_KEY_RESPONDER(fl_key_channel_responder_new(messenger))); } -// Called when the engine is restarted. +// Invoked by the engine right before the engine is restarted. // // This method should reset states to be as if the engine had just been started, // which usually indicates the user has requested a hot restart (Shift-R in the diff --git a/engine/src/flutter/shell/platform/windows/flutter_windows_view.h b/engine/src/flutter/shell/platform/windows/flutter_windows_view.h index 84875d603a..95ccbbfd1a 100644 --- a/engine/src/flutter/shell/platform/windows/flutter_windows_view.h +++ b/engine/src/flutter/shell/platform/windows/flutter_windows_view.h @@ -90,7 +90,7 @@ class FlutterWindowsView : public WindowBindingHandlerDelegate, // Returns the frame buffer id for the engine to render to. uint32_t GetFrameBufferId(size_t width, size_t height); - // Called when the engine is restarted. + // Invoked by the engine right before the engine is restarted. // // This should reset necessary states to as if the view has just been // created. This is typically caused by a hot restart (Shift-R in CLI.)