Made FlutterTextField that outlive FlutterTextPlatformNode not crash (flutter/engine#37735)

* Made FlutterTextField that outlive FlutterTextPlatformNode not crash. * added test
2022-11-18 13:08:01 -08:00
parent 8a1834c4d8
commit 1ac0bf4389
3 changed files with 95 additions and 58 deletions
--- a/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterTextInputPluginTest.mm
+++ b/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterTextInputPluginTest.mm
@@ -12,6 +12,10 @@
 #import <OCMock/OCMock.h>
 #import "flutter/testing/testing.h"

+@interface FlutterTextField (Testing)
+- (void)setPlatformNode:(flutter::FlutterTextPlatformNode*)node;
+@end
+
@interface FlutterTextFieldMock : FlutterTextField

@property(nonatomic) NSString* lastUpdatedString;
@@ -1434,37 +1438,47 @@ TEST(FlutterTextInputPluginTest, CanWorkWithFlutterTextField) {
  node_data.SetValue("initial text");
  ax_node.SetData(node_data);
  delegate.Init(engine.accessibilityBridge, &ax_node);
-  FlutterTextPlatformNode text_platform_node(&delegate, viewController);
+  {
+    FlutterTextPlatformNode text_platform_node(&delegate, viewController);

-  FlutterTextFieldMock* mockTextField =
-      [[FlutterTextFieldMock alloc] initWithPlatformNode:&text_platform_node
-                                             fieldEditor:viewController.textInputPlugin];
-  [viewController.view addSubview:mockTextField];
-  [mockTextField startEditing];
+    FlutterTextFieldMock* mockTextField =
+        [[FlutterTextFieldMock alloc] initWithPlatformNode:&text_platform_node
+                                               fieldEditor:viewController.textInputPlugin];
+    [viewController.view addSubview:mockTextField];
+    [mockTextField startEditing];

-  NSDictionary* arguments = @{
-    @"inputAction" : @"action",
-    @"inputType" : @{@"name" : @"inputName"},
-  };
-  FlutterMethodCall* methodCall = [FlutterMethodCall methodCallWithMethodName:@"TextInput.setClient"
-                                                                    arguments:@[ @(1), arguments ]];
-  FlutterResult result = ^(id result) {
-  };
-  [viewController.textInputPlugin handleMethodCall:methodCall result:result];
+    NSDictionary* arguments = @{
+      @"inputAction" : @"action",
+      @"inputType" : @{@"name" : @"inputName"},
+    };
+    FlutterMethodCall* methodCall =
+        [FlutterMethodCall methodCallWithMethodName:@"TextInput.setClient"
+                                          arguments:@[ @(1), arguments ]];
+    FlutterResult result = ^(id result) {
+    };
+    [viewController.textInputPlugin handleMethodCall:methodCall result:result];

-  arguments = @{
-    @"text" : @"new text",
-    @"selectionBase" : @(1),
-    @"selectionExtent" : @(2),
-    @"composingBase" : @(-1),
-    @"composingExtent" : @(-1),
-  };
+    arguments = @{
+      @"text" : @"new text",
+      @"selectionBase" : @(1),
+      @"selectionExtent" : @(2),
+      @"composingBase" : @(-1),
+      @"composingExtent" : @(-1),
+    };

-  methodCall = [FlutterMethodCall methodCallWithMethodName:@"TextInput.setEditingState"
-                                                 arguments:arguments];
-  [viewController.textInputPlugin handleMethodCall:methodCall result:result];
-  EXPECT_EQ([mockTextField.lastUpdatedString isEqualToString:@"new text"], YES);
-  EXPECT_EQ(NSEqualRanges(mockTextField.lastUpdatedSelection, NSMakeRange(1, 1)), YES);
+    methodCall = [FlutterMethodCall methodCallWithMethodName:@"TextInput.setEditingState"
+                                                   arguments:arguments];
+    [viewController.textInputPlugin handleMethodCall:methodCall result:result];
+    EXPECT_EQ([mockTextField.lastUpdatedString isEqualToString:@"new text"], YES);
+    EXPECT_EQ(NSEqualRanges(mockTextField.lastUpdatedSelection, NSMakeRange(1, 1)), YES);
+
+    // This blocks the FlutterTextFieldMock, which is held onto by the main event
+    // loop, from crashing.
+    [mockTextField setPlatformNode:nil];
+  }
+
+  // This verifies that clearing the platform node works.
+  [[NSRunLoop currentRunLoop] runUntilDate:[NSDate dateWithTimeIntervalSinceNow:0.1]];
 }

 TEST(FlutterTextInputPluginTest, CanNotBecomeResponderIfNoViewController) {
--- a/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterTextInputSemanticsObject.mm
+++ b/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterTextInputSemanticsObject.mm
@@ -91,12 +91,18 @@
 #pragma mark - NSView

 - (NSRect)frame {
+  if (!_node) {
+    return NSZeroRect;
+  }
  return _node->GetFrame();
 }

 #pragma mark - NSAccessibilityProtocol

 - (void)setAccessibilityFocused:(BOOL)isFocused {
+  if (!_node) {
+    return;
+  }
  [super setAccessibilityFocused:isFocused];
  ui::AXActionData data;
  data.action = isFocused ? ax::mojom::Action::kFocus : ax::mojom::Action::kBlur;
@@ -110,6 +116,9 @@
  if (self.currentEditor == _plugin) {
    return;
  }
+  if (!_node) {
+    return;
+  }
  // Selecting text seems to be the only way to make the field editor
  // current editor.
  [self selectText:self];
@@ -133,6 +142,10 @@
  [self updateString:textValue withSelection:selection];
 }

+- (void)setPlatformNode:(flutter::FlutterTextPlatformNode*)node {
+  _node = node;
+}
+
 #pragma mark - NSObject

 - (void)dealloc {
@@ -159,6 +172,7 @@ FlutterTextPlatformNode::FlutterTextPlatformNode(FlutterPlatformNodeDelegate* de
 }

 FlutterTextPlatformNode::~FlutterTextPlatformNode() {
+  [appkit_text_field_ setPlatformNode:nil];
  EnsureDetachedFromView();
 }

--- a/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterTextInputSemanticsObjectTest.mm
+++ b/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterTextInputSemanticsObjectTest.mm
@@ -27,39 +27,48 @@ FlutterEngine* CreateTestEngine() {

 TEST(FlutterTextInputSemanticsObjectTest, DoesInitialize) {
  FlutterEngine* engine = CreateTestEngine();
-  NSString* fixtures = @(testing::GetFixturesPath());
-  FlutterDartProject* project = [[FlutterDartProject alloc]
-      initWithAssetsPath:fixtures
-             ICUDataPath:[fixtures stringByAppendingString:@"/icudtl.dat"]];
-  FlutterViewController* viewController = [[FlutterViewController alloc] initWithProject:project];
-  [viewController loadView];
-  [engine setViewController:viewController];
-  // Create a NSWindow so that the native text field can become first responder.
-  NSWindow* window = [[NSWindow alloc] initWithContentRect:NSMakeRect(0, 0, 800, 600)
-                                                 styleMask:NSBorderlessWindowMask
-                                                   backing:NSBackingStoreBuffered
-                                                     defer:NO];
-  window.contentView = viewController.view;
+  {
+    NSString* fixtures = @(testing::GetFixturesPath());
+    FlutterDartProject* project = [[FlutterDartProject alloc]
+        initWithAssetsPath:fixtures
+               ICUDataPath:[fixtures stringByAppendingString:@"/icudtl.dat"]];
+    FlutterViewController* viewController = [[FlutterViewController alloc] initWithProject:project];
+    [viewController loadView];
+    [engine setViewController:viewController];
+    // Create a NSWindow so that the native text field can become first responder.
+    NSWindow* window = [[NSWindow alloc] initWithContentRect:NSMakeRect(0, 0, 800, 600)
+                                                   styleMask:NSBorderlessWindowMask
+                                                     backing:NSBackingStoreBuffered
+                                                       defer:NO];
+    window.contentView = viewController.view;

-  engine.semanticsEnabled = YES;
+    engine.semanticsEnabled = YES;

-  auto bridge = engine.accessibilityBridge.lock();
-  FlutterPlatformNodeDelegateMac delegate(bridge, viewController);
-  ui::AXTree tree;
-  ui::AXNode ax_node(&tree, nullptr, 0, 0);
-  ui::AXNodeData node_data;
-  node_data.SetValue("initial text");
-  ax_node.SetData(node_data);
-  delegate.Init(engine.accessibilityBridge, &ax_node);
-  // Verify that a FlutterTextField is attached to the view.
-  FlutterTextPlatformNode text_platform_node(&delegate, viewController);
-  id native_accessibility = text_platform_node.GetNativeViewAccessible();
-  EXPECT_TRUE([native_accessibility isKindOfClass:[FlutterTextField class]]);
-  auto subviews = [viewController.view subviews];
-  EXPECT_EQ([subviews count], 2u);
-  EXPECT_TRUE([subviews[0] isKindOfClass:[FlutterTextField class]]);
-  FlutterTextField* nativeTextField = subviews[0];
-  EXPECT_EQ(text_platform_node.GetNativeViewAccessible(), nativeTextField);
+    auto bridge = engine.accessibilityBridge.lock();
+    FlutterPlatformNodeDelegateMac delegate(bridge, viewController);
+    ui::AXTree tree;
+    ui::AXNode ax_node(&tree, nullptr, 0, 0);
+    ui::AXNodeData node_data;
+    node_data.SetValue("initial text");
+    ax_node.SetData(node_data);
+    delegate.Init(engine.accessibilityBridge, &ax_node);
+    // Verify that a FlutterTextField is attached to the view.
+    FlutterTextPlatformNode text_platform_node(&delegate, viewController);
+    id native_accessibility = text_platform_node.GetNativeViewAccessible();
+    EXPECT_TRUE([native_accessibility isKindOfClass:[FlutterTextField class]]);
+    auto subviews = [viewController.view subviews];
+    EXPECT_EQ([subviews count], 2u);
+    EXPECT_TRUE([subviews[0] isKindOfClass:[FlutterTextField class]]);
+    FlutterTextField* nativeTextField = subviews[0];
+    EXPECT_EQ(text_platform_node.GetNativeViewAccessible(), nativeTextField);
+  }
+
+  [engine shutDownEngine];
+  engine = nil;
+  // Pump the event loop to make sure no stray nodes cause crashes after the
+  // engine has been destroyed.
+  // From issue: https://github.com/flutter/flutter/issues/115599
+  [[NSRunLoop currentRunLoop] runUntilDate:[NSDate dateWithTimeIntervalSinceNow:0.1]];
 }

 }  // namespace flutter::testing