Switch all LayoutTests to use new accessibility relative bounding box API.

Switch all LayoutTests to use new accessibility relative bounding box API.

This change switches test_runner to use only the new relative bounding box
API to get bounding boxes of accessibility elements.

I made a few fixes in order to get existing tests to pass:

* Support explicit element rects inside a canvas
* Implement bounding boxes for inline text boxes
* Bounding box for LayoutText needs to clip to ellipses
* Bounding box for LayoutInline needs to include continuations

The next step will be to switch Chrome to use the new API, then we can
delete the old bounding box code from blink.

BUG=618120
Committed: https://crrev.com/ff627eb09af66b38346b18b03591e07ae9e0f41a
Cr-Commit-Position: refs/heads/master@{#409079}

[dmazzoni, 2016-07-22 21:51:01]

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

[commit-bot: I haz the power, 2016-07-22 21:51:21]

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-07-22 22:40:08]

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

[commit-bot: I haz the power, 2016-07-22 22:40:09]

Dry run: Try jobs failed on following builders:
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)

[dmazzoni, 2016-07-27 03:47:33]

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

[commit-bot: I haz the power, 2016-07-27 03:47:45]

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

[dmazzoni, 2016-07-27 03:54:35]

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

[commit-bot: I haz the power, 2016-07-27 03:54:49]

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-07-27 04:43:13]

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

[commit-bot: I haz the power, 2016-07-27 04:43:14]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[dmazzoni, 2016-07-27 07:36:08]

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

[commit-bot: I haz the power, 2016-07-27 07:36:24]

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

[dmazzoni, 2016-07-27 07:36:32]

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

[dmazzoni, 2016-07-27 07:36:33]

Ready for a look

[commit-bot: I haz the power, 2016-07-27 08:26:14]

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

[commit-bot: I haz the power, 2016-07-27 08:26:15]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[aboxhall, 2016-07-28 18:40:43]

https://codereview.chromium.org/2169273004/diff/40001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/accessibility/draw-focus-if-needed.html
(right):

https://codereview.chromium.org/2169273004/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/accessibility/draw-focus-if-needed.html:13: <div
id="console"></div>
Is this intentional? Does it change the expected result?

https://codereview.chromium.org/2169273004/diff/40001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/accessibility/svg-bounds-expected.txt
(right):

https://codereview.chromium.org/2169273004/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/accessibility/svg-bounds-expected.txt:29: MouthY:
275
Why has this increased?

https://codereview.chromium.org/2169273004/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/accessibility/AXInlineTextBox.cpp
(right):

https://codereview.chromium.org/2169273004/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/accessibility/AXInlineTextBox.cpp:77: if
(!m_inlineTextBox)
Maybe a comment explaining the guarantees made by this method?

https://codereview.chromium.org/2169273004/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/accessibility/AXLayoutObject.cpp (right):

https://codereview.chromium.org/2169273004/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/accessibility/AXLayoutObject.cpp:260: if
(!m_explicitElementRect.isEmpty()) {
Can you add a comment explaining the logic here?

https://codereview.chromium.org/2169273004/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/accessibility/AXNodeObject.cpp (right):

https://codereview.chromium.org/2169273004/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/accessibility/AXNodeObject.cpp:1860:
outBoundsInContainer.setSize(FloatSize(outBoundsInContainer.width(),
std::min(10.0f, outBoundsInContainer.height())));
Is there a test for this behaviour?

https://codereview.chromium.org/2169273004/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/accessibility/AXNodeObject.h (right):

https://codereview.chromium.org/2169273004/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/accessibility/AXNodeObject.h:171: void
getRelativeBounds(AXObject** container, FloatRect& boundsInContainer,
SkMatrix44& containerTransform) const override;
outContainer etc?

[dmazzoni, 2016-07-28 21:43:15]

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

[dmazzoni, 2016-07-28 21:43:16]

+mkwst - would you be willing to do an owners review? Let me know if there are
specific files you'd like reviewed by someone with more specific expertise,
thanks!

https://codereview.chromium.org/2169273004/diff/40001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/accessibility/draw-focus-if-needed.html
(right):

https://codereview.chromium.org/2169273004/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/accessibility/draw-focus-if-needed.html:13: <div
id="console"></div>
On 2016/07/28 at 18:40:42, aboxhall wrote:
> Is this intentional? Does it change the expected result?

Yeah, the issue here was that log messages during running tests
get appended to an object with an id of console, and by default if you don't
create it
the console gets placed at the *top* of the document.

This change broke some of the tests because it slightly changed the timing of
when we measured the canvas's bounds. After the first assertion printed to the
console, we now measure the location of the canvas, but now the canvas has been
shifted down the page!

The fix is just to put the logs below the canvas so the canvas doesn't move.

https://codereview.chromium.org/2169273004/diff/40001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/accessibility/svg-bounds-expected.txt
(right):

https://codereview.chromium.org/2169273004/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/accessibility/svg-bounds-expected.txt:29: MouthY:
275
On 2016/07/28 at 18:40:42, aboxhall wrote:
> Why has this increased?

Completely arbitrary rounding error. I checked it as a float and it's
approximately 274.5, so the new bounds calculation code bumped us by an epsilon
amount and it rounded up instead of down.

https://codereview.chromium.org/2169273004/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/accessibility/AXInlineTextBox.cpp
(right):

https://codereview.chromium.org/2169273004/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/accessibility/AXInlineTextBox.cpp:77: if
(!m_inlineTextBox)
On 2016/07/28 at 18:40:43, aboxhall wrote:
> Maybe a comment explaining the guarantees made by this method?

I included a lot of documentation in AXObject.h, is there anything in particular
you wanted documentation for in this method? The interface is tricky at first, I
agree, but once you understand it, this implementation is pretty trivial, right?

https://codereview.chromium.org/2169273004/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/accessibility/AXLayoutObject.cpp (right):

https://codereview.chromium.org/2169273004/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/accessibility/AXLayoutObject.cpp:260: if
(!m_explicitElementRect.isEmpty()) {
On 2016/07/28 at 18:40:43, aboxhall wrote:
> Can you add a comment explaining the logic here?

Done.

https://codereview.chromium.org/2169273004/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/accessibility/AXNodeObject.cpp (right):

https://codereview.chromium.org/2169273004/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/accessibility/AXNodeObject.cpp:1860:
outBoundsInContainer.setSize(FloatSize(outBoundsInContainer.width(),
std::min(10.0f, outBoundsInContainer.height())));
On 2016/07/28 at 18:40:43, aboxhall wrote:
> Is there a test for this behaviour?

Yes, this was covered by existing canvas accessibility tests.

https://codereview.chromium.org/2169273004/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/accessibility/AXNodeObject.h (right):

https://codereview.chromium.org/2169273004/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/accessibility/AXNodeObject.h:171: void
getRelativeBounds(AXObject** container, FloatRect& boundsInContainer,
SkMatrix44& containerTransform) const override;
On 2016/07/28 at 18:40:43, aboxhall wrote:
> outContainer etc?

Done throughout.

[dmazzoni, 2016-07-28 21:43:38]

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

[commit-bot: I haz the power, 2016-07-28 21:44:13]

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

[aboxhall, 2016-07-28 22:13:10]

lgtm

https://codereview.chromium.org/2169273004/diff/40001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/accessibility/svg-bounds-expected.txt
(right):

https://codereview.chromium.org/2169273004/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/accessibility/svg-bounds-expected.txt:29: MouthY:
275
On 2016/07/28 21:43:16, dmazzoni wrote:
> On 2016/07/28 at 18:40:42, aboxhall wrote:
> > Why has this increased?
> 
> Completely arbitrary rounding error. I checked it as a float and it's
> approximately 274.5, so the new bounds calculation code bumped us by an
epsilon
> amount and it rounded up instead of down.

It's just weird cause you call Math.floor() in the test, but ok :)

https://codereview.chromium.org/2169273004/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/accessibility/AXInlineTextBox.cpp
(right):

https://codereview.chromium.org/2169273004/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/accessibility/AXInlineTextBox.cpp:77: if
(!m_inlineTextBox)
On 2016/07/28 21:43:16, dmazzoni wrote:
> On 2016/07/28 at 18:40:43, aboxhall wrote:
> > Maybe a comment explaining the guarantees made by this method?
> 
> I included a lot of documentation in AXObject.h, is there anything in
particular
> you wanted documentation for in this method? The interface is tricky at first,
I
> agree, but once you understand it, this implementation is pretty trivial,
right?

It was more that I did a bit of a double take at returning here until I realised
you want to guarantee that all of the out params are always something sensible.
But maybe that doesn't need to be stated.

[commit-bot: I haz the power, 2016-07-28 22:41:44]

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

[commit-bot: I haz the power, 2016-07-28 22:41:45]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Mike West, 2016-08-01 11:46:17]

The approach looks good, but the test failures look relevant.

Happy to stamp this if Alice can give you a same-time-zone sanity check with the
bug fix: LGTM.

[dmazzoni, 2016-08-01 20:20:01]

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

[commit-bot: I haz the power, 2016-08-01 20:20:29]

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

[aboxhall, 2016-08-01 21:37:14]

lgtm

[commit-bot: I haz the power, 2016-08-01 21:55:36]

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

[commit-bot: I haz the power, 2016-08-01 21:55:37]

Dry run: This issue passed the CQ dry run.

[dmazzoni, 2016-08-01 22:00:55]

The CQ bit was checked by dmazzoni@chromium.org

[dmazzoni, 2016-08-01 22:00:56]

The patchset sent to the CQ was uploaded after l-g-t-m from mkwst@chromium.org
Link to the patchset: https://codereview.chromium.org/2169273004/#ps100001
(title: "Fix absolute bounds in AXInlineTextBox::elementRect")

[commit-bot: I haz the power, 2016-08-01 22:01:13]

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-08-01 22:38:37]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2016-08-01 22:41:14]

Description was changed from

==========
Switch all LayoutTests to use new accessibility relative bounding box API.

This change switches test_runner to use only the new relative bounding box
API to get bounding boxes of accessibility elements.

I made a few fixes in order to get existing tests to pass:

* Support explicit element rects inside a canvas
* Implement bounding boxes for inline text boxes
* Bounding box for LayoutText needs to clip to ellipses
* Bounding box for LayoutInline needs to include continuations

The next step will be to switch Chrome to use the new API, then we can
delete the old bounding box code from blink.

BUG=618120
==========

to

==========
Switch all LayoutTests to use new accessibility relative bounding box API.

This change switches test_runner to use only the new relative bounding box
API to get bounding boxes of accessibility elements.

I made a few fixes in order to get existing tests to pass:

* Support explicit element rects inside a canvas
* Implement bounding boxes for inline text boxes
* Bounding box for LayoutText needs to clip to ellipses
* Bounding box for LayoutInline needs to include continuations

The next step will be to switch Chrome to use the new API, then we can
delete the old bounding box code from blink.

BUG=618120

Committed: https://crrev.com/ff627eb09af66b38346b18b03591e07ae9e0f41a
Cr-Commit-Position: refs/heads/master@{#409079}
==========

[commit-bot: I haz the power, 2016-08-01 22:41:16]

Patchset 6 (id:??) landed as
https://crrev.com/ff627eb09af66b38346b18b03591e07ae9e0f41a
Cr-Commit-Position: refs/heads/master@{#409079}