Issue 224803005: Refactor BrowserAccessibility to prepare for AXNode (re-land)

Issue 224803005: Refactor BrowserAccessibility to prepare for AXNode (re-land) (Closed)

Created:
6 years, 8 months ago by dmazzoni

Modified:
6 years, 8 months ago

Reviewers:
David Tseng, aboxhall

CC:
chromium-reviews, plundblad+watch_chromium.org, aboxhall+watch_chromium.org, jam, yuzo+watch_chromium.org, darin-cc_chromium.org, dmazzoni+watch_chromium.org, dtseng+watch_chromium.org

Base URL:
svn://svn.chromium.org/chrome/trunk/src

Visibility:
Public.

More Reviews

Description

Refactor BrowserAccessibility to prepare for AXNode (re-land) The only purpose of this change is to rename some member variables and accessors in BrowserAccessibility so that in the subsequent change we can repurpose BrowserAccessibility to make use of AXNode instead. There should be no code logic changes here - just look at browser_accessibility.h, everything else just follows from that. BUG=316726 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=262673 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=262956

Patch Set 1 #

Patch Set 2 : Fix errors #

Patch Set 3 : Fix win compile #

Patch Set 4 : Win fix #

Total comments: 6

Patch Set 5 : Added clarifying comment #

Patch Set 6 : Rebase #

Patch Set 7 : Fix merge error #

Patch Set 8 : Fix gtk #

Created: 6 years, 8 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+395 lines, -415 lines)			Patch
M	content/browser/accessibility/accessibility_tree_formatter.cc	View		2 chunks	+2 lines, -2 lines	0 comments	Download
M	content/browser/accessibility/accessibility_tree_formatter_gtk.cc	View	1 2 3 4 5 6 7	2 chunks	+5 lines, -3 lines	0 comments	Download
M	content/browser/accessibility/browser_accessibility.h	View	1 2 3 4 5 6	5 chunks	+31 lines, -42 lines	0 comments	Download
M	content/browser/accessibility/browser_accessibility.cc	View	1 2 3 4 5	29 chunks	+111 lines, -121 lines	0 comments	Download
M	content/browser/accessibility/browser_accessibility_android.cc	View	1 2 3 4 5	18 chunks	+56 lines, -55 lines	0 comments	Download
M	content/browser/accessibility/browser_accessibility_cocoa.mm	View	1 2 3 4 5	12 chunks	+15 lines, -15 lines	0 comments	Download
M	content/browser/accessibility/browser_accessibility_gtk.cc	View	1 2 3 4 5 6 7	7 chunks	+9 lines, -9 lines	0 comments	Download
M	content/browser/accessibility/browser_accessibility_manager.cc	View	1 2 3 4 5	10 chunks	+20 lines, -19 lines	0 comments	Download
M	content/browser/accessibility/browser_accessibility_manager_android.cc	View	1 2 3 4 5	9 chunks	+38 lines, -38 lines	0 comments	Download
M	content/browser/accessibility/browser_accessibility_manager_gtk.cc	View	1 2 3 4 5 6 7	1 chunk	+2 lines, -2 lines	0 comments	Download
M	content/browser/accessibility/browser_accessibility_manager_mac.mm	View		1 chunk	+1 line, -1 line	0 comments	Download
M	content/browser/accessibility/browser_accessibility_manager_unittest.cc	View		4 chunks	+10 lines, -10 lines	0 comments	Download
M	content/browser/accessibility/browser_accessibility_manager_win.cc	View	1 2 3 4 5	2 chunks	+2 lines, -2 lines	0 comments	Download
M	content/browser/accessibility/browser_accessibility_win.h	View		1 chunk	+0 lines, -4 lines	0 comments	Download
M	content/browser/accessibility/browser_accessibility_win.cc	View	1 2 3 4 5	39 chunks	+86 lines, -85 lines	0 comments	Download
M	content/browser/accessibility/browser_accessibility_win_unittest.cc	View	1 2 3 4 5	3 chunks	+7 lines, -7 lines	0 comments	Download

Messages

Total messages: 20 (0 generated)

Expand Messages | Collapse Messages

dmazzoni

Whoever has a chance, Alice or David - please mostly focus on browser_accessibility.h mostly, everything ...

6 years, 8 months ago (2014-04-04 21:43:47 UTC) #1

aboxhall

https://codereview.chromium.org/224803005/diff/60001/content/browser/accessibility/browser_accessibility.h File content/browser/accessibility/browser_accessibility.h (left): https://codereview.chromium.org/224803005/diff/60001/content/browser/accessibility/browser_accessibility.h#oldcode69 content/browser/accessibility/browser_accessibility.h:69: int32 renderer_id, I assume this is mostly for clarification? ...

6 years, 8 months ago (2014-04-04 22:18:09 UTC) #2

dmazzoni

6 years, 8 months ago (2014-04-04 23:27:39 UTC) #3

https://codereview.chromium.org/224803005/diff/60001/content/browser/accessib...
File content/browser/accessibility/browser_accessibility.h (left):

https://codereview.chromium.org/224803005/diff/60001/content/browser/accessib...
content/browser/accessibility/browser_accessibility.h:69: int32 renderer_id,
On 2014/04/04 22:18:10, aboxhall wrote:
> I assume this is mostly for clarification? It was never that great a name.

Yeah, this is to match AXNode. The "other" ID has now been renamed
unique_id_win(), it's only used on Windows.

https://codereview.chromium.org/224803005/diff/60001/content/browser/accessib...
File content/browser/accessibility/browser_accessibility.h (right):

https://codereview.chromium.org/224803005/diff/60001/content/browser/accessib...
content/browser/accessibility/browser_accessibility.h:162: BrowserAccessibility*
GetParent() const { return deprecated_parent_; }
On 2014/04/04 22:18:10, aboxhall wrote:
> How will the traversing logic work after the switch?

Afterwards, each BrowserAccessibility will have a pointer to its corresponding
AXNode, and it will use AXNode to get its data and its parent/child pointers.

BrowserAccessibility will contain little more than an AXNode and a pointer to
the BrowserAccessibilityManager. The platform subclasses of BrowserAccessibility
may contain additional platform-specific data and flags.

> i.e. why are these methods
> this still implemented in terms of deprecated_children_ when everything else
is
> implemented in terms of data_?

AXNodeData contains everything other than the tree structure pointers. AXNode
has the pointers to the parent and children. So after the switch, we'll access
AXNode's data() and AXNode's parent() and AXNode's children().

https://codereview.chromium.org/224803005/diff/60001/content/browser/accessib...
content/browser/accessibility/browser_accessibility.h:164: uint32
InternalChildCount() const { return deprecated_children_.size(); }
On 2014/04/04 22:18:10, aboxhall wrote:
> Why are this and the below method named 'Internal*'?

To contrast with PlatformChildCount and PlatformGetChild, which return the right
platform-specific way to walk the tree - for example some nodes are leaf nodes
on some platforms but not others.

I added a comment.