Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from 22 commits
Commits
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
12 changes: 4 additions & 8 deletions ci/licenses_golden/licenses_flutter
Original file line number Diff line number Diff line change
Expand Up @@ -2405,6 +2405,8 @@ ORIGIN: ../../../flutter/shell/platform/android/vsync_waiter_android.cc + ../../
ORIGIN: ../../../flutter/shell/platform/android/vsync_waiter_android.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/shell/platform/common/accessibility_bridge.cc + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/shell/platform/common/accessibility_bridge.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/shell/platform/common/alert_platform_node_delegate.cc + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/shell/platform/common/alert_platform_node_delegate.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/shell/platform/common/client_wrapper/binary_messenger_impl.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/shell/platform/common/client_wrapper/byte_buffer_streams.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/shell/platform/common/client_wrapper/core_implementations.cc + ../../../flutter/LICENSE
Expand Down Expand Up @@ -3065,12 +3067,8 @@ ORIGIN: ../../../flutter/shell/platform/linux/public/flutter_linux/fl_texture_re
ORIGIN: ../../../flutter/shell/platform/linux/public/flutter_linux/fl_value.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/shell/platform/linux/public/flutter_linux/fl_view.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/shell/platform/linux/public/flutter_linux/flutter_linux.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/shell/platform/windows/accessibility_alert.cc + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/shell/platform/windows/accessibility_alert.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/shell/platform/windows/accessibility_bridge_windows.cc + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/shell/platform/windows/accessibility_bridge_windows.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/shell/platform/windows/accessibility_root_node.cc + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/shell/platform/windows/accessibility_root_node.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/shell/platform/windows/angle_surface_manager.cc + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/shell/platform/windows/angle_surface_manager.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/shell/platform/windows/client_wrapper/flutter_engine.cc + ../../../flutter/LICENSE
Expand Down Expand Up @@ -4894,6 +4892,8 @@ FILE: ../../../flutter/shell/platform/android/vsync_waiter_android.cc
FILE: ../../../flutter/shell/platform/android/vsync_waiter_android.h
FILE: ../../../flutter/shell/platform/common/accessibility_bridge.cc
FILE: ../../../flutter/shell/platform/common/accessibility_bridge.h
FILE: ../../../flutter/shell/platform/common/alert_platform_node_delegate.cc
FILE: ../../../flutter/shell/platform/common/alert_platform_node_delegate.h
FILE: ../../../flutter/shell/platform/common/client_wrapper/binary_messenger_impl.h
FILE: ../../../flutter/shell/platform/common/client_wrapper/byte_buffer_streams.h
FILE: ../../../flutter/shell/platform/common/client_wrapper/core_implementations.cc
Expand Down Expand Up @@ -5573,12 +5573,8 @@ FILE: ../../../flutter/shell/platform/linux/public/flutter_linux/fl_texture_regi
FILE: ../../../flutter/shell/platform/linux/public/flutter_linux/fl_value.h
FILE: ../../../flutter/shell/platform/linux/public/flutter_linux/fl_view.h
FILE: ../../../flutter/shell/platform/linux/public/flutter_linux/flutter_linux.h
FILE: ../../../flutter/shell/platform/windows/accessibility_alert.cc
FILE: ../../../flutter/shell/platform/windows/accessibility_alert.h
FILE: ../../../flutter/shell/platform/windows/accessibility_bridge_windows.cc
FILE: ../../../flutter/shell/platform/windows/accessibility_bridge_windows.h
FILE: ../../../flutter/shell/platform/windows/accessibility_root_node.cc
FILE: ../../../flutter/shell/platform/windows/accessibility_root_node.h
FILE: ../../../flutter/shell/platform/windows/angle_surface_manager.cc
FILE: ../../../flutter/shell/platform/windows/angle_surface_manager.h
FILE: ../../../flutter/shell/platform/windows/client_wrapper/flutter_engine.cc
Expand Down
2 changes: 2 additions & 0 deletions shell/platform/common/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -80,11 +80,13 @@ source_set("common_cpp_switches") {
source_set("common_cpp_accessibility") {
public = [
"accessibility_bridge.h",
"alert_platform_node_delegate.h",
"flutter_platform_node_delegate.h",
]

sources = [
"accessibility_bridge.cc",
"alert_platform_node_delegate.cc",
"flutter_platform_node_delegate.cc",
]

Expand Down
41 changes: 41 additions & 0 deletions shell/platform/common/alert_platform_node_delegate.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
// Copyright 2013 The Flutter Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include "alert_platform_node_delegate.h"

namespace flutter {

AlertPlatformNodeDelegate::AlertPlatformNodeDelegate(
ui::AXPlatformNodeDelegate& parent_delegate)
: parent_delegate_(parent_delegate) {
data_.role = ax::mojom::Role::kAlert;
data_.id = id_.Get();
}

AlertPlatformNodeDelegate::~AlertPlatformNodeDelegate() {}

gfx::AcceleratedWidget
AlertPlatformNodeDelegate::GetTargetForNativeAccessibilityEvent() {
return parent_delegate_.GetTargetForNativeAccessibilityEvent();
}

gfx::NativeViewAccessible AlertPlatformNodeDelegate::GetParent() {
return parent_delegate_.GetNativeViewAccessible();
}

const ui::AXUniqueId& AlertPlatformNodeDelegate::GetUniqueId() const {
return id_;
}

const ui::AXNodeData& AlertPlatformNodeDelegate::GetData() const {
return data_;
}

void AlertPlatformNodeDelegate::SetText(const std::u16string& text) {
data_.SetName(text);
data_.SetDescription(text);
data_.SetValue(text);
}

} // namespace flutter
51 changes: 51 additions & 0 deletions shell/platform/common/alert_platform_node_delegate.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
// Copyright 2013 The Flutter Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#ifndef FLUTTER_SHELL_PLATFORM_COMMON_ALERT_PLATFORM_NODE_DELEGATE_H_
#define FLUTTER_SHELL_PLATFORM_COMMON_ALERT_PLATFORM_NODE_DELEGATE_H_

#include "flutter/third_party/accessibility/ax/ax_node_data.h"
#include "flutter/third_party/accessibility/ax/platform/ax_platform_node_delegate_base.h"

namespace flutter {

// A delegate for a node that holds the text of an a11y alert that a
// screen-reader should announce. The delegate is used to construct an
// AXPlatformNode, and in order to serve as an alert, only needs to be able to
// hold a text announcement and make that text available to the platform node.
class AlertPlatformNodeDelegate : public ui::AXPlatformNodeDelegateBase {
Copy link
Member

Choose a reason for hiding this comment

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

Add a doc comment. (True, we lack a lot in existing code but good to remedy in new code)

Copy link
Member

Choose a reason for hiding this comment

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

Please do a pass, there's several other fields that are also missing comments

public:
explicit AlertPlatformNodeDelegate(
ui::AXPlatformNodeDelegate& parent_delegate);
~AlertPlatformNodeDelegate();

AlertPlatformNodeDelegate(const AlertPlatformNodeDelegate& other) = delete;
AlertPlatformNodeDelegate operator=(const AlertPlatformNodeDelegate& other) =
delete;

// Set the alert text of the node for which this is the delegate.
void SetText(const std::u16string& text);
Copy link
Member

Choose a reason for hiding this comment

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

Consider void SetText(std::u16string_view text);. It's super lightweight, sometimes better performance, and allows for passing a char16_t* string as well (unlikely as that is).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The AXNodeData methods that this method calls take string& parameters. Am I mistaken that constructing a string from a string_view effectively copies it?

Copy link
Member

Choose a reason for hiding this comment

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

I'm also leaning towards keeping this a string reference, but, the string reference is also copied when AXNodeData stores it as an attribute.

Copy link
Member

Choose a reason for hiding this comment

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

The AXNodeData methods that this method calls take string& parameters. Am I mistaken that constructing a string from a string_view effectively copies it?

Yes - to benefit from this, you'd need any methods this gets passed to to also take a view rather than a string ref. If the current implementation is doing so, then leave as-is; at some point we can do a sweep of the codebase to clean these up across the board.


// |AXPlatformNodeDelegate|
gfx::NativeViewAccessible GetParent() override;

private:
// AXPlatformNodeDelegate overrides.
gfx::AcceleratedWidget GetTargetForNativeAccessibilityEvent() override;
const ui::AXUniqueId& GetUniqueId() const override;
const ui::AXNodeData& GetData() const override;

// Delegate of the parent of this node. Returned by GetParent.
ui::AXPlatformNodeDelegate& parent_delegate_;

// Node Data that contains the alert text. Returned by GetData.
ui::AXNodeData data_;

// A unique ID used to identify this node. Returned by GetUniqueId.
ui::AXUniqueId id_;
};

} // namespace flutter

#endif // FLUTTER_SHELL_PLATFORM_COMMON_ALERT_PLATFORM_NODE_DELEGATE_H_
4 changes: 0 additions & 4 deletions shell/platform/windows/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,8 @@ source_set("flutter_windows_headers") {
source_set("flutter_windows_source") {
# Common Windows sources.
sources = [
"accessibility_alert.cc",
"accessibility_alert.h",
"accessibility_bridge_windows.cc",
"accessibility_bridge_windows.h",
"accessibility_root_node.cc",
"accessibility_root_node.h",
"angle_surface_manager.cc",
"angle_surface_manager.h",
"cursor_handler.cc",
Expand Down
187 changes: 0 additions & 187 deletions shell/platform/windows/accessibility_alert.cc

This file was deleted.

Loading