Skip to content

Commit

Permalink
a11y inspect: unify event recorders and tree formatters factory
Browse files Browse the repository at this point in the history
mechanisms

Bug: 1133330
Change-Id: Idf95ce5a74541a61d78455a1821192381bb40bd0
AX-Relnotes: n/a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2668451
Commit-Queue: Alexander Surkov <asurkov@igalia.com>
Reviewed-by: Dominic Mazzoni <dmazzoni@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#850327}
  • Loading branch information
asurkov authored and Chromium LUCI CQ committed Feb 3, 2021
1 parent 4b02105 commit a20abfb
Show file tree
Hide file tree
Showing 23 changed files with 192 additions and 155 deletions.
26 changes: 0 additions & 26 deletions content/browser/accessibility/accessibility_event_recorder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,38 +4,12 @@

#include "content/browser/accessibility/accessibility_event_recorder.h"

#include "build/build_config.h"
#include "content/browser/accessibility/browser_accessibility_manager.h"
#include "ui/base/buildflags.h"

namespace content {

AccessibilityEventRecorder::AccessibilityEventRecorder(
BrowserAccessibilityManager* manager)
: manager_(manager) {}

#if !defined(OS_WIN) && !defined(OS_MAC) && !BUILDFLAG(USE_ATK)
// static
std::unique_ptr<AccessibilityEventRecorder> AccessibilityEventRecorder::Create(
BrowserAccessibilityManager* manager,
base::ProcessId pid,
const AXTreeSelector& selector) {
return std::make_unique<AccessibilityEventRecorder>(manager);
}

// static
std::vector<AccessibilityEventRecorder::TestPass>
AccessibilityEventRecorder::GetTestPasses() {
#if defined(OS_ANDROID)
// Note: Android doesn't do a "blink" pass; the blink tree is different on
// Android because we exclude inline text boxes, for performance.
return {{"android", &AccessibilityEventRecorder::Create}};
#else // defined(OS_ANDROID)
return {
{"blink", &AccessibilityEventRecorder::Create},
};
#endif // defined(OS_ANDROID)
}
#endif

} // namespace content
11 changes: 0 additions & 11 deletions content/browser/accessibility/accessibility_event_recorder.h
Original file line number Diff line number Diff line change
Expand Up @@ -38,23 +38,12 @@ class BrowserAccessibilityManager;
// As currently designed, there should only be one instance of this class.
class CONTENT_EXPORT AccessibilityEventRecorder : public ui::AXEventRecorder {
public:
// Construct the right platform-specific subclass.
static std::unique_ptr<AccessibilityEventRecorder> Create(
BrowserAccessibilityManager* manager = nullptr,
base::ProcessId pid = 0,
const AXTreeSelector& selector = {});

// Get a set of factory methods to create event-recorders, one for each test
// pass; see |DumpAccessibilityTestBase|.
using EventRecorderFactory = std::unique_ptr<AccessibilityEventRecorder> (*)(
BrowserAccessibilityManager* manager,
base::ProcessId pid,
const AXTreeSelector& selector);
struct TestPass {
const char* name;
EventRecorderFactory create_recorder;
};
static std::vector<TestPass> GetTestPasses();

AccessibilityEventRecorder(BrowserAccessibilityManager* manager);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,24 +54,6 @@ gboolean AccessibilityEventRecorderAuraLinux::OnATKEventReceived(
return true;
}

// static
std::unique_ptr<AccessibilityEventRecorder> AccessibilityEventRecorder::Create(
BrowserAccessibilityManager* manager,
base::ProcessId pid,
const AXTreeSelector& selector) {
return std::make_unique<AccessibilityEventRecorderAuraLinux>(manager, pid,
selector);
}

std::vector<AccessibilityEventRecorder::TestPass>
AccessibilityEventRecorder::GetTestPasses() {
// Both the Blink pass and native pass use the same recorder
return {
{"blink", &AccessibilityEventRecorder::Create},
{"linux", &AccessibilityEventRecorder::Create},
};
}

bool AccessibilityEventRecorderAuraLinux::ShouldUseATSPI() {
return pid_ != base::GetCurrentProcId() ||
!application_name_match_pattern_.empty();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ class CONTENT_EXPORT AccessibilityEventRecorderMac
public:
AccessibilityEventRecorderMac(BrowserAccessibilityManager* manager,
base::ProcessId pid,
AXUIElementRef node);
const AXTreeSelector& selector);
~AccessibilityEventRecorderMac() override;

// Callback executed every time we receive an event notification.
Expand Down
23 changes: 3 additions & 20 deletions content/browser/accessibility/accessibility_event_recorder_mac.mm
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,11 @@ static void EventReceivedThunk(AXObserverRef observer_ref,
this_ptr->EventReceived(element, notification, user_info);
}

// static
std::unique_ptr<AccessibilityEventRecorder> AccessibilityEventRecorder::Create(
AccessibilityEventRecorderMac::AccessibilityEventRecorderMac(
BrowserAccessibilityManager* manager,
base::ProcessId pid,
const AXTreeSelector& selector) {
const AXTreeSelector& selector)
: AccessibilityEventRecorder(manager), observer_run_loop_source_(nullptr) {
AXUIElementRef node = nil;
if (pid) {
node = AXUIElementCreateApplication(pid);
Expand All @@ -49,23 +49,6 @@ static void EventReceivedThunk(AXObserverRef observer_ref,
}
}

return std::make_unique<AccessibilityEventRecorderMac>(manager, pid, node);
}

std::vector<AccessibilityEventRecorder::TestPass>
AccessibilityEventRecorder::GetTestPasses() {
// Both the Blink pass and native pass use the same recorder
return {
{"blink", &AccessibilityEventRecorder::Create},
{"mac", &AccessibilityEventRecorder::Create},
};
}

AccessibilityEventRecorderMac::AccessibilityEventRecorderMac(
BrowserAccessibilityManager* manager,
base::ProcessId pid,
AXUIElementRef node)
: AccessibilityEventRecorder(manager), observer_run_loop_source_(NULL) {
if (kAXErrorSuccess !=
AXObserverCreateWithInfoCallback(pid, EventReceivedThunk,
observer_ref_.InitializeInto())) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,15 +45,6 @@ std::string UiaIdentifierToStringPretty(int32_t id) {
volatile base::subtle::Atomic32 AccessibilityEventRecorderUia::instantiated_ =
0;

// static
std::unique_ptr<AccessibilityEventRecorder>
AccessibilityEventRecorderUia::CreateUia(BrowserAccessibilityManager* manager,
base::ProcessId pid,
const AXTreeSelector& selector) {
return std::make_unique<AccessibilityEventRecorderUia>(manager, pid,
selector.pattern);
}

AccessibilityEventRecorderUia::AccessibilityEventRecorderUia(
BrowserAccessibilityManager* manager,
base::ProcessId pid,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,6 @@ class AccessibilityEventRecorderUia : public AccessibilityEventRecorder {
const base::StringPiece& application_name_match_pattern);
~AccessibilityEventRecorderUia() override;

static std::unique_ptr<AccessibilityEventRecorder> CreateUia(
BrowserAccessibilityManager* manager,
base::ProcessId pid,
const AXTreeSelector& selector);

// Called to ensure the event recorder has finished recording async events.
void FlushAsyncEvents() override;

Expand Down
26 changes: 0 additions & 26 deletions content/browser/accessibility/accessibility_event_recorder_win.cc
Original file line number Diff line number Diff line change
Expand Up @@ -79,32 +79,6 @@ std::string AccessibilityEventToStringUTF8(int32_t event_id) {
AccessibilityEventRecorderWin* AccessibilityEventRecorderWin::instance_ =
nullptr;

// static
std::unique_ptr<AccessibilityEventRecorder> AccessibilityEventRecorder::Create(
BrowserAccessibilityManager* manager,
base::ProcessId pid,
const AXTreeSelector& selector) {
if (!selector.pattern.empty()) {
LOG(FATAL) << "Recording accessibility events from an application name "
"match pattern not supported on this platform yet.";
}

return std::make_unique<AccessibilityEventRecorderWin>(manager, pid,
selector.pattern);
}

std::vector<AccessibilityEventRecorder::TestPass>
AccessibilityEventRecorder::GetTestPasses() {
// In addition to the 'Blink' pass, Windows includes two accessibility APIs
// that need to be tested independently (MSAA & UIA); the Blink pass uses the
// same recorder as the MSAA pass.
return {
{"blink", &AccessibilityEventRecorder::Create},
{"win", &AccessibilityEventRecorder::Create},
{"uia", &AccessibilityEventRecorderUia::CreateUia},
};
}

// static
CALLBACK void AccessibilityEventRecorderWin::WinEventHookThunk(
HWINEVENTHOOK handle,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
#include "base/win/scoped_variant.h"
#include "build/build_config.h"
#include "content/browser/accessibility/accessibility_browsertest.h"
#include "content/browser/accessibility/accessibility_event_recorder.h"
#include "content/browser/accessibility/accessibility_tree_formatter_utils_win.h"
#include "content/browser/accessibility/browser_accessibility_manager_win.h"
#include "content/browser/accessibility/browser_accessibility_state_impl.h"
Expand Down Expand Up @@ -835,8 +834,8 @@ class NativeWinEventWaiter {
NativeWinEventWaiter(BrowserAccessibilityManager* manager,
const std::string& match_pattern)
: event_recorder_(
AccessibilityEventRecorder::Create(manager,
base::GetCurrentProcId())),
AXInspectFactory::CreatePlatformRecorder(manager,
base::GetCurrentProcId())),
match_pattern_(match_pattern) {
event_recorder_->ListenToEvents(base::BindRepeating(
&NativeWinEventWaiter::OnEvent, base::Unretained(this)));
Expand All @@ -851,7 +850,7 @@ class NativeWinEventWaiter {
void Wait() { run_loop_.Run(); }

private:
std::unique_ptr<AccessibilityEventRecorder> event_recorder_;
std::unique_ptr<ui::AXEventRecorder> event_recorder_;
std::string match_pattern_;
base::RunLoop run_loop_;
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,7 @@ using ui::AXTreeFormatter;

// DumpAccessibilityTestBase
DumpAccessibilityTestBase::DumpAccessibilityTestBase()
: event_recorder_factory_(nullptr),
enable_accessibility_after_navigating_(false),
: enable_accessibility_after_navigating_(false),
test_helper_(DumpAccessibilityTestHelper::TestPasses()[GetParam()]) {}

DumpAccessibilityTestBase::~DumpAccessibilityTestBase() {}
Expand Down Expand Up @@ -183,21 +182,7 @@ void DumpAccessibilityTestBase::ParseHtmlForExtraDirectives(

void DumpAccessibilityTestBase::RunTest(const base::FilePath file_path,
const char* file_dir) {
// Get all the tree formatters; the test is run independently on each one.
auto test_passes = DumpAccessibilityTestHelper::TestPasses();
auto event_recorders = AccessibilityEventRecorder::GetTestPasses();
CHECK(event_recorders.size() == test_passes.size());

// The current test number is supplied as a test parameter.
size_t current_pass = GetParam();
CHECK_LT(current_pass, test_passes.size());
CHECK_EQ(test_passes.size(), event_recorders.size());

event_recorder_factory_ = event_recorders[current_pass].create_recorder;

RunTestForPlatform(file_path, file_dir);

event_recorder_factory_ = nullptr;
}

void DumpAccessibilityTestBase::RunTestForPlatform(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@

#include "base/strings/string16.h"
#include "base/test/scoped_feature_list.h"
#include "content/browser/accessibility/accessibility_event_recorder.h"
#include "content/public/browser/ax_inspect_factory.h"
#include "content/public/test/content_browser_test.h"
#include "content/public/test/dump_accessibility_test_helper.h"
Expand All @@ -20,6 +19,7 @@
namespace content {

class BrowserAccessibility;
class BrowserAccessibilityManager;
class DumpAccessibilityTestHelper;

// Base class for an accessibility browsertest that takes an HTML file as
Expand Down Expand Up @@ -126,9 +126,6 @@ class DumpAccessibilityTestBase : public ContentBrowserTest,
// The node filters loaded from the test file.
std::vector<ui::AXNodeFilter> node_filters_;

// The current tree-formatter and event-recorder factories.
AccessibilityEventRecorder::EventRecorderFactory event_recorder_factory_;

// Whether we should enable accessibility after navigating to the page,
// otherwise we enable it first.
bool enable_accessibility_after_navigating_;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
#include "base/threading/thread_restrictions.h"
#include "build/build_config.h"
#include "build/chromeos_buildflags.h"
#include "content/browser/accessibility/accessibility_event_recorder.h"
#include "content/browser/accessibility/browser_accessibility.h"
#include "content/browser/accessibility/browser_accessibility_manager.h"
#include "content/browser/accessibility/browser_accessibility_state_impl.h"
Expand Down Expand Up @@ -101,7 +100,7 @@ class DumpAccessibilityEventsTest : public DumpAccessibilityTestBase {
std::string final_tree_;
};

bool IsRecordingComplete(AccessibilityEventRecorder& event_recorder,
bool IsRecordingComplete(ui::AXEventRecorder& event_recorder,
std::vector<std::string>& run_until) {
// If no @*-RUN-UNTIL-EVENT directives, then having any events is enough.
LOG(ERROR) << "=== IsRecordingComplete#1 run_until size=" << run_until.size();
Expand Down Expand Up @@ -139,9 +138,13 @@ std::vector<std::string> DumpAccessibilityEventsTest::Dump(
bool run_go_again = false;
std::vector<std::string> result;
do {
// Create a new Event Recorder for the run
std::unique_ptr<AccessibilityEventRecorder> event_recorder =
event_recorder_factory_(
// Create a new Event Recorder for the run.
size_t current_pass = GetParam();
auto test_passes = DumpAccessibilityTestHelper::TestPasses();

std::unique_ptr<ui::AXEventRecorder> event_recorder =
AXInspectFactory::CreateRecorder(
test_passes[current_pass],
web_contents->GetRootBrowserAccessibilityManager(), pid, {});
event_recorder->SetOnlyWebEvents(true);

Expand Down Expand Up @@ -237,17 +240,17 @@ void DumpAccessibilityEventsTest::RunEventTest(
// Parameterize the tests so that each test-pass is run independently.
struct DumpAccessibilityEventsTestPassToString {
std::string operator()(const ::testing::TestParamInfo<size_t>& i) const {
auto passes = AccessibilityEventRecorder::GetTestPasses();
auto passes = DumpAccessibilityTestHelper::TestPasses();
CHECK_LT(i.param, passes.size());
return passes[i.param].name;
return std::string(passes[i.param]);
}
};

INSTANTIATE_TEST_SUITE_P(
All,
DumpAccessibilityEventsTest,
::testing::Range(size_t{0},
AccessibilityEventRecorder::GetTestPasses().size()),
DumpAccessibilityTestHelper::TestPasses().size()),
DumpAccessibilityEventsTestPassToString());

IN_PROC_BROWSER_TEST_P(DumpAccessibilityEventsTest,
Expand Down
4 changes: 2 additions & 2 deletions content/browser/web_contents/web_contents_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@
#include "components/download/public/common/download_stats.h"
#include "components/rappor/public/rappor_utils.h"
#include "components/url_formatter/url_formatter.h"
#include "content/browser/accessibility/accessibility_event_recorder.h"
#include "content/browser/accessibility/browser_accessibility.h"
#include "content/browser/bad_message.h"
#include "content/browser/browser_main_loop.h"
Expand Down Expand Up @@ -4017,7 +4016,8 @@ void WebContentsImpl::RecordAccessibilityEvents(
auto* ax_mgr = GetOrCreateRootBrowserAccessibilityManager();
DCHECK(ax_mgr);
base::ProcessId pid = base::Process::Current().Pid();
event_recorder_ = content::AccessibilityEventRecorder::Create(ax_mgr, pid);
event_recorder_ =
content::AXInspectFactory::CreatePlatformRecorder(ax_mgr, pid);
event_recorder_->ListenToEvents(*callback);
} else {
if (event_recorder_) {
Expand Down
Loading

0 comments on commit a20abfb

Please sign in to comment.