Ensure that PlatformViewIOS does not call into Shell semantics APIs during destruction (flutter#30835)

Author: jason-simmons

shell/platform/darwin/ios/framework/Source/accessibility_bridge_test.mm

@@ -1664,4 +1664,51 @@ - (void)testFlutterSemanticsScrollViewManagedObjectLifecycleCorrectly {
   // flutterSemanticsScrollView) will cause an EXC_BAD_ACCESS.
   XCTAssertFalse([flutterSemanticsScrollView isAccessibilityElement]);
 }
+
+- (void)testPlatformViewDestructorDoesNotCallSemanticsAPIs {
+  class TestDelegate : public flutter::MockDelegate {
+   public:
+    void OnPlatformViewSetSemanticsEnabled(bool enabled) override { set_semantics_enabled_calls++; }
+    int set_semantics_enabled_calls = 0;
+  };
+
+  TestDelegate test_delegate;
+  auto thread = std::make_unique<fml::Thread>("AccessibilityBridgeTest");
+  auto thread_task_runner = thread->GetTaskRunner();
+  flutter::TaskRunners runners(/*label=*/self.name.UTF8String,
+                               /*platform=*/thread_task_runner,
+                               /*raster=*/thread_task_runner,
+                               /*ui=*/thread_task_runner,
+                               /*io=*/thread_task_runner);
+
+  fml::AutoResetWaitableEvent latch;
+  thread_task_runner->PostTask([&] {
+    auto platform_view = std::make_unique<flutter::PlatformViewIOS>(
+        /*delegate=*/test_delegate,
+        /*rendering_api=*/flutter::IOSRenderingAPI::kSoftware,
+        /*platform_views_controller=*/nil,
+        /*task_runners=*/runners);
+
+    id mockFlutterViewController = OCMClassMock([FlutterViewController class]);
+    auto flutterPlatformViewsController =
+        std::make_shared<flutter::FlutterPlatformViewsController>();
+    OCMStub([mockFlutterViewController platformViewsController])
+        .andReturn(flutterPlatformViewsController.get());
+    auto weakFactory =
+        std::make_unique<fml::WeakPtrFactory<FlutterViewController>>(mockFlutterViewController);
+    platform_view->SetOwnerViewController(weakFactory->GetWeakPtr());
+
+    platform_view->SetSemanticsEnabled(true);
+    XCTAssertNotEqual(test_delegate.set_semantics_enabled_calls, 0);
+
+    // Deleting PlatformViewIOS should not call OnPlatformViewSetSemanticsEnabled
+    test_delegate.set_semantics_enabled_calls = 0;
+    platform_view.reset();
+    XCTAssertEqual(test_delegate.set_semantics_enabled_calls, 0);
+
+    latch.Signal();
+  });
+  latch.Wait();
+}
+
 @end

shell/platform/darwin/ios/platform_view_ios.h

@@ -113,20 +113,20 @@ class PlatformViewIOS final : public PlatformView {
     id<NSObject> observer_;
   };
 
-  /// Smart pointer that guarantees we communicate clearing Accessibility
+  /// Wrapper that guarantees we communicate clearing Accessibility
   /// information to Dart.
-  class AccessibilityBridgePtr {
+  class AccessibilityBridgeManager {
    public:
-    explicit AccessibilityBridgePtr(const std::function<void(bool)>& set_semantics_enabled);
-    AccessibilityBridgePtr(const std::function<void(bool)>& set_semantics_enabled,
-                           AccessibilityBridge* bridge);
-    ~AccessibilityBridgePtr();
+    explicit AccessibilityBridgeManager(const std::function<void(bool)>& set_semantics_enabled);
+    AccessibilityBridgeManager(const std::function<void(bool)>& set_semantics_enabled,
+                               AccessibilityBridge* bridge);
     explicit operator bool() const noexcept { return static_cast<bool>(accessibility_bridge_); }
-    AccessibilityBridge* operator->() const noexcept { return accessibility_bridge_.get(); }
-    void reset(AccessibilityBridge* bridge = nullptr);
+    AccessibilityBridge* get() const noexcept { return accessibility_bridge_.get(); }
+    void Set(std::unique_ptr<AccessibilityBridge> bridge);
+    void Clear();
 
    private:
-    FML_DISALLOW_COPY_AND_ASSIGN(AccessibilityBridgePtr);
+    FML_DISALLOW_COPY_AND_ASSIGN(AccessibilityBridgeManager);
     std::unique_ptr<AccessibilityBridge> accessibility_bridge_;
     std::function<void(bool)> set_semantics_enabled_;
   };
@@ -138,7 +138,7 @@ class PlatformViewIOS final : public PlatformView {
   std::unique_ptr<IOSSurface> ios_surface_;
   std::shared_ptr<IOSContext> ios_context_;
   const std::shared_ptr<FlutterPlatformViewsController>& platform_views_controller_;
-  AccessibilityBridgePtr accessibility_bridge_;
+  AccessibilityBridgeManager accessibility_bridge_;
   fml::scoped_nsprotocol<FlutterTextInputPlugin*> text_input_plugin_;
   fml::closure firstFrameCallback_;
   ScopedObserver dealloc_view_controller_observer_;

shell/platform/darwin/ios/platform_view_ios.mm

@@ -16,11 +16,11 @@
 
 namespace flutter {
 
-PlatformViewIOS::AccessibilityBridgePtr::AccessibilityBridgePtr(
+PlatformViewIOS::AccessibilityBridgeManager::AccessibilityBridgeManager(
     const std::function<void(bool)>& set_semantics_enabled)
-    : AccessibilityBridgePtr(set_semantics_enabled, nullptr) {}
+    : AccessibilityBridgeManager(set_semantics_enabled, nullptr) {}
 
-PlatformViewIOS::AccessibilityBridgePtr::AccessibilityBridgePtr(
+PlatformViewIOS::AccessibilityBridgeManager::AccessibilityBridgeManager(
     const std::function<void(bool)>& set_semantics_enabled,
     AccessibilityBridge* bridge)
     : accessibility_bridge_(bridge), set_semantics_enabled_(set_semantics_enabled) {
@@ -29,20 +29,14 @@
   }
 }
 
-PlatformViewIOS::AccessibilityBridgePtr::~AccessibilityBridgePtr() {
-  if (accessibility_bridge_) {
-    set_semantics_enabled_(false);
-  }
+void PlatformViewIOS::AccessibilityBridgeManager::Set(std::unique_ptr<AccessibilityBridge> bridge) {
+  accessibility_bridge_ = std::move(bridge);
+  set_semantics_enabled_(true);
 }
 
-void PlatformViewIOS::AccessibilityBridgePtr::reset(AccessibilityBridge* bridge) {
-  if (accessibility_bridge_) {
-    set_semantics_enabled_(false);
-  }
-  accessibility_bridge_.reset(bridge);
-  if (accessibility_bridge_) {
-    set_semantics_enabled_(true);
-  }
+void PlatformViewIOS::AccessibilityBridgeManager::Clear() {
+  set_semantics_enabled_(false);
+  accessibility_bridge_.reset();
 }
 
 PlatformViewIOS::PlatformViewIOS(
@@ -83,7 +77,7 @@
   if (ios_surface_ || !owner_controller) {
     NotifyDestroyed();
     ios_surface_.reset();
-    accessibility_bridge_.reset();
+    accessibility_bridge_.Clear();
   }
   owner_controller_ = owner_controller;
 
@@ -95,7 +89,7 @@
                                                           queue:[NSOperationQueue mainQueue]
                                                      usingBlock:^(NSNotification* note) {
                                                        // Implicit copy of 'this' is fine.
-                                                       accessibility_bridge_.reset();
+                                                       accessibility_bridge_.Clear();
                                                        owner_controller_.reset();
                                                      }] retain]);
 
@@ -119,7 +113,7 @@
   FML_DCHECK(ios_surface_ != nullptr);
 
   if (accessibility_bridge_) {
-    accessibility_bridge_.reset(new AccessibilityBridge(
+    accessibility_bridge_.Set(std::make_unique<AccessibilityBridge>(
         owner_controller_.get(), this, [owner_controller_.get() platformViewsController]));
   }
 }
@@ -166,10 +160,10 @@
     return;
   }
   if (enabled && !accessibility_bridge_) {
-    accessibility_bridge_.reset(new AccessibilityBridge(
+    accessibility_bridge_.Set(std::make_unique<AccessibilityBridge>(
         owner_controller_.get(), this, [owner_controller_.get() platformViewsController]));
   } else if (!enabled && accessibility_bridge_) {
-    accessibility_bridge_.reset();
+    accessibility_bridge_.Clear();
   } else {
     PlatformView::SetSemanticsEnabled(enabled);
   }
@@ -185,7 +179,7 @@
                                       flutter::CustomAccessibilityActionUpdates actions) {
   FML_DCHECK(owner_controller_);
   if (accessibility_bridge_) {
-    accessibility_bridge_->UpdateSemantics(std::move(update), std::move(actions));
+    accessibility_bridge_.get()->UpdateSemantics(std::move(update), std::move(actions));
     [[NSNotificationCenter defaultCenter] postNotificationName:FlutterSemanticsUpdateNotification
                                                         object:owner_controller_.get()];
   }
@@ -198,7 +192,7 @@
 
 void PlatformViewIOS::OnPreEngineRestart() const {
   if (accessibility_bridge_) {
-    accessibility_bridge_->clearState();
+    accessibility_bridge_.get()->clearState();
   }
   if (!owner_controller_) {
     return;