Enable canvas.drawFocusIfNeeded() by default.

Enable canvas.drawFocusIfNeeded() by default.

Enable following methods:
  - drawFocusIfNeeded(Element);
  - drawFocusIfNeeded(Path2D, Element);

Intent to ship:
  https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/KE_X_C6vOQQ

Additionally, we should remove the behavior that updates accessibility.
It has been transferred to 'Hit Regions' feature.

BUG=331923
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=174904

[zino, 2014-05-21 13:28:35]

Please take a look.

Thank you.

[Justin Novosad, 2014-05-21 14:27:46]

https://codereview.chromium.org/299823003/diff/1/Source/core/html/canvas/Canv...
File Source/core/html/canvas/CanvasRenderingContext2D.cpp (left):

https://codereview.chromium.org/299823003/diff/1/Source/core/html/canvas/Canv...
Source/core/html/canvas/CanvasRenderingContext2D.cpp:2317: void
CanvasRenderingContext2D::updateFocusRingAccessibility(const Path& path,
Element* element)
The new behavior that is implemented here need to be tested

[dmazzoni, 2014-05-21 17:18:50]

I think you may also need to delete the focus ring tests from
LayoutTests/accessibility. Just delete them, don't disable them. Hopefully we
can borrow some of that code when we do hit regions.

https://codereview.chromium.org/299823003/diff/1/Source/core/html/canvas/Canv...
File Source/core/html/canvas/CanvasRenderingContext2D.cpp (right):

https://codereview.chromium.org/299823003/diff/1/Source/core/html/canvas/Canv...
Source/core/html/canvas/CanvasRenderingContext2D.cpp:2291: bool
CanvasRenderingContext2D::drawCustomFocusRing(Element* element)
Delete this

https://codereview.chromium.org/299823003/diff/1/Source/core/html/canvas/Canv...
File Source/core/html/canvas/CanvasRenderingContext2D.idl (right):

https://codereview.chromium.org/299823003/diff/1/Source/core/html/canvas/Canv...
Source/core/html/canvas/CanvasRenderingContext2D.idl:101:
[RuntimeEnabled=ExperimentalCanvasFeatures] boolean drawCustomFocusRing(Element
element);
You should delete drawCustomFocusRing entirely, too.

[Rik, 2014-05-21 20:48:49]

This looks good to me. I'm unsure if the removal of drawCustomFocusRing should
be in this patch or in a separate one.

Why do we still have a runtime flag for Path2D?

[zino, 2014-05-22 09:55:46]

On 2014/05/21 20:48:49, Rik wrote:
> This looks good to me. I'm unsure if the removal of drawCustomFocusRing should
> be in this patch or in a separate one.
> 
> Why do we still have a runtime flag for Path2D?

Thank you for review.

It has a runtime flag but it is always enabled by default.
We just use to make it trivial to enable/disable Path2D features if necessary.
(e.g. name conflicts)

Please refer to this CL and @eseidel's comments.
https://codereview.chromium.org/227583006/

[zino, 2014-05-22 10:06:56]

On 2014/05/21 17:18:50, dmazzoni wrote:
> I think you may also need to delete the focus ring tests from
> LayoutTests/accessibility. Just delete them, don't disable them. Hopefully we
> can borrow some of that code when we do hit regions.
> 
>
https://codereview.chromium.org/299823003/diff/1/Source/core/html/canvas/Canv...
> File Source/core/html/canvas/CanvasRenderingContext2D.cpp (right):
> 
>
https://codereview.chromium.org/299823003/diff/1/Source/core/html/canvas/Canv...
> Source/core/html/canvas/CanvasRenderingContext2D.cpp:2291: bool
> CanvasRenderingContext2D::drawCustomFocusRing(Element* element)
> Delete this
> 
>
https://codereview.chromium.org/299823003/diff/1/Source/core/html/canvas/Canv...
> File Source/core/html/canvas/CanvasRenderingContext2D.idl (right):
> 
>
https://codereview.chromium.org/299823003/diff/1/Source/core/html/canvas/Canv...
> Source/core/html/canvas/CanvasRenderingContext2D.idl:101:
> [RuntimeEnabled=ExperimentalCanvasFeatures] boolean
drawCustomFocusRing(Element
> element);
> You should delete drawCustomFocusRing entirely, too.

Thank you for review.

I just removed accessibility tests and I create a new CL to remove
drawCustomFocusRing
Please refer to https://codereview.chromium.org/289283019.
But it isn't ready yet. I will remove it soon after landing this.

Thank you :)

[zino, 2014-05-22 10:14:44]

On 2014/05/21 14:27:46, junov wrote:
>
https://codereview.chromium.org/299823003/diff/1/Source/core/html/canvas/Canv...
> File Source/core/html/canvas/CanvasRenderingContext2D.cpp (left):
> 
>
https://codereview.chromium.org/299823003/diff/1/Source/core/html/canvas/Canv...
> Source/core/html/canvas/CanvasRenderingContext2D.cpp:2317: void
> CanvasRenderingContext2D::updateFocusRingAccessibility(const Path& path,
> Element* element)
> The new behavior that is implemented here need to be tested

Thank you for review.

Sorry, I wasn't understanding your comment.
It seems that the behavior is already covered by
LT/f/c/draw-focus-if-needed.html  and draw-focus-if-needed-with-path2d.html.

Could you please explain in more detail?

[zino, 2014-05-24 03:39:45]

Ping Owners

Could you please review?

[dmazzoni, 2014-05-24 04:47:16]

lgtm

[zino, 2014-05-24 14:34:52]

On 2014/05/24 04:47:16, dmazzoni wrote:
> lgtm

Thank you for lgtm :)

And I'm waiting for other reviewer's comments.
Especially, this may need API owner's approval.

[eseidel, 2014-05-27 18:29:11]

lgtm

[commit-bot: I haz the power, 2014-05-27 18:29:36]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/jinho.bang@samsung.com/299823003/20001

[commit-bot: I haz the power, 2014-05-27 20:29:06]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  linux_blink_dbg on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/linux_blink_dbg/builds/9855)

[commit-bot: I haz the power, 2014-05-27 22:21:20]

Change committed as 174904