Skip to content

Commit

Permalink
[macOS] Reset keyboard states on engine restart (flutter#29283)
Browse files Browse the repository at this point in the history
With this PR, the state of the keyboard system will be reset when the engine is reset (typically during a hot restart.)
  • Loading branch information
dkwingsmt authored Oct 28, 2021
1 parent 3a5dd99 commit 4c4b773
Show file tree
Hide file tree
Showing 11 changed files with 116 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ private void attach(FlutterNativeView view) {
}

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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
19 changes: 19 additions & 0 deletions shell/platform/darwin/macos/framework/Source/FlutterEngine.mm
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,14 @@ - (void)sendUserLocales;
*/
- (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.
Expand Down Expand Up @@ -296,6 +304,11 @@ - (BOOL)runWithEntrypoint:(NSString*)entrypoint {

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);
Expand Down Expand Up @@ -584,6 +597,12 @@ - (void)engineCallbackOnPlatformMessage:(const FlutterPlatformMessage*)message {
}
}

- (void)engineCallbackOnPreEngineRestart {
if (_viewController) {
[_viewController onPreEngineRestart];
}
}

/**
* Note: Called from dealloc. Should not use accessors or other methods.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ - (instancetype)initWithViewController:(FlutterViewController*)viewController {
[_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;
Expand All @@ -208,6 +208,11 @@ - (BOOL)isFirstResponder {

- (void)dealloc {
[_channel setMethodCallHandler:nil];
if (_textInputContext) {
[_textInputContext deactivate];
[_textInputContext discardMarkedText];
_textInputContext = nil;
}
}

#pragma mark - Private
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,11 @@ - (void)configureTrackingArea;
*/
- (void)addInternalPlugins;

/**
* Creates and registers keyboard related components.
*/
- (void)initializeKeyboard;

/**
* Calls dispatchMouseEvent:phase: with a phase determined by self.mouseState.
*
Expand Down Expand Up @@ -358,6 +363,10 @@ - (void)setMouseTrackingMode:(FlutterMouseTrackingMode)mode {
[self configureTrackingArea];
}

- (void)onPreEngineRestart {
[self initializeKeyboard];
}

#pragma mark - Private methods

- (BOOL)launchEngine {
Expand Down Expand Up @@ -431,9 +440,9 @@ - (void)configureTrackingArea {
}
}

- (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,
Expand All @@ -451,6 +460,12 @@ - (void)addInternalPlugins {
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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ - (bool)testKeyEventsArePropagatedIfNotHandled;
- (bool)testKeyEventsAreNotPropagatedIfHandled;
- (bool)testFlagsChangedEventsArePropagatedIfNotHandled;
- (bool)testPerformKeyEquivalentSynthesizesKeyUp;
- (bool)testKeyboardIsRestartedOnEngineRestart;

+ (void)respondFalseForSendEvent:(const FlutterKeyEvent&)event
callback:(nullable FlutterKeyEventCallback)callback
Expand Down Expand Up @@ -171,6 +172,10 @@ + (void)respondFalseForSendEvent:(const FlutterKeyEvent&)event
ASSERT_TRUE([[FlutterViewControllerTestObjC alloc] testPerformKeyEquivalentSynthesizesKeyUp]);
}

TEST(FlutterViewControllerTest, TestKeyboardIsRestartedOnEngineRestart) {
ASSERT_TRUE([[FlutterViewControllerTestObjC alloc] testKeyboardIsRestartedOnEngineRestart]);
}

} // namespace flutter::testing

@implementation FlutterViewControllerTestObjC
Expand Down Expand Up @@ -456,6 +461,60 @@ - (bool)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 {
Expand Down
2 changes: 1 addition & 1 deletion shell/platform/embedder/embedder.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions shell/platform/linux/fl_engine.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion shell/platform/linux/fl_engine_private.h
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion shell/platform/linux/fl_view.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion shell/platform/windows/flutter_windows_view.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.)
Expand Down

0 comments on commit 4c4b773

Please sign in to comment.