addressing comments

Author: chunhtai

shell/platform/darwin/macos/framework/Source/FlutterPlatformNodeDelegateMacTest.mm

@@ -280,9 +280,9 @@
   double pixelRatio = view.bounds.size.width == 0 ? 1 : scaledSize.width / view.bounds.size.width;
 
   double expectedFrameSize = rectSize * transformFactor / pixelRatio;
-  EXPECT_EQ(
-      NSEqualRects(native_text_field.frame, NSMakeRect(0, 0, expectedFrameSize, expectedFrameSize)),
-      YES);
+  EXPECT_EQ(NSEqualRects(native_text_field.frame, NSMakeRect(0, 600 - expectedFrameSize,
+                                                             expectedFrameSize, expectedFrameSize)),
+            YES);
   // The text of TextInputPlugin only starts syncing editing state to the
   // native text field when it becomes the first responder.
   [native_text_field becomeFirstResponder];

shell/platform/darwin/macos/framework/Source/FlutterTextInputPlugin.mm

@@ -356,7 +356,7 @@ - (void)updateTextAndSelection {
   int extend = _activeModel->selection().extent();
   NSRange selection = NSMakeRange(MIN(start, extend), ABS(start - extend));
   // There may be a native text field client if VoiceOver is on.
-  // In this case, this plugin have to update text and selection through
+  // In this case, this plugin has to update text and selection through
   // the client in order for VoiceOver to announce the text editing
   // properly.
   if (_client) {

shell/platform/darwin/macos/framework/Source/FlutterTextInputPluginTest.mm

@@ -228,8 +228,7 @@ - (bool)testFirstRectForCharacterRange {
   FlutterTextPlatformNode textPlatformNode(&delegate, engine);
 
   FlutterTextFieldMock* mockTextField =
-      [[FlutterTextFieldMock alloc] initWithPlatformNode:&textPlatformNode];
-  [mockTextField setFieldEditor:viewController.textInputPlugin];
+      [[FlutterTextFieldMock alloc] initWithPlatformNode:&textPlatformNode andEngine:engine];
   [viewController.view addSubview:mockTextField];
   [mockTextField becomeFirstResponder];
 
@@ -260,4 +259,39 @@ - (bool)testFirstRectForCharacterRange {
   EXPECT_EQ(NSEqualRanges(mockTextField.lastUpdatedSelection, NSMakeRange(1, 1)), YES);
 }
 
+TEST(FlutterTextInputPluginTest, CanNotBecomeResponderIfNoViewController) {
+  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];
+  // Creates 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;
+
+  auto bridge = engine.accessibilityBridge.lock();
+  FlutterPlatformNodeDelegateMac delegate(engine);
+  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);
+  FlutterTextPlatformNode textPlatformNode(&delegate, engine);
+
+  FlutterTextField* textField = textPlatformNode.GetNativeViewAccessible();
+  EXPECT_EQ([textField becomeFirstResponder], YES);
+  // Removes view controller.
+  [engine setViewController:nil];
+  EXPECT_EQ([textField becomeFirstResponder], NO);
+}
+
 }  // namespace flutter::testing

shell/platform/darwin/macos/framework/Source/FlutterTextInputSemanticsObject.h

@@ -69,24 +69,16 @@ class FlutterTextPlatformNode : public ui::AXPlatformNodeBase {
  * This causes VoiceOver to announce text editing feedbacks.
  *
  * All of the text editing events in this native text field are redirected to the
- * FlutterTextInputPlugin and will eventually update this text field through text input channel.
+ * text input plugin returned by the FlutterTextFieldDelegate.
  */
 @interface FlutterTextField : NSTextField
 
 /**
- * Initializes a FlutterTextField.
- *
- * The plugin will be stored as an __unsafe_unretained pointer, and the FlutterTextField expects
- * the point is valid during its lifetime.
- */
-- (instancetype)initWithPlatformNode:(flutter::FlutterTextPlatformNode*)node;
-
-/**
- * Assign the field editor for this FlutterTextField. The plugin will be stored as an
- * __unsafe_unretained pointer, and the FlutterTextField expects the pointer is valid
- * during its lifetime.
+ * Initializes a FlutterTextField that uses the textInputPlugin from the FlutterTextFieldDelegate
+ * as its field editor.
  */
-- (void)setFieldEditor:(FlutterTextInputPlugin*)plugin;
+- (instancetype)initWithPlatformNode:(flutter::FlutterTextPlatformNode*)node
+                           andEngine:(FlutterEngine*)engine;
 
 /**
  * Updates the string value and the selection of this text field.

shell/platform/darwin/macos/framework/Source/FlutterTextInputSemanticsObject.mm

@@ -14,37 +14,28 @@
 
 #pragma mark - FlutterTextFieldCell
 /**
- * A convenient class that can be used to set a custom field editor for a
- * NSTextField.
- *
- * The FlutterTextField uses this class set the FlutterTextInputPlugin as
- * its field editor.
+ * The NSCell for FlutterTextField that it sets the field editor of FlutterTextField
+ * to the text input plugin from the FlutterTextFieldDelegate.
  */
 @interface FlutterTextFieldCell : NSTextFieldCell
 
 /**
  * Initializes the NSCell for the input NSTextField.
  */
-- (instancetype)initWithTextField:(NSTextField*)textField;
-
-/**
- * Assign the field editor to be used by the NSTextField.
- */
-- (void)setFieldEditor:(FlutterTextInputPlugin*)editor;
+- (instancetype)initWithTextField:(FlutterTextField*)textField andEngine:(FlutterEngine*)engine;
 
 @end
 
 @implementation FlutterTextFieldCell {
-  // NSTextView does not support _weak reference, so this class stores
-  // __unsafe_unretained.
-  __unsafe_unretained NSTextView* _editor;
+  __weak FlutterEngine* _engine;
 }
 
 #pragma mark - Private
 
-- (instancetype)initWithTextField:(NSTextField*)textField {
+- (instancetype)initWithTextField:(FlutterTextField*)textField andEngine:(FlutterEngine*)engine {
   self = [super initTextCell:textField.stringValue];
   if (self) {
+    _engine = engine;
     [self setControlView:textField];
     // Read only text fields are sent to the mac embedding as static
     // texts. This text field must be editable and selectable at this
@@ -55,14 +46,10 @@ - (instancetype)initWithTextField:(NSTextField*)textField {
   return self;
 }
 
-- (void)setFieldEditor:(FlutterTextInputPlugin*)editor {
-  _editor = editor;
-}
-
 #pragma mark - NSCell
 
 - (NSTextView*)fieldEditorForView:(NSView*)controlView {
-  return _editor;
+  return _engine.viewController.textInputPlugin;
 }
 
 @end
@@ -71,38 +58,31 @@ - (NSTextView*)fieldEditorForView:(NSView*)controlView {
 
 @implementation FlutterTextField {
   flutter::FlutterTextPlatformNode* _node;
-  // NSTextView does not support _weak reference, so this class holds
-  // __unsafe_unretained.
-  __unsafe_unretained FlutterTextInputPlugin* _plugin;
+  __weak FlutterEngine* _engine;
 }
 
 #pragma mark - Public
 
-- (instancetype)initWithPlatformNode:(flutter::FlutterTextPlatformNode*)node {
+- (instancetype)initWithPlatformNode:(flutter::FlutterTextPlatformNode*)node
+                           andEngine:(FlutterEngine*)engine {
   self = [super initWithFrame:NSZeroRect];
   if (self) {
     _node = node;
-    [self setCell:[[FlutterTextFieldCell alloc] initWithTextField:self]];
+    _engine = engine;
+    [self setCell:[[FlutterTextFieldCell alloc] initWithTextField:self andEngine:engine]];
   }
   return self;
 }
 
-- (void)setFieldEditor:(FlutterTextInputPlugin*)plugin {
-  _plugin = plugin;
-  NSAssert([self.cell isKindOfClass:[FlutterTextFieldCell class]],
-           @"The cell of a FlutterTextField must be a FlutterTextFieldCell");
-  FlutterTextFieldCell* cell = (FlutterTextFieldCell*)self.cell;
-  [cell setFieldEditor:plugin];
-}
-
 - (void)updateString:(NSString*)string withSelection:(NSRange)selection {
-  NSAssert(_plugin.client == self,
+  FlutterTextInputPlugin* plugin = _engine.viewController.textInputPlugin;
+  NSAssert(plugin.client == self,
            @"Can't update FlutterTextField when it is not the first responder");
   if (![[self stringValue] isEqualToString:string]) {
     [self setStringValue:string];
   }
-  if (!NSEqualRanges(_plugin.selectedRange, selection)) {
-    [_plugin setSelectedRange:selection];
+  if (!NSEqualRanges(plugin.selectedRange, selection)) {
+    [plugin setSelectedRange:selection];
   }
 }
 
@@ -124,13 +104,17 @@ - (void)setAccessibilityFocused:(BOOL)isFocused {
 #pragma mark - NSResponder
 
 - (BOOL)becomeFirstResponder {
-  if (_plugin.client == self && [_plugin isFirstResponder]) {
+  FlutterTextInputPlugin* plugin = _engine.viewController.textInputPlugin;
+  if (!plugin) {
+    return NO;
+  }
+  if (plugin.client == self && [plugin isFirstResponder]) {
     // This text field is already the first responder.
     return YES;
   }
   BOOL result = [super becomeFirstResponder];
   if (result) {
-    _plugin.client = self;
+    plugin.client = self;
     // The default implementation of the becomeFirstResponder will change the
     // text editing state. Need to manually set it back.
     NSString* textValue = @(_node->GetStringAttribute(ax::mojom::StringAttribute::kValue).data());
@@ -155,16 +139,14 @@ - (BOOL)becomeFirstResponder {
                                                  FlutterEngine* engine) {
   Init(delegate);
   engine_ = engine;
-  native_text_field_ = [[FlutterTextField alloc] initWithPlatformNode:this];
-  [native_text_field_ setFieldEditor:engine.viewController.textInputPlugin];
+  native_text_field_ = [[FlutterTextField alloc] initWithPlatformNode:this andEngine:engine];
   native_text_field_.bezeled = NO;
   native_text_field_.drawsBackground = NO;
   native_text_field_.bordered = NO;
   native_text_field_.focusRingType = NSFocusRingTypeNone;
 }
 
 FlutterTextPlatformNode::~FlutterTextPlatformNode() {
-  [native_text_field_ setFieldEditor:nil];
   EnsureDetachedFromView();
 }
 
@@ -176,6 +158,9 @@ - (BOOL)becomeFirstResponder {
 }
 
 NSRect FlutterTextPlatformNode::GetFrameRelativeToControllerView() {
+  if (!engine_.viewController.viewLoaded) {
+    return NSZeroRect;
+  }
   FlutterPlatformNodeDelegate* delegate = static_cast<FlutterPlatformNodeDelegate*>(GetDelegate());
   bool offscreen;
   auto bridge_ptr = delegate->GetOwnerBridge().lock();
@@ -188,8 +173,9 @@ - (BOOL)becomeFirstResponder {
   // increasing to bottom-right. Flip the y coordinate to convert from Flutter
   // coordinates to macOS coordinates.
   ns_local_bounds.origin.y = -ns_local_bounds.origin.y - ns_local_bounds.size.height;
-  return [engine_.viewController.flutterView convertRectFromBacking:ns_local_bounds];
-  ;
+  NSRect ns_view_bounds =
+      [engine_.viewController.flutterView convertRectFromBacking:ns_local_bounds];
+  return [engine_.viewController.flutterView convertRect:ns_view_bounds toView:nil];
 }
 
 bool FlutterTextPlatformNode::EnsureAttachedToView() {