Bug 49067

Summary: Convert SVGPoint/SVGPointList to the new SVGPropertyTearOff concept
Product: WebKit Reporter: Nikolas Zimmermann <zimmermann>
Component: SVGAssignee: Nikolas Zimmermann <zimmermann>
Status: RESOLVED FIXED    
Severity: Normal CC: dglazkov, eric, krit, mdelaney7, simon.fraser, webkit.review.bot, zimmermann
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: OS X 10.5   
Bug Depends on:    
Bug Blocks: 47905    
Attachments:
Description Flags
Patch
none
Patch v2
none
Patch v3 krit: review+

Description Nikolas Zimmermann 2010-11-05 05:42:35 PDT
Convert SVGPoint/SVGPointList to the new SVGPropertyTearOff concept.
Comment 1 Nikolas Zimmermann 2010-11-05 05:43:51 PDT
Created attachment 73057 [details]
Patch

Uploading a preliminary patch, yet missing the LayoutTests/ changes, and the ChangeLogs.
I only want to get some early EWS results, as I'm leaving soon.
Comment 2 Nikolas Zimmermann 2010-11-05 06:06:17 PDT
Created attachment 73058 [details]
Patch v2

Updated to trunk, now the patch should apply.
Comment 3 Eric Seidel (no email) 2010-11-05 07:10:43 PDT
Attachment 73058 [details] did not build on chromium:
Build output: http://queues.webkit.org/results/5352001
Comment 4 Dirk Schulze 2010-11-05 08:16:30 PDT
I thought you have a chromium build on your machine, why does every first patch of yours fail on chromium? ;-)
Comment 5 Nikolas Zimmermann 2010-11-05 15:13:48 PDT
(In reply to comment #4)
> I thought you have a chromium build on your machine, why does every first patch of yours fail on chromium? ;-)

I don't test every patch on chromium unless I'm adding new functionality, like StrictTypeChecking etc. that was not implemented on Chromium and had to be tested. If this patch compiles, it will work on V8, as the code is 1:1 equal to JSC.
Comment 6 WebKit Review Bot 2010-11-05 19:01:15 PDT
Attachment 73058 [details] did not build on chromium:
Build output: http://queues.webkit.org/results/5295018
Comment 7 Nikolas Zimmermann 2010-11-08 01:15:46 PST
Created attachment 73222 [details]
Patch v3

Complete patch including ChangeLogs and LayoutTests changes. Only difference to "Patch v2" are CodeGeneratorV8.pm changes, to make it build on chromium/v8.
Comment 8 Dirk Schulze 2010-11-08 05:17:51 PST
Comment on attachment 73222 [details]
Patch v3

r=me. The solution with SVGAnimatedPoints is still a bit sad. We should discuss it on www-svg to change it in the Spec as well. Maybe we can move it into SVG1.1se? Anyway, it doesn't block this patch since it doesn't affect anything on the user side, with the exception that it runs faster. Please update the win proj file before landing.
Comment 9 Nikolas Zimmermann 2010-11-08 05:44:24 PST
(In reply to comment #8)
> (From update of attachment 73222 [details])
> r=me. The solution with SVGAnimatedPoints is still a bit sad. We should discuss it on www-svg to change it in the Spec as well. Maybe we can move it into SVG1.1se? Anyway, it doesn't block this patch since it doesn't affect anything on the user side, with the exception that it runs faster. Please update the win proj file before landing.

Synced the vcproj to trunk before landing, now it should work as expected.
Thanks for the review!

Landed in r71512.
Comment 10 Nikolas Zimmermann 2010-11-08 06:00:40 PST
Trying to fix windows build in r71514. A warning was produced.
Comment 11 Simon Fraser (smfr) 2010-11-08 10:17:35 PST
@@ -21385,7 +21372,6 @@
 			isa = PBXProject;
 			buildConfigurationList = 149C284308902B11008A9EFC /* Build configuration list for PBXProject "WebCore" */;
 			compatibilityVersion = "Xcode 2.4";
-			developmentRegion = English;
 			hasScannedForEncodings = 1;
 			knownRegions = (

Please update to Xcode 3.2.4 so this change does not happen.
Comment 12 Nikolas Zimmermann 2010-11-08 13:10:54 PST
(In reply to comment #11)
> @@ -21385,7 +21372,6 @@
>              isa = PBXProject;
>              buildConfigurationList = 149C284308902B11008A9EFC /* Build configuration list for PBXProject "WebCore" */;
>              compatibilityVersion = "Xcode 2.4";
> -            developmentRegion = English;
>              hasScannedForEncodings = 1;
>              knownRegions = (
> 
> Please update to Xcode 3.2.4 so this change does not happen.

Aww, I'm sorry, forgot to remove the offending line :(
AFAIK there's no XCode 3.2.4 for Leopard, which is still my main dev platform?