Skip to content

Commit

Permalink
Ensure that PlatformViewIOS does not call into Shell semantics APIs d…
Browse files Browse the repository at this point in the history 
…uring destruction (#30835)
  • Loading branch information
@jason-simmons @JsouLiang
jason-simmons authored and JsouLiang committed Jan 14, 2022
1 parent 791e660 commit a85f986
Show file tree
Hide file tree
Showing 3 changed files with 73 additions and 32 deletions.
47 changes: 47 additions & 0 deletions shell/platform/darwin/ios/framework/Source/accessibility_bridge_test.mm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
20 changes: 10 additions & 10 deletions shell/platform/darwin/ios/platform_view_ios.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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_;
};
Expand All @@ -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_;
Expand Down
38 changes: 16 additions & 22 deletions shell/platform/darwin/ios/platform_view_ios.mm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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(
Expand Down Expand Up @@ -83,7 +77,7 @@
if (ios_surface_ || !owner_controller) {
NotifyDestroyed();
ios_surface_.reset();
accessibility_bridge_.reset();
accessibility_bridge_.Clear();
}
owner_controller_ = owner_controller;

Expand All @@ -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]);

Expand All @@ -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]));
}
}
Expand Down Expand Up @@ -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);
}
Expand All @@ -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()];
}
Expand All @@ -198,7 +192,7 @@

void PlatformViewIOS::OnPreEngineRestart() const {
if (accessibility_bridge_) {
accessibility_bridge_->clearState();
accessibility_bridge_.get()->clearState();
}
if (!owner_controller_) {
return;
Expand Down

0 comments on commit a85f986

Please sign in to comment.