Skip to content

Commit

Permalink
[M119 minibranch] Reland "Invalidate the entire document when turning…
Browse files Browse the repository at this point in the history
… on accessibility"

This reverts commit 7083fde.

Reason for revert: fixed failing test, by adding code to ensure AXContext is created in InspectorOverlayAgent::SetInspectTool()"

Fixed: 1495644

Original change's description:
> Revert "Invalidate the entire document when turning on accessibility"
>
> This reverts commit 8958c24.
>
> Reason for revert: Causing consistent failure on Linux ASAN LSAN
> Sample failure: https://ci.chromium.org/ui/p/chromium/builders/ci/Linux%20ASan%20LSan%20Tests%20(1)/119801/overview
>
> Original change's description:
> > Invalidate the entire document when turning on accessibility
> >
> > This uncovered a few cases where we need to update the lifecycle as
> > a result of this invalidation (printing, AOM, devtools). These
> > cases may have been pre-existing bugs.
> >
> > Bug: 1485824
> >
> > Change-Id: I17b64fc35c9a76ad90c7bb29cc183c5b71e866c9
> > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4970122
> > Reviewed-by: Mason Freed <masonf@chromium.org>
> > Auto-Submit: Chris Harrelson <chrishtr@chromium.org>
> > Commit-Queue: Chris Harrelson <chrishtr@chromium.org>
> > Cr-Commit-Position: refs/heads/main@{#1214578}
>
> Bug: 1485824
> Change-Id: I4f1c0d56100e474fd3d1d498877704dc4e47c87c
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4975220
> Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
> Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
> Owners-Override: Jiewei Qian <qjw@chromium.org>
> Auto-Submit: Jiewei Qian <qjw@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#1214654}

(cherry picked from commit 624434c)

(cherry picked from commit a84a4f6)

Bug: 1485824
Change-Id: I90bec1ced0e9b9ae2f91c73f11d087c266529e00
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4976744
Reviewed-by: Mason Freed <masonf@chromium.org>
Commit-Queue: Chris Harrelson <chrishtr@chromium.org>
Cr-Original-Original-Commit-Position: refs/heads/main@{#1214997}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4977280
Owners-Override: Prudhvikumar Bommana <pbommana@google.com>
Cr-Original-Commit-Position: refs/branch-heads/6045@{#965}
Cr-Original-Branched-From: 905e8bd-refs/heads/main@{#1204232}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4984808
Owners-Override: Krishna Govind <govind@chromium.org>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Reviewed-by: Krishna Govind <govind@chromium.org>
Cr-Commit-Position: refs/branch-heads/6045_53@{#3}
Cr-Branched-From: 7ff9f59-refs/branch-heads/6045@{#929}
Cr-Branched-From: 905e8bd-refs/heads/main@{#1204232}
  • Loading branch information
chrishtr authored and Krishna Govind committed Oct 27, 2023
1 parent e9528db commit d11f31e
Show file tree
Hide file tree
Showing 9 changed files with 34 additions and 7 deletions.
1 change: 1 addition & 0 deletions third_party/blink/renderer/core/css/style_change_reason.cc
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
namespace blink {

namespace style_change_reason {
const char kAccessibility[] = "Accessibility";
const char kActiveStylesheetsUpdate[] = "ActiveStylesheetsUpdate";
const char kAnimation[] = "Animation";
const char kAttribute[] = "Attribute";
Expand Down
1 change: 1 addition & 0 deletions third_party/blink/renderer/core/css/style_change_reason.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ namespace blink {
class QualifiedName;

namespace style_change_reason {
extern const char kAccessibility[];
extern const char kActiveStylesheetsUpdate[];
extern const char kAnimation[];
extern const char kAttribute[];
Expand Down
19 changes: 18 additions & 1 deletion third_party/blink/renderer/core/dom/document.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2759,11 +2759,20 @@ void Document::EnsurePaintLocationDataValidForNode(

void Document::GetPageDescription(uint32_t page_index,
WebPrintPageDescription* description) {
GetPageDescription(*StyleForPage(page_index), description);
View()->UpdateLifecycleToLayoutClean(DocumentUpdateReason::kUnknown);
GetPageDescriptionNoLifecycleUpdate(*StyleForPage(page_index), description);
}

void Document::GetPageDescription(const ComputedStyle& style,
WebPrintPageDescription* description) {
View()->UpdateLifecycleToLayoutClean(DocumentUpdateReason::kUnknown);
GetPageDescriptionNoLifecycleUpdate(style, description);
}

void Document::GetPageDescriptionNoLifecycleUpdate(
const ComputedStyle& style,
WebPrintPageDescription* description) {
DocumentLifecycle::DisallowTransitionScope scope(Lifecycle());
const WebPrintPageDescription input_description = *description;

switch (style.GetPageSizeType()) {
Expand Down Expand Up @@ -3166,6 +3175,14 @@ void Document::AddAXContext(AXContext* context) {
if (!ax_object_cache_) {
ax_object_cache_ =
AXObjectCache::Create(*this, ComputeAXModeFromAXContexts(ax_contexts_));
// Invalidate style on the entire document, because accessibility
// needs to compute style on all elements, even those in
// content-visibility:auto subtrees.
if (documentElement()) {
documentElement()->SetNeedsStyleRecalc(
kSubtreeStyleChange, StyleChangeReasonForTracing::Create(
style_change_reason::kAccessibility));
}
g_ax_object_cache_count++;
}
}
Expand Down
5 changes: 3 additions & 2 deletions third_party/blink/renderer/core/dom/document.h
Original file line number Diff line number Diff line change
Expand Up @@ -758,10 +758,11 @@ class CORE_EXPORT Document : public ContainerNode,
// Gets the description for the specified page. This includes preferred page
// size and margins in pixels, assuming 96 pixels per inch. The size and
// margins must be initialized to the default values that are used if auto is
// specified. Note that, if the |page_index| variant of the function is used,
// layout needs to be complete, since page names are determined during layout.
// specified. Updates layout as needed to get the description.
void GetPageDescription(uint32_t page_index, WebPrintPageDescription*);
void GetPageDescription(const ComputedStyle&, WebPrintPageDescription*);
void GetPageDescriptionNoLifecycleUpdate(const ComputedStyle&,
WebPrintPageDescription*);

ResourceFetcher* Fetcher() const { return fetcher_.Get(); }

Expand Down
4 changes: 2 additions & 2 deletions third_party/blink/renderer/core/dom/element.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2185,11 +2185,11 @@ const AtomicString& Element::computedRole() {
if (!document.IsActive() || !document.View()) {
return g_null_atom;
}
AXContext ax_context(document, ui::kAXModeBasic);
if (document.Lifecycle().GetState() < DocumentLifecycle::kPrePaintClean) {
document.View()->UpdateAllLifecyclePhasesExceptPaint(
DocumentUpdateReason::kJavaScript);
}
AXContext ax_context(document, ui::kAXModeBasic);
ax_context.GetAXObjectCache().ProcessDeferredAccessibilityEvents(document);
return ax_context.GetAXObjectCache().ComputedRoleForNode(this);
}
Expand All @@ -2199,11 +2199,11 @@ String Element::computedName() {
if (!document.IsActive() || !document.View()) {
return String();
}
AXContext ax_context(document, ui::kAXModeBasic);
if (document.Lifecycle().GetState() < DocumentLifecycle::kPrePaintClean) {
document.View()->UpdateAllLifecyclePhasesExceptPaint(
DocumentUpdateReason::kJavaScript);
}
AXContext ax_context(document, ui::kAXModeBasic);
ax_context.GetAXObjectCache().ProcessDeferredAccessibilityEvents(document);
return ax_context.GetAXObjectCache().ComputedNameForNode(this);
}
Expand Down
2 changes: 2 additions & 0 deletions third_party/blink/renderer/core/frame/local_frame_view.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4029,6 +4029,8 @@ void LocalFrameView::PaintOutsideOfLifecycle(GraphicsContext& context,
const CullRect& cull_rect) {
DCHECK(PaintOutsideOfLifecycleIsAllowed(context, *this));

UpdateAllLifecyclePhasesExceptPaint(DocumentUpdateReason::kPrinting);

SCOPED_UMA_AND_UKM_TIMER(GetUkmAggregator(), LocalFrameUkmAggregator::kPaint);

ForAllNonThrottledLocalFrameViews([](LocalFrameView& frame_view) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -481,7 +481,10 @@ protocol::Response InspectorOverlayAgent::enable() {
}

void InspectorOverlayAgent::EnsureAXContext(Node* node) {
Document& document = node->GetDocument();
EnsureAXContext(node->GetDocument());
}

void InspectorOverlayAgent::EnsureAXContext(Document& document) {
if (!document_to_ax_context_.Contains(&document)) {
auto context = std::make_unique<AXContext>(document, ui::kAXModeComplete);
document_to_ax_context_.Set(&document, std::move(context));
Expand Down Expand Up @@ -1641,6 +1644,7 @@ protocol::Response InspectorOverlayAgent::SetInspectTool(
LoadOverlayPageResource();
EvaluateInOverlay("setOverlay", inspect_tool->GetOverlayName());
EnsureEnableFrameOverlay();
EnsureAXContext(frame->GetDocument());
ScheduleUpdate();
return protocol::Response::Success();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,7 @@ class CORE_EXPORT InspectorOverlayAgent final

void Inspect(Node*);
void EnsureAXContext(Node*);
void EnsureAXContext(Document&);
void DispatchBufferedTouchEvents();
void SetPageIsScrolling(bool is_scrolling);
WebInputEventResult HandleInputEvent(const WebInputEvent&);
Expand Down
2 changes: 1 addition & 1 deletion third_party/blink/renderer/core/layout/layout_view.cc
Original file line number Diff line number Diff line change
Expand Up @@ -676,7 +676,7 @@ PhysicalSize LayoutView::PageAreaSize(wtf_size_t page_index,
const ComputedStyle* page_style =
GetDocument().StyleForPage(page_index, page_name);
WebPrintPageDescription description = default_page_description_;
GetDocument().GetPageDescription(*page_style, &description);
GetDocument().GetPageDescriptionNoLifecycleUpdate(*page_style, &description);

gfx::SizeF page_size(
std::max(.0f, description.size.width() -
Expand Down

0 comments on commit d11f31e

Please sign in to comment.