Add versions of isPointIn*() that take a Path parameter.

Add versions of isPointInPath, isPointInStroke that take a Path parameter.

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

The spec reference is:
  http://www.w3.org/html/wg/drafts/2dcontext/master/#dom-context-2d-ispointinstroke
  http://www.whatwg.org/specs/web-apps/current-work/multipage/the-canvas-element.html#dom-context-2d-ispointinstroke

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

[Rik, 2014-02-25 07:46:45]

looks great!

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

https://codereview.chromium.org/179383002/diff/1/Source/core/html/canvas/Canv...
Source/core/html/canvas/CanvasRenderingContext2D.cpp:1007: return
isPointInPathInternal(domPath->path(), x, y, windingRuleString);
should you check if the pointer is valid?

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

https://codereview.chromium.org/179383002/diff/1/Source/core/html/canvas/Canv...
Source/core/html/canvas/CanvasRenderingContext2D.h:158: bool
isPointInPath(DOMPath*, const float x, const float y, const String& winding =
"nonzero");
should the DOMPath have a name?

https://codereview.chromium.org/179383002/diff/1/Source/core/html/canvas/Canv...
Source/core/html/canvas/CanvasRenderingContext2D.h:314: bool
isPointInPathInternal(const Path&, const float x, const float y, const String&
windingRuleString);
same here.

[zino, 2014-02-25 08:46:55]

Thank you for your comment. :)
I've just added validation check.

Also, I don't think DOMPath needs a name in Blink.

Blink Coding Style:
  http://www.chromium.org/blink/coding-style#TOC-Names
  Names - 9

Right:
  void setCount(size_t);
  void doSomething(ScriptExecutionContext*);

Wrong:
  void setCount(size_t count);
  void doSomething(ScriptExecutionContext* context);

[jcgregorio, 2014-02-25 14:29:08]

https://codereview.chromium.org/179383002/diff/20001/Source/core/html/canvas/...
File Source/core/html/canvas/CanvasRenderingContext2D.idl (right):

https://codereview.chromium.org/179383002/diff/20001/Source/core/html/canvas/...
Source/core/html/canvas/CanvasRenderingContext2D.idl:95: boolean
isPointInPath(float x, float y);
Unfortunately, the methods flagged with
[RuntimeEnabled=ExperimentalCanvasFeatures] can't be the first mention of a
method in an IDL, which is part of crbug.com/339000, this ends up hiding all
versions of isPointInPath if the enable-experimental-canvas-features isn't
supplied.

If you build chrome with your patch and run it w/o
--enable-experimental-canvas-features you can see that isPointInPath isn't
defined:

> var c = document.createElement('canvas');
> var ctx = c.getContext('2d');
> ctx.isPointInPath
undefined

Same for isPointInStroke.

[Justin Novosad, 2014-02-25 16:40:02]

Was there an "intent to implement" thread on blink-dev for this?

[Justin Novosad, 2014-02-25 16:45:08]

https://codereview.chromium.org/179383002/diff/20001/Source/core/html/canvas/...
File Source/core/html/canvas/CanvasRenderingContext2D.cpp (right):

https://codereview.chromium.org/179383002/diff/20001/Source/core/html/canvas/...
Source/core/html/canvas/CanvasRenderingContext2D.cpp:1007: if (!domPath)
Shouldn't this case throw a type mismatch DOM exception? Also, the layout test
should cover this case.

https://codereview.chromium.org/179383002/diff/20001/Source/core/html/canvas/...
Source/core/html/canvas/CanvasRenderingContext2D.cpp:1041: if (!domPath)
DOM exception?

[Rik, 2014-02-25 17:37:53]

https://codereview.chromium.org/179383002/diff/20001/Source/core/html/canvas/...
File Source/core/html/canvas/CanvasRenderingContext2D.cpp (right):

https://codereview.chromium.org/179383002/diff/20001/Source/core/html/canvas/...
Source/core/html/canvas/CanvasRenderingContext2D.cpp:1041: if (!domPath)
On 2014/02/25 16:45:08, junov wrote:
> DOM exception?

Shouldn't that be called by blink's IDL preprocessor? If not, you should throw a
type error. http://www.w3.org/TR/WebIDL/#es-object

[jcgregorio, 2014-02-25 17:48:30]

https://codereview.chromium.org/179383002/diff/20001/Source/core/html/canvas/...
File Source/core/html/canvas/CanvasRenderingContext2D.cpp (right):

https://codereview.chromium.org/179383002/diff/20001/Source/core/html/canvas/...
Source/core/html/canvas/CanvasRenderingContext2D.cpp:1041: if (!domPath)
It should be caught by the code generated from the IDL and throw, but a test
case for that would be a good idea.

On 2014/02/25 17:37:54, Rik wrote:
> On 2014/02/25 16:45:08, junov wrote:
> > DOM exception?
> 
> Shouldn't that be called by blink's IDL preprocessor? If not, you should throw
a
> type error. http://www.w3.org/TR/WebIDL/#es-object

[zino, 2014-02-28 05:23:38]

Thank you for reviews. I addressed your all comments.
Could you please review again?

[Rik, 2014-02-28 05:36:42]

On 2014/02/28 05:23:38, zino wrote:
> Thank you for reviews. I addressed your all comments.
> Could you please review again?

lgtm

[jcgregorio, 2014-02-28 15:23:01]

On 2014/02/28 05:36:42, Rik wrote:
> On 2014/02/28 05:23:38, zino wrote:
> > Thank you for reviews. I addressed your all comments.
> > Could you please review again?
> 
> lgtm

non-owner lgtm

[zino, 2014-03-03 09:05:46]

I'm waiting for owner reviews. :)

[Justin Novosad, 2014-03-03 15:52:14]

On 2014/03/03 09:05:46, zino wrote:
> I'm waiting for owner reviews. :)

lgtm

[zino, 2014-03-04 02:01:51]

The CQ bit was checked by jinho.bang@samsung.com

[commit-bot: I haz the power, 2014-03-04 02:02:43]

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

[commit-bot: I haz the power, 2014-03-04 18:25:54]

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

[commit-bot: I haz the power, 2014-03-04 18:25:55]

Retried try job too often on win_layout for step(s) webkit_lint
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_layout...

[zino, 2014-03-05 00:43:56]

The CQ bit was checked by jinho.bang@samsung.com

[commit-bot: I haz the power, 2014-03-05 00:44:51]

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

[commit-bot: I haz the power, 2014-03-05 06:23:48]

Change committed as 168437