SVG JS bindings "context" pointer needs to move onto binding impls Right now many SVG JS bindings have an extra context() parameter which is stored on the binding object. That puts some SVG bindings up against the cell size limit. The current plan for bug 27088 is to add a JSGlobaObject* on DOMObject. That would put these SVG binding classes over the CELL_SIZE limit. sub IsSVGTypeNeedingContextParameter { my $implClassName = shift; if ($implClassName =~ /SVG/ and not $implClassName =~ /Element/) { return 1 unless $implClassName =~ /SVGPaint/ or $implClassName =~ /SVGColor/ or $implClassName =~ /SVGDocument/; } return 0; }
This might also be a good time to convert more of these custom impls into JSSVGPODTypes.
SVGAngle/SVGPreserveAspectRatio have been converted, the last two remaining types. Working on a patch to fix this bug.
*** Bug 27278 has been marked as a duplicate of this bug. ***
Created attachment 45344 [details] Initial patch This patch is NOT ready for review yet, I just want to see wheter V8 compiles or not ;-)
Attachment 45344 [details] did not pass style-queue: Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1 WebCore/bindings/js/JSSVGPathSegCustom.cpp:23: Found other header before a header this file implements. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4] Skipping input 'WebCore/bindings/js/DOMObjectWithSVGContext.h': Can't open for reading Total errors found: 1
Created attachment 45345 [details] Updated patch Still not r?, add some minor style changes to let EWS go on.
style-queue ran check-webkit-style on attachment 45345 [details] without any errors.
Created attachment 45353 [details] Patch v1 I am not sure how to test the x-frame prototype chain stuff. I guess there are existing tests for HTML, Sam, please enlighten me :-)
style-queue ran check-webkit-style on attachment 45353 [details] without any errors.
Okay, I have tried abarths testcases on bug 27088 with and without this patch and it makes no difference (all SVG* objects say 'PASS'). Also the layout tests show no changes. Now DOMObjectWithSVGContext is gone, and no one is using deprecatedLexicalGlobalObject anymore from SVG ... so I am wondering what I actually fixed here, besides removing the DOMOBjectWithSVGContext hack? Can anyone come up with a testcase which fails without this bug? (Adam Barth pointed out that Eric might know :-)
Comment on attachment 45353 [details] Patch v1 This looks good to me, though at some point I think we should take it a step further and move this stuff completely out of the bindings, and into the core implementation. As for tests, fast/dom/prototype-inheritance.html and fast/dom/prototype-inheritance-2.html should be good tests to look at to test this, but they are probably not currently being hit, because the SVG objects don't all have interfaces on the window or because it takes more effort to make one of these guys that simply iterating the window object. I would say r=me, but I think you should try and make a test first.
bug 20430 is the reason why so many SVG constructor objects are missing. I really should finish the patch I started for that so long ago...
Created attachment 47164 [details] Updated patch Updated this patch against ToT. I am working on further patches in that area, that will touch all JSSVGPOD* files (as I'm going to rewrite the ANIMATED_PROPERTY_* declaration macros for svg/). This patch is blocking my further work, I am unable to create a testcase, and I only did this patch to cleanup code, there is no bug about prototype chain problems, and I don't fully understand what this patch is supposed to fix - for me it's just cleanup :-) Let's get this landed please, Sam was already okay with the code, just a testcase should be created. No one came up with a testcase, so please let's move forward even without a testcase.
Attachment 47164 [details] did not build on mac: Build output: http://webkit-commit-queue.appspot.com/results/204409
(In reply to comment #13) > Created an attachment (id=47164) [details] > Updated patch > > Updated this patch against ToT. > > I am working on further patches in that area, that will touch all JSSVGPOD* > files (as I'm going to rewrite the ANIMATED_PROPERTY_* declaration macros for > svg/). This patch is blocking my further work, I am unable to create a > testcase, and I only did this patch to cleanup code, there is no bug about > prototype chain problems, and I don't fully understand what this patch is > supposed to fix - for me it's just cleanup :-) > > Let's get this landed please, Sam was already okay with the code, just a > testcase should be created. No one came up with a testcase, so please let's > move forward even without a testcase. That is okay with me. Creating the test case has been hard, but it is not crucial in this edge case. I will continue to work on it for the time being.
(In reply to comment #14) > Attachment 47164 [details] did not build on mac: > Build output: http://webkit-commit-queue.appspot.com/results/204409 Hmm, is it really not building? The log is empty, and it worked for me - tried a from scratch build on Leopard.
Comment on attachment 47164 [details] Updated patch r=me assuming you make sure everything builds.
(In reply to comment #17) > (From update of attachment 47164 [details]) > r=me assuming you make sure everything builds. Thanks a lot! During the Xcode project updates, I forgot to say the "Private" role for JSSVGContextCache.h - now WebKit builds fine.
Landed in r53666.