Bug 109997 - [Chromium] Add runtime flag for CanvasPath
Summary: [Chromium] Add runtime flag for CanvasPath
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Canvas (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Dirk Schulze
URL:
Keywords:
Depends on:
Blocks: 97333
  Show dependency treegraph
 
Reported: 2013-02-15 18:16 PST by Dirk Schulze
Modified: 2013-02-20 13:55 PST (History)
10 users (show)

See Also:


Attachments
Patch (8.89 KB, patch)
2013-02-15 18:22 PST, Dirk Schulze
no flags Details | Formatted Diff | Diff
Patch (9.71 KB, patch)
2013-02-16 00:54 PST, Dirk Schulze
abarth: review+
abarth: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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>