Skip to content

Commit

Permalink
Move a11y to post-lifecycle steps (iteration #4)
Browse files Browse the repository at this point in the history 
A11y is moved to the end of the document lifecycle, where it can
occur in parallel with other tasks.

Parts of the code that used to force layout updates in order to
update the AX tree now call UpdateAXForAllDocuments(), a new
method introduced in the previous CL, which first updates the
lifecycle and then manually updates the AX tree.

Additional code cleanups in AXObjectCacheImpl related to
popups were also now possible.

Test changes:
* Changes to css-display-expected-win.txt and content-editable-notifications.html were due to a subtle change in event ordering,
but are still correct.

Follow-ups:
* Try DCHECK(!IsFrozen()) inside of UpdateAXForAllDocuments().
* Update SCOPED_DISALLOW_LIFECYCLE_TRANSITION to use document_ and
  popup_document_ and remove the argument.

Bug: None
Change-Id: If01ee1b43672e58be5a120eaa7cc9572170f6257
Cq-Do-Not-Cancel-Tryjobs: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3964415
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Commit-Queue: Aaron Leventhal <aleventhal@chromium.org>
Reviewed-by: Stefan Zager <szager@chromium.org>
Reviewed-by: David Tseng <dtseng@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1062392}
  • Loading branch information
@aleventhal
aleventhal authored and Chromium LUCI CQ committed Oct 21, 2022
1 parent 57da010 commit da5d2ff
Show file tree
Hide file tree
Showing 13 changed files with 106 additions and 341 deletions.
13 changes: 4 additions & 9 deletions content/renderer/accessibility/render_accessibility_impl.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -371,7 +371,7 @@ void RenderAccessibilityImpl::HitTest(
}

void RenderAccessibilityImpl::PerformAction(const ui::AXActionData& data) {
const WebDocument& document = GetMainDocument();
WebDocument document = GetMainDocument();
if (document.IsNull())
return;

Expand Down Expand Up @@ -1147,16 +1147,11 @@ void RenderAccessibilityImpl::SendPendingAccessibilityEvents() {

#if DCHECK_IS_ON()
// Protect against lifecycle changes in the popup document, if any.
// If no popup document, use the main document -- it's harmless to protect it
// twice, and some document is needed because this cannot be done in an if
// statement because it's scoped.
WebDocument popup_document = GetPopupDocument();
WebDocument popup_or_main_document =
popup_document.IsNull() ? document : GetPopupDocument();
std::unique_ptr<blink::WebDisallowTransitionScope> disallow2;
if (!image_annotation_debugging_) {
disallow = std::make_unique<blink::WebDisallowTransitionScope>(
&popup_or_main_document);
if (!image_annotation_debugging_ && !popup_document.IsNull()) {
disallow2 =
std::make_unique<blink::WebDisallowTransitionScope>(&popup_document);
}
#endif

Expand Down
164 changes: 0 additions & 164 deletions content/renderer/accessibility/render_accessibility_impl_browsertest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -465,170 +465,6 @@ TEST_F(RenderAccessibilityImplTest, SendFullAccessibilityTreeOnReload) {
EXPECT_EQ(6, CountAccessibilityNodesSentToBrowser());
}

TEST_F(RenderAccessibilityImplTest, TestDeferred) {
constexpr char html[] = R"HTML(
<body>
<div>
a
</div>
</body>
)HTML";
LoadHTML(html);
task_environment_.RunUntilIdle();

// We should have had load complete. Subsequent events are deferred unless
// there is a user interaction.
ExpectScheduleStatusScheduledDeferred();
ExpectScheduleModeDeferEvents();

// Simulate a page load to test deferred behavior.
GetRenderAccessibilityImpl()->DidCommitProvisionalLoad(
ui::PageTransition::PAGE_TRANSITION_LINK);
ClearHandledUpdates();
WebDocument document = GetMainFrame()->GetDocument();
EXPECT_FALSE(document.IsNull());
WebAXObject root_obj = WebAXObject::FromWebDocument(document);
EXPECT_FALSE(root_obj.IsNull());

// No events should have been scheduled or sent.
ExpectScheduleStatusNotWaiting();
ExpectScheduleModeDeferEvents();

// Send a non-interactive event, it should be scheduled with a delay.
GetRenderAccessibilityImpl()->HandleAXEvent(
ui::AXEvent(root_obj.AxID(), ax::mojom::Event::kLocationChanged));
ExpectScheduleStatusScheduledDeferred();
ExpectScheduleModeDeferEvents();

task_environment_.RunUntilIdle();
// Ensure event is not sent as it is scheduled with a delay.
ExpectScheduleStatusScheduledDeferred();
ExpectScheduleModeDeferEvents();

// Perform action, causing immediate event processing.
ui::AXActionData action;
action.action = ax::mojom::Action::kFocus;
GetRenderAccessibilityImpl()->PerformAction(action);
ScheduleSendPendingAccessibilityEvents();

// Once in immediate mode, stays in immediate mode until events are sent.
GetRenderAccessibilityImpl()->HandleAXEvent(
ui::AXEvent(root_obj.AxID(), ax::mojom::Event::kLocationChanged));
ExpectScheduleStatusScheduledImmediate();
ExpectScheduleModeProcessEventsImmediately();

// Once events have been sent, defer next batch.
ScheduleSendPendingAccessibilityEvents();
task_environment_.RunUntilIdle();
ExpectScheduleStatusScheduledDeferred();
ExpectScheduleModeDeferEvents();

const std::vector<ax::mojom::Event> kNonInteractiveEvents = {
ax::mojom::Event::kAriaAttributeChanged,
ax::mojom::Event::kChildrenChanged,
ax::mojom::Event::kDocumentTitleChanged,
ax::mojom::Event::kExpandedChanged,
ax::mojom::Event::kHide,
ax::mojom::Event::kLayoutComplete,
ax::mojom::Event::kLocationChanged,
ax::mojom::Event::kMenuListValueChanged,
ax::mojom::Event::kRowCollapsed,
ax::mojom::Event::kRowCountChanged,
ax::mojom::Event::kRowExpanded,
ax::mojom::Event::kScrollPositionChanged,
ax::mojom::Event::kScrolledToAnchor,
ax::mojom::Event::kSelectedChildrenChanged,
ax::mojom::Event::kShow,
ax::mojom::Event::kTextChanged};

for (ax::mojom::Event event : kNonInteractiveEvents) {
// Send an interactive event, it should be scheduled with a delay.
GetRenderAccessibilityImpl()->HandleAXEvent(
ui::AXEvent(root_obj.AxID(), event));
ExpectScheduleModeDeferEvents();
}

ScheduleSendPendingAccessibilityEvents();
ExpectScheduleStatusScheduledDeferred();

const std::vector<ax::mojom::Event> kInteractiveEvents = {
ax::mojom::Event::kActiveDescendantChanged,
ax::mojom::Event::kBlur,
ax::mojom::Event::kCheckedStateChanged,
ax::mojom::Event::kClicked,
ax::mojom::Event::kDocumentSelectionChanged,
ax::mojom::Event::kFocus,
ax::mojom::Event::kHover,
ax::mojom::Event::kLoadComplete,
ax::mojom::Event::kValueChanged};

for (ax::mojom::Event event : kInteractiveEvents) {
// Once events have been sent, defer next batch.
task_environment_.RunUntilIdle();
ExpectScheduleModeDeferEvents();
ExpectScheduleStatusScheduledDeferred();

// Send an interactive event, it should be scheduled with a delay.
GetRenderAccessibilityImpl()->HandleAXEvent(
ui::AXEvent(root_obj.AxID(), event));
ExpectScheduleModeProcessEventsImmediately();
ExpectScheduleStatusScheduledImmediate();

ScheduleSendPendingAccessibilityEvents();
}

task_environment_.RunUntilIdle();

// Event has been sent, no longer waiting on ack.
ExpectScheduleStatusScheduledDeferred();
ExpectScheduleModeDeferEvents();
}

TEST_F(RenderAccessibilityImplTest, TestChangesOnFocusModeAreImmediate) {
LoadHTML(R"HTML(
<body>
<div id=a tabindex=0>
a
</div>
<script>document.getElementById('a').focus();</script>
</body>
)HTML");
task_environment_.RunUntilIdle();

// We should have had load complete. Subsequent events are deferred unless
// there is a user interaction.
ExpectScheduleStatusScheduledDeferred();
ExpectScheduleModeDeferEvents();

// Simulate a page load to test deferred behavior.
GetRenderAccessibilityImpl()->DidCommitProvisionalLoad(
ui::PageTransition::PAGE_TRANSITION_LINK);
ClearHandledUpdates();
WebDocument document = GetMainFrame()->GetDocument();
EXPECT_FALSE(document.IsNull());
WebAXObject root_obj = WebAXObject::FromWebDocument(document);
EXPECT_FALSE(root_obj.IsNull());

WebAXObject html = root_obj.ChildAt(0);
WebAXObject body = html.ChildAt(0);
WebAXObject node_a = body.ChildAt(0);

// No events should have been scheduled or sent.
ExpectScheduleStatusNotWaiting();
ExpectScheduleModeDeferEvents();

// Marking the focused object dirty causes changes to be sent immediately.
GetRenderAccessibilityImpl()->MarkWebAXObjectDirty(node_a, false);
ExpectScheduleStatusScheduledImmediate();
ExpectScheduleModeProcessEventsImmediately();

task_environment_.RunUntilIdle();

// Event has been sent, no longer waiting on ack.
ExpectScheduleStatusScheduledDeferred();
ExpectScheduleModeDeferEvents();
}

TEST_F(RenderAccessibilityImplTest, HideAccessibilityObject) {
// Test RenderAccessibilityImpl and make sure it sends the
// proper event to the browser when an object in the tree
Expand Down
4 changes: 2 additions & 2 deletions content/test/data/accessibility/event/css-display-expected-win.txt
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
EVENT_OBJECT_HIDE on <div.a> role=ROLE_SYSTEM_GROUPING name="Heading" level=2
EVENT_OBJECT_REORDER on <div> role=ROLE_SYSTEM_TOOLBAR IA2_STATE_HORIZONTAL
EVENT_OBJECT_SHOW on <div.b> role=ROLE_SYSTEM_GROUPING name="Banner"
IA2_EVENT_TEXT_INSERTED on <div> role=ROLE_SYSTEM_TOOLBAR IA2_STATE_HORIZONTAL new_text={'<obj>' start=0 end=1}
IA2_EVENT_TEXT_REMOVED on <div> role=ROLE_SYSTEM_TOOLBAR IA2_STATE_HORIZONTAL old_text={'<obj>' start=0 end=1}
IA2_EVENT_TEXT_INSERTED on <div> role=ROLE_SYSTEM_TOOLBAR IA2_STATE_HORIZONTAL new_text={'<obj>' start=1 end=2}
IA2_EVENT_TEXT_REMOVED on <div> role=ROLE_SYSTEM_TOOLBAR IA2_STATE_HORIZONTAL old_text={'<obj>' start=0 end=1}
26 changes: 2 additions & 24 deletions third_party/blink/renderer/core/dom/document_lifecycle.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -128,19 +128,6 @@ bool DocumentLifecycle::CanAdvanceTo(LifecycleState next_state) const {
return true;
if (next_state == kStyleClean)
return true;
// InAccessibility only runs if there is an ExistingAXObjectCache.
if (next_state == kInAccessibility)
return true;
if (next_state == kInCompositingInputsUpdate)
return true;
if (next_state == kInPrePaint)
return true;
break;
case kInAccessibility:
if (next_state == kAccessibilityClean)
return true;
break;
case kAccessibilityClean:
if (next_state == kInCompositingInputsUpdate)
return true;
if (next_state == kInPrePaint)
Expand All @@ -156,8 +143,6 @@ bool DocumentLifecycle::CanAdvanceTo(LifecycleState next_state) const {
return true;
if (next_state == kInPrePaint)
return true;
if (next_state == kInAccessibility)
return true;
break;
case kInPrePaint:
if (next_state == kPrePaintClean)
Expand All @@ -172,8 +157,6 @@ bool DocumentLifecycle::CanAdvanceTo(LifecycleState next_state) const {
return true;
if (next_state == kInPrePaint)
return true;
if (next_state == kInAccessibility)
return true;
break;
case kInPaint:
if (next_state == kPaintClean)
Expand All @@ -188,8 +171,6 @@ bool DocumentLifecycle::CanAdvanceTo(LifecycleState next_state) const {
return true;
if (next_state == kInPaint)
return true;
if (next_state == kInAccessibility)
return true;
break;
case kStopping:
return next_state == kStopped;
Expand All @@ -209,9 +190,8 @@ bool DocumentLifecycle::CanRewindTo(LifecycleState next_state) const {
next_state == g_deprecated_transition_stack->To())
return true;
return state_ == kStyleClean || state_ == kAfterPerformLayout ||
state_ == kLayoutClean || state_ == kAccessibilityClean ||
state_ == kCompositingInputsClean || state_ == kPrePaintClean ||
state_ == kPaintClean;
state_ == kLayoutClean || state_ == kCompositingInputsClean ||
state_ == kPrePaintClean || state_ == kPaintClean;
}

#define DEBUG_STRING_CASE(StateName) \
Expand All @@ -229,8 +209,6 @@ static WTF::String StateAsDebugString(
DEBUG_STRING_CASE(kInPerformLayout);
DEBUG_STRING_CASE(kAfterPerformLayout);
DEBUG_STRING_CASE(kLayoutClean);
DEBUG_STRING_CASE(kInAccessibility);
DEBUG_STRING_CASE(kAccessibilityClean);
DEBUG_STRING_CASE(kInCompositingInputsUpdate);
DEBUG_STRING_CASE(kCompositingInputsClean);
DEBUG_STRING_CASE(kInPrePaint);
Expand Down
5 changes: 0 additions & 5 deletions third_party/blink/renderer/core/dom/document_lifecycle.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,11 +62,6 @@ class CORE_EXPORT DocumentLifecycle {
kAfterPerformLayout,
kLayoutClean,

// In InAccessibility step, fire deferred accessibility events which
// require layout to be in a clean state.
kInAccessibility,
kAccessibilityClean,

kInCompositingInputsUpdate,
kCompositingInputsClean,

Expand Down
2 changes: 2 additions & 0 deletions third_party/blink/renderer/core/dom/element.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2129,6 +2129,7 @@ const AtomicString& Element::computedRole() {
DocumentUpdateReason::kJavaScript);
}
AXContext ax_context(document, ui::kAXModeBasic);
ax_context.GetAXObjectCache().ProcessDeferredAccessibilityEvents(document);
return ax_context.GetAXObjectCache().ComputedRoleForNode(this);
}

Expand All @@ -2142,6 +2143,7 @@ String Element::computedName() {
DocumentUpdateReason::kJavaScript);
}
AXContext ax_context(document, ui::kAXModeBasic);
ax_context.GetAXObjectCache().ProcessDeferredAccessibilityEvents(document);
return ax_context.GetAXObjectCache().ComputedNameForNode(this);
}

Expand Down
19 changes: 3 additions & 16 deletions third_party/blink/renderer/core/frame/local_frame_view.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1041,6 +1041,7 @@ void LocalFrameView::RunPostLifecycleSteps() {
base::AutoReset<bool> in_post_lifecycle_steps(&in_post_lifecycle_steps_,
true);
AllowThrottlingScope allow_throttling(*this);
RunAccessibilitySteps();
RunIntersectionObserverSteps();
if (mobile_friendliness_checker_)
mobile_friendliness_checker_->MaybeRecompute();
Expand Down Expand Up @@ -2253,7 +2254,6 @@ bool LocalFrameView::UpdateLifecyclePhases(

// Only the following target states are supported.
DCHECK(target_state == DocumentLifecycle::kLayoutClean ||
target_state == DocumentLifecycle::kAccessibilityClean ||
target_state == DocumentLifecycle::kCompositingInputsClean ||
target_state == DocumentLifecycle::kPrePaintClean ||
target_state == DocumentLifecycle::kPaintClean);
Expand Down Expand Up @@ -2434,13 +2434,6 @@ void LocalFrameView::UpdateLifecyclePhasesInternal(
DisallowLayoutInvalidationScope disallow_layout_invalidation(this);
#endif

DCHECK_GE(target_state, DocumentLifecycle::kAccessibilityClean);
run_more_lifecycle_phases = RunAccessibilityLifecyclePhase(target_state);
DCHECK(ShouldThrottleRendering() || !ExistingAXObjectCache() ||
Lifecycle().GetState() == DocumentLifecycle::kAccessibilityClean);
if (!run_more_lifecycle_phases)
return;

DEVTOOLS_TIMELINE_TRACE_EVENT_INSTANT_WITH_CATEGORIES(
TRACE_DISABLED_BY_DEFAULT("devtools.timeline"), "SetLayerTreeId",
inspector_set_layer_tree_id::Data, frame_.Get());
Expand Down Expand Up @@ -2849,10 +2842,8 @@ void LocalFrameView::RunPaintLifecyclePhase(PaintBenchmarkMode benchmark_mode) {
GetPage()->Animator().ReportFrameAnimations(GetCompositorAnimationHost());
}

bool LocalFrameView::RunAccessibilityLifecyclePhase(
DocumentLifecycle::LifecycleState target_state) {
TRACE_EVENT0("blink,benchmark",
"LocalFrameView::RunAccessibilityLifecyclePhase");
void LocalFrameView::RunAccessibilitySteps() {
TRACE_EVENT0("blink,benchmark", "LocalFrameView::RunAccessibilitySteps");

SCOPED_UMA_AND_UKM_TIMER(EnsureUkmAggregator(),
LocalFrameUkmAggregator::kAccessibility);
Expand All @@ -2863,14 +2854,10 @@ bool LocalFrameView::RunAccessibilityLifecyclePhase(

ForAllNonThrottledLocalFrameViews([](LocalFrameView& frame_view) {
if (AXObjectCache* cache = frame_view.ExistingAXObjectCache()) {
frame_view.Lifecycle().AdvanceTo(DocumentLifecycle::kInAccessibility);
cache->ProcessDeferredAccessibilityEvents(
*frame_view.GetFrame().GetDocument());
frame_view.Lifecycle().AdvanceTo(DocumentLifecycle::kAccessibilityClean);
}
});

return target_state > DocumentLifecycle::kAccessibilityClean;
}

void LocalFrameView::EnqueueScrollAnchoringAdjustment(
Expand Down
3 changes: 1 addition & 2 deletions third_party/blink/renderer/core/frame/local_frame_view.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -894,8 +894,6 @@ class CORE_EXPORT LocalFrameView final
// earlier if we don't need to run future lifecycle phases.
bool RunStyleAndLayoutLifecyclePhases(
DocumentLifecycle::LifecycleState target_state);
bool RunAccessibilityLifecyclePhase(
DocumentLifecycle::LifecycleState target_state);
bool RunCompositingInputsLifecyclePhase(
DocumentLifecycle::LifecycleState target_state);
bool RunPrePaintLifecyclePhase(
Expand All @@ -916,6 +914,7 @@ class CORE_EXPORT LocalFrameView final

DocumentLifecycle& Lifecycle() const;

void RunAccessibilitySteps();
void RunIntersectionObserverSteps();
void RenderThrottlingStatusChanged();

Expand Down
Loading

0 comments on commit da5d2ff

Please sign in to comment.