Show ancestor hierarchy in accessibility panel

Show ancestor hierarchy in accessibility panel

BUG=560525
Committed: https://crrev.com/3558706c1b2a630557929b61fefc74e28975f3a6
Committed: https://crrev.com/97fbce44062367ee4a63cb751c3f56cb226c080d
Cr-Original-Commit-Position: refs/heads/master@{#419369}
Cr-Commit-Position: refs/heads/master@{#419457}

[aboxhall, 2016-09-14 04:02:09]

aboxhall@chromium.org changed reviewers:
+ dgozman@chromium.org, pfeldman@chromium.org

[aboxhall, 2016-09-14 04:02:10]

Screenshots in bug.

Note that this is not the intended final state; eventually the view will
include:
- Selecting nodes to inspect from the accessibility tree (this would trigger
inspecting the associated DOM node and thus re-building the accessibility
sidebar pane)
- Showing child nodes and siblings of selected node
- Toggling whether to include ignored DOM nodes in the partial tree.

[aboxhall, 2016-09-14 04:06:26]

aboxhall@chromium.org changed reviewers:
+ dmazzoni@chromium.org

[aboxhall, 2016-09-14 04:06:26]

+dmazzoni for modules/accessibility changes

[aboxhall, 2016-09-14 04:08:37]

On 2016/09/14 04:06:26, aboxhall wrote:
> +dmazzoni for modules/accessibility changes

I just toggled browser accessibility mode on via chrome://accessibility and
requested the full tree for the page I was inspecting as well as viewing the
Accessibility sidebar pane, and there don't seem to be any interactions. Any
other ideas as to what I could do to check whether this design may cause issues?

[aboxhall, 2016-09-14 21:10:33]

Ping?

[aboxhall, 2016-09-15 17:20:05]

Ping?

[dgozman, 2016-09-15 17:45:45]

https://codereview.chromium.org/2322413003/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/inspector/browser_protocol.json (right):

https://codereview.chromium.org/2322413003/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/inspector/browser_protocol.json:4225: "name":
"getAXNodeForDOMNode",
getAXNodeChain

https://codereview.chromium.org/2322413003/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/devtools/front_end/accessibility/AXTreePane.js
(right):

https://codereview.chromium.org/2322413003/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/accessibility/AXTreePane.js:39:
previous.selectable = true;
Only the last is selectable?

https://codereview.chromium.org/2322413003/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/accessibility/AXTreePane.js:89: if
("name" in this._axNode && this._axNode.name.value !== "") {
just ... && this._axNode.name.value) {

https://codereview.chromium.org/2322413003/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/accessibility/AXTreePane.js:99:
appendNameElement: function(name)
_appendNameElement

https://codereview.chromium.org/2322413003/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/accessibility/AXTreePane.js:110:
appendRoleElement: function(role)
_appendRoleElement

https://codereview.chromium.org/2322413003/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/accessibility/AXTreePane.js:122:
appendIgnoredNodeElement: function()
_appendIgnoredNodeElement

https://codereview.chromium.org/2322413003/diff/100001/third_party/WebKit/Sou...
File
third_party/WebKit/Source/devtools/front_end/accessibility/AccessibilitySidebarView.js
(right):

https://codereview.chromium.org/2322413003/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/accessibility/AccessibilitySidebarView.js:45:
);
strange formatting

https://codereview.chromium.org/2322413003/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/accessibility/AccessibilitySidebarView.js:47:
this._ariaSubPane.parentWidget().contentElement.classList.remove("sidebar-hidden-override");
Let's instead call showView()/removeView() on the _sidebarPaneStack.

https://codereview.chromium.org/2322413003/diff/100001/third_party/WebKit/Sou...
File
third_party/WebKit/Source/modules/accessibility/InspectorAccessibilityAgent.cpp
(right):

https://codereview.chromium.org/2322413003/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/accessibility/InspectorAccessibilityAgent.cpp:377:
discardFrontendBindings();
If the cache is not used in subsequent protocol calls, let's clean it up at the
end of this method? Then it makes sense to turn it into a local variable instead
of a member. Pretty much like it was before.

https://codereview.chromium.org/2322413003/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/accessibility/InspectorAccessibilityAgent.cpp:392:
*errorString = "Can't inspect out of process frames yet";
This actually means frame is detached. Out of process will not even have a
document.

https://codereview.chromium.org/2322413003/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/accessibility/InspectorAccessibilityAgent.cpp:405:
AXObjectCacheImpl* cacheImpl = toAXObjectCacheImpl(cache);
If we actually need an AXObjectCacheImpl, let's replace AXObjectCache with impl
everywhere. There is no need to trick ourselves pretending it's not an impl.

https://codereview.chromium.org/2322413003/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/accessibility/InspectorAccessibilityAgent.cpp:407:
*nodes = protocol::Array<protocol::Accessibility::AXNode>::create();
Let's not create nodes if there was no axObject.

[dmazzoni, 2016-09-15 17:47:11]

I'm still trying to grapple with the implications of having
two AXObjectCaches pointing to the same document.

I can't think of a scenario where the ASSERT in
~LayoutObject will trip, but that one still makes me worry.

What I'm curious about now is whether there's a possible UAF
if there's a mutation to the DOM tree or the layout tree
during the lifetime of the inspector-owned AXObjectCache.

Here's a sketch of how to make it crash (or prove that it
won't):

1. Do an ASAN build, that will maximize the chances we'll
catch potential UAFs rather than hoping to get lucky with a
hard crash
2. Create a webpage that constantly changes the DOM with
setInterval
3. Inspect a descendant or ancestor of the node that's
changing, and keep reloading it. I expect a UAF / crash
because it will walk to its ancestor or descendant to
compute accessibility properties and that object will
have a pointer to a Node or LayoutObject that's been freed.

Oh, maybe inspect a node that has aria-labelledby and then
have a descendant of that label target be the one that changes.
You know that the computation of the first node's name will
recurse into the label, but if it doesn't know the label was
mutated it will have a stale reference to a node.

This wouldn't break with a normal AXObjectCache owned by the
document because deleting a Node or LayoutObject calls into the
AXObjectCache to invalidate it, but with your new AXObjectCache
that won't happen.

If this doesn't break, that's great. Perhaps Inspector already
flushes everything when there's a DOM mutation or layout.

[aboxhall, 2016-09-15 20:30:08]

Just dgozman's comments for now.

https://codereview.chromium.org/2322413003/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/inspector/browser_protocol.json (right):

https://codereview.chromium.org/2322413003/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/inspector/browser_protocol.json:4225: "name":
"getAXNodeForDOMNode",
On 2016/09/15 17:45:44, dgozman wrote:
> getAXNodeChain

Done.

https://codereview.chromium.org/2322413003/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/devtools/front_end/accessibility/AXTreePane.js
(right):

https://codereview.chromium.org/2322413003/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/accessibility/AXTreePane.js:39:
previous.selectable = true;
On 2016/09/15 17:45:44, dgozman wrote:
> Only the last is selectable?

For now. Later on others will be selectable and will sync inspector focus (see
comments sent with initial request for review).

https://codereview.chromium.org/2322413003/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/accessibility/AXTreePane.js:89: if
("name" in this._axNode && this._axNode.name.value !== "") {
On 2016/09/15 17:45:44, dgozman wrote:
> just ... && this._axNode.name.value) {

Done.

https://codereview.chromium.org/2322413003/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/accessibility/AXTreePane.js:99:
appendNameElement: function(name)
On 2016/09/15 17:45:44, dgozman wrote:
> _appendNameElement

Done.

https://codereview.chromium.org/2322413003/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/accessibility/AXTreePane.js:110:
appendRoleElement: function(role)
On 2016/09/15 17:45:44, dgozman wrote:
> _appendRoleElement

Done.

https://codereview.chromium.org/2322413003/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/accessibility/AXTreePane.js:122:
appendIgnoredNodeElement: function()
On 2016/09/15 17:45:44, dgozman wrote:
> _appendIgnoredNodeElement

Done.

https://codereview.chromium.org/2322413003/diff/100001/third_party/WebKit/Sou...
File
third_party/WebKit/Source/devtools/front_end/accessibility/AccessibilitySidebarView.js
(right):

https://codereview.chromium.org/2322413003/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/accessibility/AccessibilitySidebarView.js:45:
);
On 2016/09/15 17:45:44, dgozman wrote:
> strange formatting

Editors are fun

https://codereview.chromium.org/2322413003/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/accessibility/AccessibilitySidebarView.js:47:
this._ariaSubPane.parentWidget().contentElement.classList.remove("sidebar-hidden-override");
On 2016/09/15 17:45:44, dgozman wrote:
> Let's instead call showView()/removeView() on the _sidebarPaneStack.

Won't that change the order?

https://codereview.chromium.org/2322413003/diff/100001/third_party/WebKit/Sou...
File
third_party/WebKit/Source/modules/accessibility/InspectorAccessibilityAgent.cpp
(right):

https://codereview.chromium.org/2322413003/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/accessibility/InspectorAccessibilityAgent.cpp:377:
discardFrontendBindings();
On 2016/09/15 17:45:44, dgozman wrote:
> If the cache is not used in subsequent protocol calls, let's clean it up at
the
> end of this method? Then it makes sense to turn it into a local variable
instead
> of a member. Pretty much like it was before.

It will be used in later changes, once I add getChildNodes to allow expanding
siblings and children of the inspected node.

https://codereview.chromium.org/2322413003/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/accessibility/InspectorAccessibilityAgent.cpp:392:
*errorString = "Can't inspect out of process frames yet";
On 2016/09/15 17:45:44, dgozman wrote:
> This actually means frame is detached. Out of process will not even have a
> document.

Ah ok, changed error string.

https://codereview.chromium.org/2322413003/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/accessibility/InspectorAccessibilityAgent.cpp:405:
AXObjectCacheImpl* cacheImpl = toAXObjectCacheImpl(cache);
On 2016/09/15 17:45:44, dgozman wrote:
> If we actually need an AXObjectCacheImpl, let's replace AXObjectCache with
impl
> everywhere. There is no need to trick ourselves pretending it's not an impl.

Done.

https://codereview.chromium.org/2322413003/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/accessibility/InspectorAccessibilityAgent.cpp:407:
*nodes = protocol::Array<protocol::Accessibility::AXNode>::create();
On 2016/09/15 17:45:44, dgozman wrote:
> Let's not create nodes if there was no axObject.

I don't follow - either way an object gets added to this array, it's never
empty.

[aboxhall, 2016-09-15 22:56:25]

On 2016/09/15 17:47:11, dmazzoni wrote:
> I'm still trying to grapple with the implications of having
> two AXObjectCaches pointing to the same document.
> 
> I can't think of a scenario where the ASSERT in
> ~LayoutObject will trip, but that one still makes me worry.
> 
> What I'm curious about now is whether there's a possible UAF
> if there's a mutation to the DOM tree or the layout tree
> during the lifetime of the inspector-owned AXObjectCache.
> 
> Here's a sketch of how to make it crash (or prove that it
> won't):
> 
> 1. Do an ASAN build, that will maximize the chances we'll
> catch potential UAFs rather than hoping to get lucky with a
> hard crash
> 2. Create a webpage that constantly changes the DOM with
> setInterval
> 3. Inspect a descendant or ancestor of the node that's
> changing, and keep reloading it. I expect a UAF / crash
> because it will walk to its ancestor or descendant to
> compute accessibility properties and that object will
> have a pointer to a Node or LayoutObject that's been freed.

I'm not sure what you mean by "keep reloading it" here.

I created http://output.jsbin.com/velexucepe and did an ASAN build, but the only
way to "reload" is to inspect another node and then move back, and that discards
the AXObjectCache and rebuilds it.

> Oh, maybe inspect a node that has aria-labelledby and then
> have a descendant of that label target be the one that changes.
> You know that the computation of the first node's name will
> recurse into the label, but if it doesn't know the label was
> mutated it will have a stale reference to a node.
> 
> This wouldn't break with a normal AXObjectCache owned by the
> document because deleting a Node or LayoutObject calls into the
> AXObjectCache to invalidate it, but with your new AXObjectCache
> that won't happen.
> 
> If this doesn't break, that's great. Perhaps Inspector already
> flushes everything when there's a DOM mutation or layout.

[dmazzoni, 2016-09-15 23:22:28]

On Thu, Sep 15, 2016 at 3:56 PM <aboxhall@chromium.org> wrote:

> I created http://output.jsbin.com/velexucepe and did an ASAN build, but
> the only
> way to "reload" is to inspect another node and then move back, and that
> discards
> the AXObjectCache and rebuilds it.
>

If moving to another node causes it to discard the AXObjectCache, why do
you need to keep the AXObjectCache around again?

What could you do to trigger accessing the AXObjectCache that you have
stored in that map after you've finished building the tree?

-- 
You received this message because you are subscribed to the Google Groups "Blink
Reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to blink-reviews+unsubscribe@chromium.org.

[dmazzoni, 2016-09-15 23:22:29]

On Thu, Sep 15, 2016 at 3:56 PM <aboxhall@chromium.org> wrote:

> I created http://output.jsbin.com/velexucepe and did an ASAN build, but
> the only
> way to "reload" is to inspect another node and then move back, and that
> discards
> the AXObjectCache and rebuilds it.
>

If moving to another node causes it to discard the AXObjectCache, why do
you need to keep the AXObjectCache around again?

What could you do to trigger accessing the AXObjectCache that you have
stored in that map after you've finished building the tree?

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[chromium-reviews, 2016-09-16 16:41:58]

On Thu, Sep 15, 2016 at 4:22 PM, Dominic Mazzoni <dmazzoni@chromium.org>
wrote:

> On Thu, Sep 15, 2016 at 3:56 PM <aboxhall@chromium.org> wrote:
>
>> I created http://output.jsbin.com/velexucepe and did an ASAN build, but
>> the only
>> way to "reload" is to inspect another node and then move back, and that
>> discards
>> the AXObjectCache and rebuilds it.
>>
>
> If moving to another node causes it to discard the AXObjectCache, why do
> you need to keep the AXObjectCache around again?
>
> What could you do to trigger accessing the AXObjectCache that you have
> stored in that map after you've finished building the tree?
>

Expand child nodes of current or parent node.

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[blink-reviews, 2016-09-16 16:41:58]

On Thu, Sep 15, 2016 at 4:22 PM, Dominic Mazzoni <dmazzoni@chromium.org>
wrote:

> On Thu, Sep 15, 2016 at 3:56 PM <aboxhall@chromium.org> wrote:
>
>> I created http://output.jsbin.com/velexucepe and did an ASAN build, but
>> the only
>> way to "reload" is to inspect another node and then move back, and that
>> discards
>> the AXObjectCache and rebuilds it.
>>
>
> If moving to another node causes it to discard the AXObjectCache, why do
> you need to keep the AXObjectCache around again?
>
> What could you do to trigger accessing the AXObjectCache that you have
> stored in that map after you've finished building the tree?
>

Expand child nodes of current or parent node.

-- 
You received this message because you are subscribed to the Google Groups "Blink
Reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to blink-reviews+unsubscribe@chromium.org.

[aboxhall, 2016-09-16 18:07:35]

After discussing further with Dominic, I decided it would be easier to stick
with ScopedAXObjectCache and work around the challenges of expanding
sibling/child nodes instead.

[aboxhall, 2016-09-16 18:10:35]

The CQ bit was checked by aboxhall@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-16 18:10:55]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-16 20:24:43]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-16 20:24:44]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[dgozman, 2016-09-17 00:36:23]

Looks pretty good! A couple of comments below.

https://codereview.chromium.org/2322413003/diff/100001/third_party/WebKit/Sou...
File
third_party/WebKit/Source/devtools/front_end/accessibility/AccessibilitySidebarView.js
(right):

https://codereview.chromium.org/2322413003/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/accessibility/AccessibilitySidebarView.js:47:
this._ariaSubPane.parentWidget().contentElement.classList.remove("sidebar-hidden-override");
On 2016/09/15 20:30:08, aboxhall wrote:
> On 2016/09/15 17:45:44, dgozman wrote:
> > Let's instead call showView()/removeView() on the _sidebarPaneStack.
> 
> Won't that change the order?

There is a second parameter |insertBefore|.

https://codereview.chromium.org/2322413003/diff/100001/third_party/WebKit/Sou...
File
third_party/WebKit/Source/modules/accessibility/InspectorAccessibilityAgent.cpp
(right):

https://codereview.chromium.org/2322413003/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/accessibility/InspectorAccessibilityAgent.cpp:407:
*nodes = protocol::Array<protocol::Accessibility::AXNode>::create();
On 2016/09/15 20:30:08, aboxhall wrote:
> On 2016/09/15 17:45:44, dgozman wrote:
> > Let's not create nodes if there was no axObject.
> 
> I don't follow - either way an object gets added to this array, it's never
> empty.

Then it should not be optional in protocol.

https://codereview.chromium.org/2322413003/diff/140001/third_party/WebKit/Sou...
File
third_party/WebKit/Source/modules/accessibility/InspectorAccessibilityAgent.cpp
(right):

https://codereview.chromium.org/2322413003/diff/140001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/accessibility/InspectorAccessibilityAgent.cpp:304:
std::unique_ptr<AXNode> buildObjectForIgnoredNode(Node* node, const AXObject*
axObject, AXObjectCacheImpl* cache)
Looks like cache is unused. Remove?

https://codereview.chromium.org/2322413003/diff/140001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/accessibility/InspectorAccessibilityAgent.cpp:327:
std::unique_ptr<AXNode> buildProtocolAXObject(AXObject* axObject,
AXObjectCacheImpl* cache)
ditto

[aboxhall, 2016-09-17 01:12:24]

The CQ bit was checked by aboxhall@chromium.org to run a CQ dry run

[aboxhall, 2016-09-17 01:12:34]

https://codereview.chromium.org/2322413003/diff/100001/third_party/WebKit/Sou...
File
third_party/WebKit/Source/devtools/front_end/accessibility/AccessibilitySidebarView.js
(right):

https://codereview.chromium.org/2322413003/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/accessibility/AccessibilitySidebarView.js:47:
this._ariaSubPane.parentWidget().contentElement.classList.remove("sidebar-hidden-override");
On 2016/09/17 00:36:23, dgozman wrote:
> On 2016/09/15 20:30:08, aboxhall wrote:
> > On 2016/09/15 17:45:44, dgozman wrote:
> > > Let's instead call showView()/removeView() on the _sidebarPaneStack.
> > 
> > Won't that change the order?
> 
> There is a second parameter |insertBefore|.

As discussed, will follow up as this is non-trivial.

https://codereview.chromium.org/2322413003/diff/100001/third_party/WebKit/Sou...
File
third_party/WebKit/Source/modules/accessibility/InspectorAccessibilityAgent.cpp
(right):

https://codereview.chromium.org/2322413003/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/accessibility/InspectorAccessibilityAgent.cpp:407:
*nodes = protocol::Array<protocol::Accessibility::AXNode>::create();
On 2016/09/17 00:36:23, dgozman wrote:
> On 2016/09/15 20:30:08, aboxhall wrote:
> > On 2016/09/15 17:45:44, dgozman wrote:
> > > Let's not create nodes if there was no axObject.
> > 
> > I don't follow - either way an object gets added to this array, it's never
> > empty.
> 
> Then it should not be optional in protocol.

Done.

https://codereview.chromium.org/2322413003/diff/140001/third_party/WebKit/Sou...
File
third_party/WebKit/Source/modules/accessibility/InspectorAccessibilityAgent.cpp
(right):

https://codereview.chromium.org/2322413003/diff/140001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/accessibility/InspectorAccessibilityAgent.cpp:304:
std::unique_ptr<AXNode> buildObjectForIgnoredNode(Node* node, const AXObject*
axObject, AXObjectCacheImpl* cache)
On 2016/09/17 00:36:23, dgozman wrote:
> Looks like cache is unused. Remove?

Done.

https://codereview.chromium.org/2322413003/diff/140001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/accessibility/InspectorAccessibilityAgent.cpp:327:
std::unique_ptr<AXNode> buildProtocolAXObject(AXObject* axObject,
AXObjectCacheImpl* cache)
On 2016/09/17 00:36:23, dgozman wrote:
> ditto

Done.

[commit-bot: I haz the power, 2016-09-17 01:12:46]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[dgozman, 2016-09-17 01:28:03]

I hope all the test expectation changes are just indents, but codereview failed
to show that...

lgtm modulo adding one more call into tests

https://codereview.chromium.org/2322413003/diff/160001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/inspector-protocol/accessibility/accessibility-getAXNode.html
(right):

https://codereview.chromium.org/2322413003/diff/160001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/inspector-protocol/accessibility/accessibility-getAXNode.html:29:
InspectorTest.sendCommand("Accessibility.getAXNodeChain", { "nodeId":
node.nodeId, "fetchAncestors": false }, onGotAccessibilityNode.bind(null,
node));
Let's add a call with fetchAncestors: true to this test as well.

[aboxhall, 2016-09-17 01:46:06]

On 2016/09/17 01:28:03, dgozman wrote:
> I hope all the test expectation changes are just indents, but codereview
failed
> to show that...
> 
> lgtm modulo adding one more call into tests
> 
>
https://codereview.chromium.org/2322413003/diff/160001/third_party/WebKit/Lay...
> File
>
third_party/WebKit/LayoutTests/inspector-protocol/accessibility/accessibility-getAXNode.html
> (right):
> 
>
https://codereview.chromium.org/2322413003/diff/160001/third_party/WebKit/Lay...
>
third_party/WebKit/LayoutTests/inspector-protocol/accessibility/accessibility-getAXNode.html:29:
> InspectorTest.sendCommand("Accessibility.getAXNodeChain", { "nodeId":
> node.nodeId, "fetchAncestors": false }, onGotAccessibilityNode.bind(null,
> node));
> Let's add a call with fetchAncestors: true to this test as well.

Added a new test instead since it would be non-trivial to add in this test
(promise hell) plus it meant I could give it a more interesting ancestor tree.

[aboxhall, 2016-09-17 01:47:30]

The CQ bit was checked by aboxhall@chromium.org

[aboxhall, 2016-09-17 01:47:31]

The patchset sent to the CQ was uploaded after l-g-t-m from dgozman@chromium.org
Link to the patchset: https://codereview.chromium.org/2322413003/#ps180001
(title: "Add new test for ancestors")

[commit-bot: I haz the power, 2016-09-17 01:47:45]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-17 03:33:08]

Committed patchset #10 (id:180001)

[commit-bot: I haz the power, 2016-09-17 03:36:38]

Description was changed from

==========
Show ancestor hierarchy in accessibility panel

BUG=560525
==========

to

==========
Show ancestor hierarchy in accessibility panel

BUG=560525

Committed: https://crrev.com/3558706c1b2a630557929b61fefc74e28975f3a6
Cr-Commit-Position: refs/heads/master@{#419369}
==========

[commit-bot: I haz the power, 2016-09-17 03:36:40]

Patchset 10 (id:??) landed as
https://crrev.com/3558706c1b2a630557929b61fefc74e28975f3a6
Cr-Commit-Position: refs/heads/master@{#419369}

[fs, 2016-09-17 19:20:36]

A revert of this CL (patchset #10 id:180001) has been created in
https://codereview.chromium.org/2351443003/ by fs@opera.com.

The reason for reverting is: Fails the compile step on Linux ChromiumOS GN
(headless_unittests).

[aboxhall, 2016-09-19 13:39:29]

Description was changed from

==========
Show ancestor hierarchy in accessibility panel

BUG=560525

Committed: https://crrev.com/3558706c1b2a630557929b61fefc74e28975f3a6
Cr-Commit-Position: refs/heads/master@{#419369}
==========

to

==========
Show ancestor hierarchy in accessibility panel

BUG=560525

Committed: https://crrev.com/3558706c1b2a630557929b61fefc74e28975f3a6
Cr-Commit-Position: refs/heads/master@{#419369}
==========

[aboxhall, 2016-09-19 13:39:39]

The CQ bit was checked by aboxhall@chromium.org

[commit-bot: I haz the power, 2016-09-19 13:39:55]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-19 14:55:37]

Description was changed from

==========
Show ancestor hierarchy in accessibility panel

BUG=560525

Committed: https://crrev.com/3558706c1b2a630557929b61fefc74e28975f3a6
Cr-Commit-Position: refs/heads/master@{#419369}
==========

to

==========
Show ancestor hierarchy in accessibility panel

BUG=560525

Committed: https://crrev.com/3558706c1b2a630557929b61fefc74e28975f3a6
Cr-Commit-Position: refs/heads/master@{#419369}
==========

[commit-bot: I haz the power, 2016-09-19 14:55:39]

Committed patchset #10 (id:180001)

[commit-bot: I haz the power, 2016-09-19 14:58:31]

Description was changed from

==========
Show ancestor hierarchy in accessibility panel

BUG=560525

Committed: https://crrev.com/3558706c1b2a630557929b61fefc74e28975f3a6
Cr-Commit-Position: refs/heads/master@{#419369}
==========

to

==========
Show ancestor hierarchy in accessibility panel

BUG=560525

Committed: https://crrev.com/3558706c1b2a630557929b61fefc74e28975f3a6
Committed: https://crrev.com/97fbce44062367ee4a63cb751c3f56cb226c080d
Cr-Original-Commit-Position: refs/heads/master@{#419369}
Cr-Commit-Position: refs/heads/master@{#419457}
==========

[commit-bot: I haz the power, 2016-09-19 14:58:33]

Patchset 10 (id:??) landed as
https://crrev.com/97fbce44062367ee4a63cb751c3f56cb226c080d
Cr-Commit-Position: refs/heads/master@{#419457}