Bug 109997

Summary: [Chromium] Add runtime flag for CanvasPath
Product: WebKit Reporter: Dirk Schulze <krit>
Component: CanvasAssignee: Dirk Schulze <krit>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, dglazkov, eric, fishd, jamesr, jochen, ojan.autocc, tkent+wkapi, tony, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 97333    
Attachments:
Description Flags
Patch
none
Patch abarth: review+, abarth: commit-queue-

Description Dirk Schulze 2013-02-15 18:16:31 PST
Add runtime flag for CanvasPath on Chromium.
Comment 1 Dirk Schulze 2013-02-15 18:22:47 PST
Created attachment 188681 [details]
Patch
Comment 2 WebKit Review Bot 2013-02-15 18:24:15 PST
Please wait for approval from abarth@webkit.org, dglazkov@chromium.org, fishd@chromium.org, jamesr@chromium.org or tkent@chromium.org before submitting, as this patch contains changes to the Chromium public API. See also https://trac.webkit.org/wiki/ChromiumWebKitAPI.
Comment 3 Eric Seidel (no email) 2013-02-15 22:09:09 PST
I'm surprised more of this isn't autogenerated.  Looks reasonable to me.
Comment 4 WebKit Review Bot 2013-02-15 23:31:21 PST
Comment on attachment 188681 [details]
Patch

Attachment 188681 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/16595445

New failing tests:
inspector/profiler/canvas2d/canvas2d-api-changes.html
Comment 5 Dirk Schulze 2013-02-16 00:54:43 PST
Created attachment 188698 [details]
Patch
Comment 6 Darin Fisher (:fishd, Google) 2013-02-18 09:13:05 PST
Chromium WebKit API changes look good to me.
Comment 7 Adam Barth 2013-02-19 15:23:00 PST
Comment on attachment 188698 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=188698&action=review

> LayoutTests/platform/chromium/TestExpectations:4352
> +webkit.org/b/108508 inspector/profiler/canvas2d/canvas2d-api-changes.html [ Failure ]

Rather than skipping this test, we should probably update the baseline.
Comment 8 Adam Barth 2013-02-19 15:23:42 PST
Comment on attachment 188698 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=188698&action=review

> Tools/DumpRenderTree/chromium/TestShell.cpp:153
> +    WebRuntimeFeatures::enableCanvasPath(true);

Presumably we should now pass some tests relating to paths.  There should be some red lines in TestExpectations because of this change.
Comment 9 Dirk Schulze 2013-02-19 15:27:33 PST
I wanted to do that with fixing bug 108508 (enabling the compiler flag). I can land the patches right after each other. So that this will be fixed immediately. This includes fixes for the other tests that are skipped right now as well.
Comment 10 Adam Barth 2013-02-19 18:18:09 PST
Comment on attachment 188698 [details]
Patch

ok
Comment 11 Adam Barth 2013-02-19 18:18:39 PST
Comment on attachment 188698 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=188698&action=review

> Source/WebKit/chromium/features.gypi:39
> -      'ENABLE_CANVAS_PATH=0',
> +      'ENABLE_CANVAS_PATH=1',

I thought this patch enabled the compiler flag as well, but maybe I'm confused.
Comment 12 Dirk Schulze 2013-02-19 20:08:02 PST
(In reply to comment #11)
> (From update of attachment 188698 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=188698&action=review
> 
> > Source/WebKit/chromium/features.gypi:39
> > -      'ENABLE_CANVAS_PATH=0',
> > +      'ENABLE_CANVAS_PATH=1',
> 
> I thought this patch enabled the compiler flag as well, but maybe I'm confused.

I agree. This does enable it, but just for Chromium. I will remove this flag from this patch, but still wait for landing until a patch for bug 108508 got accepted - if that is ok for you.
Comment 13 Adam Barth 2013-02-19 20:22:34 PST
That's fine.  I'm not trying to give you a hard time.  Just seeking to understand.
Comment 14 Dirk Schulze 2013-02-20 13:55:20 PST
Committed r143502: <http://trac.webkit.org/changeset/143502>