New issue
Advanced search Search tips

Issue 867324 link

Starred by 1 user
Status: Fixed
Owner:
dmazz...@chromium.org
Closed: Sep 10
Cc:
aboxhall@chromium.org
dmazz...@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug
Team-Accessibility



Sign in to add a comment

Blink should remove the accessibilityEnabled setting in favor of reference-counting

Project Member Reported by dmazz...@chromium.org, Jul 25 
Here are some problems we should solve:

If accessibility is enabled after a ScopedAXObjectCache is created, there will be two AXObjectCaches.

If accessibility is disabled while a ScopedAXObjectCache is still alive, it will stop working because it was just wrapping the Document's AXObjectCache.

If accessibility is disabled, two simultaneous uses of ScopedAXObjectCache can't share the same underlying cache, they'll each duplicate the effort.

None of these are critical issues today because they only affect developers who are using the Inspector or testing AOM. However as we get closer to AOM shipping it's important to solve these issues correctly.

Instead perhaps we should have a class AXContext that keeps accessibility support alive for a particular Document while it's in scope.

Comment 1 by dmazz...@chromium.org, Jul 25 

Googlers, see this design doc for more detail: go/axcontext
Project Member

Comment 2 by bugdroid1@chromium.org, Aug 23 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/82bb230ce4500058b2593879c67279749f66491f

commit 82bb230ce4500058b2593879c67279749f66491f
Author: Dominic Mazzoni <dmazzoni@chromium.org>
Date: Thu Aug 23 20:42:05 2018

Use new class AXContext to control enabling Blink accessibility.

With this change, creating an AXContext for a Document is the
only way to enable accessibility. As long as there's at least
one AXContext associated with a Document, it will have an
AXObjectCache, and as soon as the last AXContext goes out of
scope, the Document will destroy its AXObjectCache.

This completely replaces the accessibilityEnabled setting
and ScopedAXObjectCache, which interacted poorly with each
other.

See bug and mini design doc (go/axcontext) for more details.

Bug:  867324 
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_layout_ng
Change-Id: Ia84372ec49c93931ea8c2de11ab909475893277c
Reviewed-on: https://chromium-review.googlesource.com/1150824
Commit-Queue: Dominic Mazzoni <dmazzoni@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Alice Boxhall <aboxhall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#585606}
[modify] https://crrev.com/82bb230ce4500058b2593879c67279749f66491f/components/pdf/renderer/pdf_accessibility_tree_browsertest.cc
[modify] https://crrev.com/82bb230ce4500058b2593879c67279749f66491f/content/renderer/accessibility/render_accessibility_impl.cc
[modify] https://crrev.com/82bb230ce4500058b2593879c67279749f66491f/content/renderer/accessibility/render_accessibility_impl.h
[modify] https://crrev.com/82bb230ce4500058b2593879c67279749f66491f/content/renderer/render_frame_impl.cc
[modify] https://crrev.com/82bb230ce4500058b2593879c67279749f66491f/content/shell/test_runner/accessibility_controller.cc
[modify] https://crrev.com/82bb230ce4500058b2593879c67279749f66491f/content/shell/test_runner/accessibility_controller.h
[modify] https://crrev.com/82bb230ce4500058b2593879c67279749f66491f/third_party/blink/public/BUILD.gn
[add] https://crrev.com/82bb230ce4500058b2593879c67279749f66491f/third_party/blink/public/web/web_ax_context.h
[modify] https://crrev.com/82bb230ce4500058b2593879c67279749f66491f/third_party/blink/public/web/web_ax_object.h
[modify] https://crrev.com/82bb230ce4500058b2593879c67279749f66491f/third_party/blink/public/web/web_settings.h
[modify] https://crrev.com/82bb230ce4500058b2593879c67279749f66491f/third_party/blink/renderer/core/accessibility/BUILD.gn
[add] https://crrev.com/82bb230ce4500058b2593879c67279749f66491f/third_party/blink/renderer/core/accessibility/ax_context.cc
[add] https://crrev.com/82bb230ce4500058b2593879c67279749f66491f/third_party/blink/renderer/core/accessibility/ax_context.h
[modify] https://crrev.com/82bb230ce4500058b2593879c67279749f66491f/third_party/blink/renderer/core/accessibility/ax_object_cache.cc
[modify] https://crrev.com/82bb230ce4500058b2593879c67279749f66491f/third_party/blink/renderer/core/accessibility/ax_object_cache.h
[modify] https://crrev.com/82bb230ce4500058b2593879c67279749f66491f/third_party/blink/renderer/core/aom/computed_accessible_node.cc
[modify] https://crrev.com/82bb230ce4500058b2593879c67279749f66491f/third_party/blink/renderer/core/aom/computed_accessible_node.h
[modify] https://crrev.com/82bb230ce4500058b2593879c67279749f66491f/third_party/blink/renderer/core/dom/container_node.cc
[modify] https://crrev.com/82bb230ce4500058b2593879c67279749f66491f/third_party/blink/renderer/core/dom/document.cc
[modify] https://crrev.com/82bb230ce4500058b2593879c67279749f66491f/third_party/blink/renderer/core/dom/document.h
[modify] https://crrev.com/82bb230ce4500058b2593879c67279749f66491f/third_party/blink/renderer/core/dom/element.cc
[modify] https://crrev.com/82bb230ce4500058b2593879c67279749f66491f/third_party/blink/renderer/core/dom/node.cc
[modify] https://crrev.com/82bb230ce4500058b2593879c67279749f66491f/third_party/blink/renderer/core/editing/editor.cc
[modify] https://crrev.com/82bb230ce4500058b2593879c67279749f66491f/third_party/blink/renderer/core/exported/web_page_popup_impl.cc
[modify] https://crrev.com/82bb230ce4500058b2593879c67279749f66491f/third_party/blink/renderer/core/exported/web_settings_impl.cc
[modify] https://crrev.com/82bb230ce4500058b2593879c67279749f66491f/third_party/blink/renderer/core/exported/web_settings_impl.h
[modify] https://crrev.com/82bb230ce4500058b2593879c67279749f66491f/third_party/blink/renderer/core/frame/local_dom_window.cc
[modify] https://crrev.com/82bb230ce4500058b2593879c67279749f66491f/third_party/blink/renderer/core/frame/local_frame_view.cc
[modify] https://crrev.com/82bb230ce4500058b2593879c67279749f66491f/third_party/blink/renderer/core/frame/settings.json5
[modify] https://crrev.com/82bb230ce4500058b2593879c67279749f66491f/third_party/blink/renderer/core/frame/visual_viewport_test.cc
[modify] https://crrev.com/82bb230ce4500058b2593879c67279749f66491f/third_party/blink/renderer/core/html/forms/picker_indicator_element.cc
[modify] https://crrev.com/82bb230ce4500058b2593879c67279749f66491f/third_party/blink/renderer/core/layout/layout_object_child_list.cc
[modify] https://crrev.com/82bb230ce4500058b2593879c67279749f66491f/third_party/blink/renderer/modules/accessibility/accessibility_object_model_test.cc
[modify] https://crrev.com/82bb230ce4500058b2593879c67279749f66491f/third_party/blink/renderer/modules/accessibility/ax_object_cache_impl.cc
[modify] https://crrev.com/82bb230ce4500058b2593879c67279749f66491f/third_party/blink/renderer/modules/accessibility/inspector_accessibility_agent.cc
[modify] https://crrev.com/82bb230ce4500058b2593879c67279749f66491f/third_party/blink/renderer/modules/accessibility/testing/accessibility_test.cc
[modify] https://crrev.com/82bb230ce4500058b2593879c67279749f66491f/third_party/blink/renderer/modules/accessibility/testing/accessibility_test.h
[modify] https://crrev.com/82bb230ce4500058b2593879c67279749f66491f/third_party/blink/renderer/modules/canvas/canvas2d/canvas_rendering_context_2d_api_test.cc
[modify] https://crrev.com/82bb230ce4500058b2593879c67279749f66491f/third_party/blink/renderer/modules/exported/BUILD.gn
[add] https://crrev.com/82bb230ce4500058b2593879c67279749f66491f/third_party/blink/renderer/modules/exported/web_ax_context.cc
[modify] https://crrev.com/82bb230ce4500058b2593879c67279749f66491f/third_party/blink/renderer/modules/exported/web_ax_object.cc
Project Member

Comment 3 by bugdroid1@chromium.org, Sep 10 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/34b81f99f94e8f54f8f676a06d4a8bdcd02c13e5

commit 34b81f99f94e8f54f8f676a06d4a8bdcd02c13e5
Author: Dominic Mazzoni <dmazzoni@chromium.org>
Date: Mon Sep 10 23:06:35 2018

Make AXObjectCache garbage-collected

This is a follow-up to http://crrev.com/c/1150824 - instead of having
Document keep track of its AXContexts, let the garbage collector do the
work for us. Each AXContext owns a Persistent<AXObjectCache>, and
the associated Document has a WeakPersistent<AXObjectCache>. That way
when the last AXContext goes out of scope, the AXObjectCache will be
cleaned up automatically.

This requires a few changes due to assumptions that AXObjectCache couldn't
outlive its corresponding document.

Bug:  867324 
Change-Id: Id0479f59c9a9652b2cb321ed14369e8996748d8a
Reviewed-on: https://chromium-review.googlesource.com/1162743
Commit-Queue: Dominic Mazzoni <dmazzoni@chromium.org>
Reviewed-by: Alice Boxhall <aboxhall@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#590104}
[modify] https://crrev.com/34b81f99f94e8f54f8f676a06d4a8bdcd02c13e5/third_party/blink/renderer/core/accessibility/ax_context.cc
[modify] https://crrev.com/34b81f99f94e8f54f8f676a06d4a8bdcd02c13e5/third_party/blink/renderer/core/accessibility/ax_context.h
[modify] https://crrev.com/34b81f99f94e8f54f8f676a06d4a8bdcd02c13e5/third_party/blink/renderer/core/accessibility/ax_object_cache.h
[modify] https://crrev.com/34b81f99f94e8f54f8f676a06d4a8bdcd02c13e5/third_party/blink/renderer/core/dom/document.cc
[modify] https://crrev.com/34b81f99f94e8f54f8f676a06d4a8bdcd02c13e5/third_party/blink/renderer/core/dom/document.h
[modify] https://crrev.com/34b81f99f94e8f54f8f676a06d4a8bdcd02c13e5/third_party/blink/renderer/core/layout/layout_object.cc
[modify] https://crrev.com/34b81f99f94e8f54f8f676a06d4a8bdcd02c13e5/third_party/blink/renderer/core/layout/layout_object.h
[modify] https://crrev.com/34b81f99f94e8f54f8f676a06d4a8bdcd02c13e5/third_party/blink/renderer/modules/accessibility/ax_layout_object.cc
[modify] https://crrev.com/34b81f99f94e8f54f8f676a06d4a8bdcd02c13e5/third_party/blink/renderer/modules/accessibility/ax_node_object.cc
[modify] https://crrev.com/34b81f99f94e8f54f8f676a06d4a8bdcd02c13e5/third_party/blink/renderer/modules/accessibility/ax_object.cc
[modify] https://crrev.com/34b81f99f94e8f54f8f676a06d4a8bdcd02c13e5/third_party/blink/renderer/modules/accessibility/ax_object.h
[modify] https://crrev.com/34b81f99f94e8f54f8f676a06d4a8bdcd02c13e5/third_party/blink/renderer/modules/accessibility/ax_object_cache_impl.cc
[modify] https://crrev.com/34b81f99f94e8f54f8f676a06d4a8bdcd02c13e5/third_party/blink/renderer/modules/accessibility/ax_object_cache_impl.h

Comment 4 by dmazz...@chromium.org, Sep 10

Status: Fixed (was: Started)
Project Member

Comment 5 by bugdroid1@chromium.org, Sep 11 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/dd0429634ffda7f94b4ad15635df012331931324

commit dd0429634ffda7f94b4ad15635df012331931324
Author: Yutaka Hirano <yhirano@chromium.org>
Date: Tue Sep 11 01:08:52 2018

Revert "Make AXObjectCache garbage-collected"

This reverts commit 34b81f99f94e8f54f8f676a06d4a8bdcd02c13e5.

Reason for revert: Due to failures on the Leak bot.
https://findit-for-me.appspot.com/waterfall/failure?url=https://build.chromium.org/p/chromium.webkit/builders/WebKit%20Linux%20Trusty%20Leak/builds/24067

Original change's description:
> Make AXObjectCache garbage-collected
> 
> This is a follow-up to http://crrev.com/c/1150824 - instead of having
> Document keep track of its AXContexts, let the garbage collector do the
> work for us. Each AXContext owns a Persistent<AXObjectCache>, and
> the associated Document has a WeakPersistent<AXObjectCache>. That way
> when the last AXContext goes out of scope, the AXObjectCache will be
> cleaned up automatically.
> 
> This requires a few changes due to assumptions that AXObjectCache couldn't
> outlive its corresponding document.
> 
> Bug:  867324 
> Change-Id: Id0479f59c9a9652b2cb321ed14369e8996748d8a
> Reviewed-on: https://chromium-review.googlesource.com/1162743
> Commit-Queue: Dominic Mazzoni <dmazzoni@chromium.org>
> Reviewed-by: Alice Boxhall <aboxhall@chromium.org>
> Reviewed-by: Kentaro Hara <haraken@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#590104}

TBR=dmazzoni@chromium.org,aboxhall@chromium.org,haraken@chromium.org

Change-Id: Ic63c70078865bf5fdc8a7743bd07484eb95068e2
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  867324 
Reviewed-on: https://chromium-review.googlesource.com/1217646
Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Commit-Queue: Yutaka Hirano <yhirano@chromium.org>
Cr-Commit-Position: refs/heads/master@{#590156}
[modify] https://crrev.com/dd0429634ffda7f94b4ad15635df012331931324/third_party/blink/renderer/core/accessibility/ax_context.cc
[modify] https://crrev.com/dd0429634ffda7f94b4ad15635df012331931324/third_party/blink/renderer/core/accessibility/ax_context.h
[modify] https://crrev.com/dd0429634ffda7f94b4ad15635df012331931324/third_party/blink/renderer/core/accessibility/ax_object_cache.h
[modify] https://crrev.com/dd0429634ffda7f94b4ad15635df012331931324/third_party/blink/renderer/core/dom/document.cc
[modify] https://crrev.com/dd0429634ffda7f94b4ad15635df012331931324/third_party/blink/renderer/core/dom/document.h
[modify] https://crrev.com/dd0429634ffda7f94b4ad15635df012331931324/third_party/blink/renderer/core/layout/layout_object.cc
[modify] https://crrev.com/dd0429634ffda7f94b4ad15635df012331931324/third_party/blink/renderer/core/layout/layout_object.h
[modify] https://crrev.com/dd0429634ffda7f94b4ad15635df012331931324/third_party/blink/renderer/modules/accessibility/ax_layout_object.cc
[modify] https://crrev.com/dd0429634ffda7f94b4ad15635df012331931324/third_party/blink/renderer/modules/accessibility/ax_node_object.cc
[modify] https://crrev.com/dd0429634ffda7f94b4ad15635df012331931324/third_party/blink/renderer/modules/accessibility/ax_object.cc
[modify] https://crrev.com/dd0429634ffda7f94b4ad15635df012331931324/third_party/blink/renderer/modules/accessibility/ax_object.h
[modify] https://crrev.com/dd0429634ffda7f94b4ad15635df012331931324/third_party/blink/renderer/modules/accessibility/ax_object_cache_impl.cc
[modify] https://crrev.com/dd0429634ffda7f94b4ad15635df012331931324/third_party/blink/renderer/modules/accessibility/ax_object_cache_impl.h
►

Sign in to add a comment