Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Isolate the AccessibilityBridge from single views #36573

Closed
wants to merge 10 commits into from
Closed

Isolate the AccessibilityBridge from single views #36573

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
f766226
Impl1
dkwingsmt Oct 3, 2022
80a735a
Win
dkwingsmt Oct 3, 2022
65edba4
Format
dkwingsmt Oct 3, 2022
eda5c5d
better comment
dkwingsmt Oct 3, 2022
186778d
Rm window plugin
dkwingsmt Oct 3, 2022
dfc01b7
Comment
dkwingsmt Oct 3, 2022
17e33db
Merge UpdateDelegate
dkwingsmt Oct 3, 2022
8b4f689
Fix windows build
dkwingsmt Oct 3, 2022
9bb124f
format
dkwingsmt Oct 4, 2022
6cd5c67
Fix build
dkwingsmt Oct 4, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 7 additions & 3 deletions shell/platform/common/accessibility_bridge.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,17 @@ constexpr int kHasScrollingAction =
FlutterSemanticsAction::kFlutterSemanticsActionScrollDown;

// AccessibilityBridge
AccessibilityBridge::AccessibilityBridge(
std::unique_ptr<AccessibilityBridgeDelegate> delegate)
: delegate_(std::move(delegate)) {
AccessibilityBridge::AccessibilityBridge() {
event_generator_.SetTree(&tree_);
tree_.AddObserver(static_cast<ui::AXTreeObserver*>(this));
}

AccessibilityBridge::AccessibilityBridge(
std::unique_ptr<AccessibilityBridgeDelegate> delegate)
: AccessibilityBridge() {
UpdateDelegate(std::move(delegate));
}

AccessibilityBridge::~AccessibilityBridge() {
event_generator_.ReleaseTree();
tree_.RemoveObserver(static_cast<ui::AXTreeObserver*>(this));
Expand Down
20 changes: 14 additions & 6 deletions shell/platform/common/accessibility_bridge.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,9 @@
namespace flutter {

//------------------------------------------------------------------------------
/// Use this class to maintain an accessibility tree. This class consumes
/// semantics updates from the embedder API and produces an accessibility tree
/// in the native format.
/// Use this class to maintain the accessibility tree for a view. This class
/// consumes semantics updates from the embedder API and produces an
/// accessibility tree in the native format.
///
/// The bridge creates an AXTree to hold the semantics data that comes from
/// Flutter semantics updates. The tree holds AXNode[s] which contain the
Expand Down Expand Up @@ -63,6 +63,7 @@ class AccessibilityBridge
class AccessibilityBridgeDelegate {
public:
virtual ~AccessibilityBridgeDelegate() = default;

//---------------------------------------------------------------------------
/// @brief Handle accessibility events generated due to accessibility
/// tree changes. These events are generated in accessibility
Expand Down Expand Up @@ -101,12 +102,19 @@ class AccessibilityBridge
CreateFlutterPlatformNodeDelegate() = 0;
};

//-----------------------------------------------------------------------------
/// @brief Creates a new instance of a accessibility bridge without a
/// delegate.
///
/// The bridge is not ready for use until a delegate has been
/// assigned. See AccessibilityBridge::UpdateDelegate.
AccessibilityBridge();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From my understanding, this trick is necessary due to a circular dependency between the accessibility bridge and the delegate. The drawback is that it's now possible for the accessibility bridge to be in an invalid state. Would it be possible to avoid this circular dependency to avoid this drawback?

For example, perhaps the accessibility bridge could provide its instance when it calls the delegate's methods?

Copy link
Contributor Author

@dkwingsmt dkwingsmt Oct 3, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're absolutely correct about the drawback.

There's another approach: The bridge is initialized with the viewId, fetches the view controller from the engine on demand, and passes this ID to the delegate and the nodes. However I'm really hoping to limit the knowledge of the view IDs, and instead let the bridge talk directly with assigned objects.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another approach is to let AccessibilityBridge to be able to host multiple AXTrees, and I think that is what chromium does, too. The AXTreeData can set the id for different AXTree, and you can also derive axtree id from a AXNode. You can let the FlutterEngine to create a wrapper delegate that wires up action and event to different ViewController and view.

Copy link
Contributor Author

@dkwingsmt dkwingsmt Oct 3, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can, but is it making a big difference? In the current approach, the engine will manage a map of view controllers and bridges. In your approach, the engine will manage a map of view controllers and one bridge, and the bridge will manage a map of delegates and trees (which is likely another class). So you're basically moving what's in the engine into the bridge singleton, which isn't much code (just map addition and map removal).

Copy link
Contributor Author

@dkwingsmt dkwingsmt Oct 4, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually I'm wondering, why can't we merge the delegate and the bridge? We'll still use virtual methods to allow platform-specific code, and reassign views when the view is being swapped. The engine will hold a map of bridges, and there will be no circular reference.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My original thought about the multiple AXTree in one bridge vs Multiple bridges was on how easy to use the bridge in different platforms. I am not sure how the embedding code will be like, so I will leave the decision to you.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another approach is to let AccessibilityBridge to be able to host multiple AXTrees, and I think that is what chromium does, too.

We can, but is it making a big difference?

I think that doing the same thing that chromium does will help us in the short term. We will definitely encounter mistakes in the short term as we add this new functionality. The more help we can get, the better.

Copy link
Contributor Author

@dkwingsmt dkwingsmt Oct 4, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the circular dependency part, I think I kinda prefer the other way around, let the delegate aware of the bridge later. If one of them has to be an unusable state during initializing, let the delegate be unusable would probably be better. The bridge is a public class that will be shared between embedding, it is better for it to be more stable

My initial thought was also the 2nd option, but I gave up because it's still very hard to initialize the bridge. There are two ways:

If the outside code assign the bridge to the delegate manually, it'd be hard to get the delegate since it's been moved in, and has to use a dangerous raw pointer:

void productionCode() {
  auto delegate = new FlutterPlatformNodeDelegateMac();
  // The delegate will be moved away before it can be assigned a bridge.
  auto bridge = new AccessibilityBridge(std::unique_ptr<FlutterPlatformNodeDelegateMac>(delegate));
  delegate->SetBridge(bridge);
}

Or the bridge can also assign the delegate automatically, but then the bridge needs to access its own weak_ptr in constructor, which somehow crashes (it seems that there's some restriction with shared_from_this and weak_from_this, which I'm not sure.)

AccessibilityBridge::AccessibilityBridge(std::unique_ptr<AccessibilityBridgeDelegate> delegate) {
  delegate_ = std::move(delegate);
  delegate_->SetBridge(weak_from_this()); // crashes
}

Copy link
Contributor Author

@dkwingsmt dkwingsmt Oct 4, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that doing the same thing that chromium does will help us in the short term. We will definitely encounter mistakes in the short term as we add this new functionality. The more help we can get, the better.

What kind of help and from whom are we expecting? The code is going to evolve and who other than the Flutter contributors will read our code?

I think the best kind of help is that we choose the best structure that suits us with the best maintainability and cleanest flow, instead of trying to fit our feet in others' shoes.

Copy link
Contributor

@chunhtai chunhtai Oct 4, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or the bridge can also assign the delegate automatically, but then the bridge needs to access its own weak_ptr in constructor, which somehow crashes (it seems that there's some restriction with shared_from_this and weak_from_this, which I'm not sure.)

shared_from_this and weak_from_this requires the AccessibilityBridge to be created with shared_pointer. It should not crash though since the flutterengine in macos already holds it as shared_ptr, and it has already used shared_from_this in UpdateDelegate. Anyway, I don't think this is the right way though, not every platform's delegate needs the reference to bridge.

If the outside code assign the bridge to the delegate manually, it'd be hard to get the delegate since it's been moved in, and has to use a dangerous raw pointer

I am not sure what moved away means, can you use shared_ptr + weak_ptr? the unqiue ptr doesn't really make sense in this ownership model if multiple objects want references from it. it would also be good to draw out the ownership model, right now there seems to be a lot of moving parts, engine, bridge, delegate, viewcontroller.


//-----------------------------------------------------------------------------
/// @brief Creates a new instance of a accessibility bridge.
///
/// @param[in] user_data A custom pointer to the data of your
/// choice. This pointer can be retrieve later
/// through GetUserData().
/// This is effectively identical to the default constructor
/// followed by AccessibilityBridge::UpdateDelegate.
explicit AccessibilityBridge(
std::unique_ptr<AccessibilityBridgeDelegate> delegate);
~AccessibilityBridge();
Expand Down
4 changes: 3 additions & 1 deletion shell/platform/darwin/macos/framework/Source/AccessibilityBridgeMacDelegate.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@ class AccessibilityBridgeMacDelegate : public AccessibilityBridge::Accessibility
/// @param[in] flutter_engine The weak reference to the FlutterEngine.
/// @param[in] view_controller The weak reference to the FlutterViewController.
explicit AccessibilityBridgeMacDelegate(__weak FlutterEngine* flutter_engine,
__weak FlutterViewController* view_controller);
__weak FlutterViewController* view_controller,
std::weak_ptr<AccessibilityBridge> bridge);
virtual ~AccessibilityBridgeMacDelegate() = default;

// |AccessibilityBridge::AccessibilityBridgeDelegate|
Expand Down Expand Up @@ -80,6 +81,7 @@ class AccessibilityBridgeMacDelegate : public AccessibilityBridge::Accessibility

__weak FlutterEngine* flutter_engine_;
__weak FlutterViewController* view_controller_;
std::weak_ptr<flutter::AccessibilityBridge> accessibility_bridge_;
};

} // namespace flutter
Expand Down
27 changes: 14 additions & 13 deletions shell/platform/darwin/macos/framework/Source/AccessibilityBridgeMacDelegate.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,15 @@

AccessibilityBridgeMacDelegate::AccessibilityBridgeMacDelegate(
__weak FlutterEngine* flutter_engine,
__weak FlutterViewController* view_controller)
: flutter_engine_(flutter_engine), view_controller_(view_controller) {}
__weak FlutterViewController* view_controller,
std::weak_ptr<AccessibilityBridge> bridge)
: flutter_engine_(flutter_engine),
view_controller_(view_controller),
accessibility_bridge_(bridge) {}

void AccessibilityBridgeMacDelegate::OnAccessibilityEvent(
ui::AXEventGenerator::TargetedEvent targeted_event) {
if (!flutter_engine_.viewController.viewLoaded || !flutter_engine_.viewController.view.window) {
if (!view_controller_.viewLoaded || !view_controller_.view.window) {
// Don't need to send accessibility events if the there is no view or window.
return;
}
Expand All @@ -47,9 +50,8 @@
AccessibilityBridgeMacDelegate::MacOSEventsFromAXEvent(ui::AXEventGenerator::Event event_type,
const ui::AXNode& ax_node) const {
// Gets the native_node with the node_id.
NSCAssert(flutter_engine_, @"Flutter engine should not be deallocated");
auto bridge = flutter_engine_.accessibilityBridge.lock();
NSCAssert(bridge, @"Accessibility bridge in flutter engine must not be null.");
auto bridge = accessibility_bridge_.lock();
NSCAssert(bridge, @"Accessibility bridge should not be expired");
auto platform_node_delegate = bridge->GetFlutterPlatformNodeDelegateFromID(ax_node.id()).lock();
NSCAssert(platform_node_delegate, @"Event target must exist in accessibility bridge.");
auto mac_platform_node_delegate =
Expand Down Expand Up @@ -292,10 +294,10 @@
case ui::AXEventGenerator::Event::CHILDREN_CHANGED: {
// NSAccessibilityCreatedNotification seems to be the only way to let
// Voiceover pick up layout changes.
NSCAssert(flutter_engine_.viewController, @"The viewController must not be nil");
NSCAssert(view_controller_, @"The viewController must not be nil");
events.push_back({
.name = NSAccessibilityCreatedNotification,
.target = flutter_engine_.viewController.view.window,
.target = view_controller_.view.window,
.user_info = nil,
});
break;
Expand Down Expand Up @@ -363,15 +365,15 @@
FlutterSemanticsAction action,
fml::MallocMapping data) {
NSCAssert(flutter_engine_, @"Flutter engine should not be deallocated");
NSCAssert(flutter_engine_.viewController.viewLoaded && flutter_engine_.viewController.view.window,
NSCAssert(view_controller_.viewLoaded && view_controller_.view.window,
@"The accessibility bridge should not receive accessibility actions if the flutter view"
@"is not loaded or attached to a NSWindow.");
[flutter_engine_ dispatchSemanticsAction:action toTarget:target withData:std::move(data)];
}

std::shared_ptr<FlutterPlatformNodeDelegate>
AccessibilityBridgeMacDelegate::CreateFlutterPlatformNodeDelegate() {
return std::make_shared<FlutterPlatformNodeDelegateMac>(flutter_engine_, view_controller_);
return std::make_shared<FlutterPlatformNodeDelegateMac>(view_controller_, accessibility_bridge_);
}

// Private method
Expand All @@ -394,9 +396,8 @@
}

bool AccessibilityBridgeMacDelegate::HasPendingEvent(ui::AXEventGenerator::Event event) const {
NSCAssert(flutter_engine_, @"Flutter engine should not be deallocated");
auto bridge = flutter_engine_.accessibilityBridge.lock();
NSCAssert(bridge, @"Accessibility bridge in flutter engine must not be null.");
auto bridge = accessibility_bridge_.lock();
NSCAssert(bridge, @"Accessibility bridge should not be expired");
std::vector<ui::AXEventGenerator::TargetedEvent> pending_events = bridge->GetPendingEvents();
for (const auto& pending_event : bridge->GetPendingEvents()) {
if (pending_event.event_params.event == event) {
Expand Down
40 changes: 25 additions & 15 deletions shell/platform/darwin/macos/framework/Source/AccessibilityBridgeMacDelegateTest.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,9 @@
class AccessibilityBridgeMacDelegateSpy : public AccessibilityBridgeMacDelegate {
public:
AccessibilityBridgeMacDelegateSpy(__weak FlutterEngine* flutter_engine,
__weak FlutterViewController* view_controller)
: AccessibilityBridgeMacDelegate(flutter_engine, view_controller) {}
__weak FlutterViewController* view_controller,
std::weak_ptr<AccessibilityBridge> bridge)
: AccessibilityBridgeMacDelegate(flutter_engine, view_controller, bridge) {}

std::unordered_map<std::string, gfx::NativeViewAccessible> actual_notifications;

Expand Down Expand Up @@ -55,7 +56,12 @@ void DispatchMacOSNotification(gfx::NativeViewAccessible native_node,
// Setting up bridge so that the AccessibilityBridgeMacDelegateSpy
// can query semantics information from.
engine.semanticsEnabled = YES;

AccessibilityBridgeMacDelegateSpy* spy =
new AccessibilityBridgeMacDelegateSpy(engine, viewController, engine.accessibilityBridge);
auto bridge = engine.accessibilityBridge.lock();
bridge->UpdateDelegate(std::unique_ptr<AccessibilityBridgeMacDelegateSpy>(spy));

FlutterSemanticsNode root;
root.id = 0;
root.flags = static_cast<FlutterSemanticsFlag>(0);
Expand All @@ -74,8 +80,6 @@ void DispatchMacOSNotification(gfx::NativeViewAccessible native_node,
bridge->CommitUpdates();
auto platform_node_delegate = bridge->GetFlutterPlatformNodeDelegateFromID(0).lock();

AccessibilityBridgeMacDelegateSpy spy(engine, viewController);

// Creates a targeted event.
ui::AXTree tree;
ui::AXNode ax_node(&tree, nullptr, 0, 0);
Expand All @@ -87,10 +91,10 @@ void DispatchMacOSNotification(gfx::NativeViewAccessible native_node,
ax::mojom::EventFrom::kNone, intent);
ui::AXEventGenerator::TargetedEvent targeted_event(&ax_node, event_params);

spy.OnAccessibilityEvent(targeted_event);
spy->OnAccessibilityEvent(targeted_event);

EXPECT_EQ(spy.actual_notifications.size(), 1u);
EXPECT_EQ(spy.actual_notifications.find([NSAccessibilityCreatedNotification UTF8String])->second,
EXPECT_EQ(spy->actual_notifications.size(), 1u);
EXPECT_EQ(spy->actual_notifications.find([NSAccessibilityCreatedNotification UTF8String])->second,
expectedTarget);
[engine shutDownEngine];
}
Expand All @@ -107,7 +111,12 @@ void DispatchMacOSNotification(gfx::NativeViewAccessible native_node,
// Setting up bridge so that the AccessibilityBridgeMacDelegateSpy
// can query semantics information from.
engine.semanticsEnabled = YES;

AccessibilityBridgeMacDelegateSpy* spy =
new AccessibilityBridgeMacDelegateSpy(engine, viewController, engine.accessibilityBridge);
auto bridge = engine.accessibilityBridge.lock();
bridge->UpdateDelegate(std::unique_ptr<AccessibilityBridgeMacDelegateSpy>(spy));

FlutterSemanticsNode root;
root.id = 0;
root.flags = static_cast<FlutterSemanticsFlag>(0);
Expand All @@ -126,8 +135,6 @@ void DispatchMacOSNotification(gfx::NativeViewAccessible native_node,
bridge->CommitUpdates();
auto platform_node_delegate = bridge->GetFlutterPlatformNodeDelegateFromID(0).lock();

AccessibilityBridgeMacDelegateSpy spy(engine, viewController);

// Creates a targeted event.
ui::AXTree tree;
ui::AXNode ax_node(&tree, nullptr, 0, 0);
Expand All @@ -139,10 +146,10 @@ void DispatchMacOSNotification(gfx::NativeViewAccessible native_node,
ax::mojom::EventFrom::kNone, intent);
ui::AXEventGenerator::TargetedEvent targeted_event(&ax_node, event_params);

spy.OnAccessibilityEvent(targeted_event);
spy->OnAccessibilityEvent(targeted_event);

// Does not send any notification if the engine is headless.
EXPECT_EQ(spy.actual_notifications.size(), 0u);
EXPECT_EQ(spy->actual_notifications.size(), 0u);
[engine shutDownEngine];
}

Expand All @@ -160,7 +167,12 @@ void DispatchMacOSNotification(gfx::NativeViewAccessible native_node,
// Setting up bridge so that the AccessibilityBridgeMacDelegateSpy
// can query semantics information from.
engine.semanticsEnabled = YES;

AccessibilityBridgeMacDelegateSpy* spy =
new AccessibilityBridgeMacDelegateSpy(engine, viewController, engine.accessibilityBridge);
auto bridge = engine.accessibilityBridge.lock();
bridge->UpdateDelegate(std::unique_ptr<AccessibilityBridgeMacDelegateSpy>(spy));

FlutterSemanticsNode root;
root.id = 0;
root.flags = static_cast<FlutterSemanticsFlag>(0);
Expand All @@ -179,8 +191,6 @@ void DispatchMacOSNotification(gfx::NativeViewAccessible native_node,
bridge->CommitUpdates();
auto platform_node_delegate = bridge->GetFlutterPlatformNodeDelegateFromID(0).lock();

AccessibilityBridgeMacDelegateSpy spy(engine, viewController);

// Creates a targeted event.
ui::AXTree tree;
ui::AXNode ax_node(&tree, nullptr, 0, 0);
Expand All @@ -192,10 +202,10 @@ void DispatchMacOSNotification(gfx::NativeViewAccessible native_node,
ax::mojom::EventFrom::kNone, intent);
ui::AXEventGenerator::TargetedEvent targeted_event(&ax_node, event_params);

spy.OnAccessibilityEvent(targeted_event);
spy->OnAccessibilityEvent(targeted_event);

// Does not send any notification if the flutter view is not attached to a NSWindow.
EXPECT_EQ(spy.actual_notifications.size(), 0u);
EXPECT_EQ(spy->actual_notifications.size(), 0u);
[engine shutDownEngine];
}

Expand Down
9 changes: 5 additions & 4 deletions shell/platform/darwin/macos/framework/Source/FlutterEngine.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -414,8 +414,8 @@ - (void)setViewController:(FlutterViewController*)controller {
[_renderer setFlutterView:controller.flutterView];

if (_semanticsEnabled && _bridge) {
_bridge->UpdateDelegate(
std::make_unique<flutter::AccessibilityBridgeMacDelegate>(self, _viewController));
_bridge->UpdateDelegate(std::make_unique<flutter::AccessibilityBridgeMacDelegate>(
self, _viewController, _bridge));
}

if (!controller && !_allowHeadlessExecution) {
Expand Down Expand Up @@ -597,8 +597,9 @@ - (void)setSemanticsEnabled:(BOOL)enabled {
if (!_semanticsEnabled && _bridge) {
_bridge.reset();
} else if (_semanticsEnabled && !_bridge) {
_bridge = std::make_shared<flutter::AccessibilityBridge>(
std::make_unique<flutter::AccessibilityBridgeMacDelegate>(self, self.viewController));
_bridge = std::make_shared<flutter::AccessibilityBridge>();
_bridge->UpdateDelegate(std::make_unique<flutter::AccessibilityBridgeMacDelegate>(
self, self.viewController, _bridge));
}
_embedderAPI.UpdateSemanticsEnabled(_engine, _semanticsEnabled);
}
Expand Down
7 changes: 4 additions & 3 deletions shell/platform/darwin/macos/framework/Source/FlutterPlatformNodeDelegateMac.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

#import "flutter/shell/platform/darwin/macos/framework/Headers/FlutterEngine.h"

#include "flutter/shell/platform/common/accessibility_bridge.h"
#include "flutter/shell/platform/common/flutter_platform_node_delegate.h"
#include "flutter/shell/platform/embedder/embedder.h"

Expand All @@ -20,8 +21,8 @@ namespace flutter {
class FlutterPlatformNodeDelegateMac : public FlutterPlatformNodeDelegate {
public:
explicit FlutterPlatformNodeDelegateMac(
__weak FlutterEngine* engine,
__weak FlutterViewController* view_controller);
__weak FlutterViewController* view_controller,
std::weak_ptr<flutter::AccessibilityBridge> accessibility_bridge);
virtual ~FlutterPlatformNodeDelegateMac();

void Init(std::weak_ptr<OwnerBridge> bridge, ui::AXNode* node) override;
Expand Down Expand Up @@ -49,8 +50,8 @@ class FlutterPlatformNodeDelegateMac : public FlutterPlatformNodeDelegate {

private:
ui::AXPlatformNode* ax_platform_node_;
__weak FlutterEngine* engine_;
__weak FlutterViewController* view_controller_;
std::weak_ptr<flutter::AccessibilityBridge> accessibility_bridge_;

gfx::RectF ConvertBoundsFromLocalToScreen(
const gfx::RectF& local_bounds) const;
Expand Down
29 changes: 12 additions & 17 deletions shell/platform/darwin/macos/framework/Source/FlutterPlatformNodeDelegateMac.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
#import "flutter/shell/platform/darwin/macos/framework/Source/FlutterPlatformNodeDelegateMac.h"

#import "flutter/shell/platform/darwin/macos/framework/Headers/FlutterAppDelegate.h"
#import "flutter/shell/platform/darwin/macos/framework/Source/FlutterEngine_Internal.h"
#import "flutter/shell/platform/darwin/macos/framework/Source/FlutterTextInputSemanticsObject.h"
#import "flutter/shell/platform/darwin/macos/framework/Source/FlutterViewController_Internal.h"

Expand All @@ -21,9 +20,9 @@
namespace flutter { // namespace

FlutterPlatformNodeDelegateMac::FlutterPlatformNodeDelegateMac(
__weak FlutterEngine* engine,
__weak FlutterViewController* view_controller)
: engine_(engine), view_controller_(view_controller) {}
__weak FlutterViewController* view_controller,
std::weak_ptr<flutter::AccessibilityBridge> accessibility_bridge)
: view_controller_(view_controller), accessibility_bridge_(accessibility_bridge) {}

void FlutterPlatformNodeDelegateMac::Init(std::weak_ptr<OwnerBridge> bridge, ui::AXNode* node) {
FlutterPlatformNodeDelegate::Init(bridge, node);
Expand All @@ -48,9 +47,9 @@
gfx::NativeViewAccessible FlutterPlatformNodeDelegateMac::GetParent() {
gfx::NativeViewAccessible parent = FlutterPlatformNodeDelegate::GetParent();
if (!parent) {
NSCAssert(engine_, @"Flutter engine should not be deallocated");
NSCAssert(engine_.viewController.viewLoaded, @"Flutter view must be loaded");
return engine_.viewController.flutterView;
NSCAssert(!accessibility_bridge_.expired(), @"Accessibility bridge should not be expired");
NSCAssert(view_controller_.viewLoaded, @"Flutter view must be loaded");
return view_controller_.flutterView;
}
return parent;
}
Expand Down Expand Up @@ -80,9 +79,8 @@
if (!text.empty()) {
return text;
};
NSCAssert(engine_, @"Flutter engine should not be deallocated");
auto bridge_ptr = engine_.accessibilityBridge.lock();
NSCAssert(bridge_ptr, @"Accessibility bridge in flutter engine must not be null.");
auto bridge_ptr = accessibility_bridge_.lock();
NSCAssert(bridge_ptr, @"Accessibility bridge should not be expired");
for (int32_t child : GetData().child_ids) {
auto delegate_child = bridge_ptr->GetFlutterPlatformNodeDelegateFromID(child).lock();
if (!delegate_child) {
Expand All @@ -105,14 +103,11 @@
// it converts the bounds from flutter coordinates to macOS coordinates.
ns_local_bounds.origin.y = -ns_local_bounds.origin.y - ns_local_bounds.size.height;

NSCAssert(engine_, @"Flutter engine should not be deallocated");
NSCAssert(engine_.viewController.viewLoaded, @"Flutter view must be loaded.");
NSRect ns_view_bounds =
[engine_.viewController.flutterView convertRectFromBacking:ns_local_bounds];
NSRect ns_window_bounds = [engine_.viewController.flutterView convertRect:ns_view_bounds
toView:nil];
NSCAssert(view_controller_.viewLoaded, @"Flutter view must be loaded.");
NSRect ns_view_bounds = [view_controller_.flutterView convertRectFromBacking:ns_local_bounds];
NSRect ns_window_bounds = [view_controller_.flutterView convertRect:ns_view_bounds toView:nil];
NSRect ns_screen_bounds =
[[engine_.viewController.flutterView window] convertRectToScreen:ns_window_bounds];
[[view_controller_.flutterView window] convertRectToScreen:ns_window_bounds];
gfx::RectF screen_bounds(ns_screen_bounds.origin.x, ns_screen_bounds.origin.y,
ns_screen_bounds.size.width, ns_screen_bounds.size.height);
return screen_bounds;
Expand Down
Loading