RESOLVED INVALID 109666
[V8] Generate wrapper methods for custom getters/setters 
https://bugs.webkit.org/show_bug.cgi?id=109666
Summary [V8] Generate wrapper methods for custom getters/setters
Kentaro Hara
Reported 2013-02-12 23:27:46 PST
Currently V8 directly calls back custom getters/setters written in custom binding files. This makes it impossible for code generators to hook custom getters/setters (e.g. Code generators cannot insert a code for FeatureObservation into custom getters/setters). We should generate wrapper methods for custom getters/setters. In the future, I will insert TRACE_EVENT() macros into these wrapper methods to profile DOM getters/setters/methods.
Attachments
Patch (9.25 KB, patch)
2013-02-12 23:30 PST, Kentaro Hara 		no flags
DetailsFormatted DiffDiff
patch for landing (9.56 KB, patch)
2013-02-13 05:57 PST, Kentaro Hara 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Kentaro Hara
Comment 1 2013-02-12 23:30:15 PST
Created attachment 188020 [details] Patch
Adam Barth
Comment 2 2013-02-12 23:31:59 PST
Comment on attachment 188020 [details] Patch ok
Kentaro Hara
Comment 3 2013-02-12 23:41:01 PST
We might want to rename XXXAccessorGetter() to XXXAttrGetterCustom().
WebKit Review Bot
Comment 4 2013-02-13 00:48:12 PST
Comment on attachment 188020 [details] Patch Rejecting attachment 188020 [details] from commit-queue. New failing tests: http/tests/security/inactive-document-with-empty-security-origin.html Full output: http://queues.webkit.org/results/16491698
WebKit Review Bot
Comment 5 2013-02-13 01:01:05 PST
Attachment 188020 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCore/bindings/scripts/CodeGeneratorV8.pm', u'Source/WebCore/bindings/scripts/test/V8/V8TestInterface.cpp', u'Source/WebCore/bindings/scripts/test/V8/V8TestObj.cpp']" exit_code: 1 Source/WebCore/bindings/scripts/test/V8/V8TestObj.cpp:505: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/WebCore/bindings/scripts/test/V8/V8TestObj.cpp:510: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/WebCore/bindings/scripts/test/V8/V8TestInterface.cpp:136: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/WebCore/bindings/scripts/test/V8/V8TestInterface.cpp:145: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Total errors found: 4 in 4 files If any of these errors are false positives, please file a bug against check-webkit-style.
WebKit Review Bot
Comment 6 2013-02-13 01:55:39 PST
Comment on attachment 188020 [details] Patch Attachment 188020 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/16546032 New failing tests: http/tests/security/inactive-document-with-empty-security-origin.html
Kentaro Hara
Comment 7 2013-02-13 02:14:39 PST
Landed in r142730.
Kentaro Hara
Comment 8 2013-02-13 05:00:28 PST
Rolled out the patch in r142737 as the patch might break chromium browser tests. Will take a detailed look tomorrow.
Andrew Wilson
Comment 9 2013-02-13 05:27:20 PST
Also, this patch was resulting in timeouts running http/tests/security/inactive-document-with-empty-security-origin.html on the chromium layout test canaries.
Kentaro Hara
Comment 10 2013-02-13 05:57:20 PST
Created attachment 188064 [details] patch for landing
Kentaro Hara
Comment 11 2013-02-13 05:58:36 PST
(In reply to comment #9) > Also, this patch was resulting in timeouts running http/tests/security/inactive-document-with-empty-security-origin.html on the chromium layout test canaries. Thanks. I uploaded a patch that fixes the problem. After confirming that ews bots get green, let me land it manually (to avoid style check errors).
WebKit Review Bot
Comment 12 2013-02-13 05:59:06 PST
Attachment 188064 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCore/bindings/scripts/CodeGeneratorV8.pm', u'Source/WebCore/bindings/scripts/test/V8/V8TestInterface.cpp', u'Source/WebCore/bindings/scripts/test/V8/V8TestObj.cpp']" exit_code: 1 Source/WebCore/bindings/scripts/test/V8/V8TestObj.cpp:505: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/WebCore/bindings/scripts/test/V8/V8TestObj.cpp:510: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/WebCore/bindings/scripts/test/V8/V8TestInterface.cpp:136: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/WebCore/bindings/scripts/test/V8/V8TestInterface.cpp:145: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Total errors found: 4 in 4 files If any of these errors are false positives, please file a bug against check-webkit-style.
Kentaro Hara
Comment 13 2013-02-13 17:38:30 PST
Landed in r142833.
Anders Carlsson
Comment 14 2013-09-01 10:32:59 PDT
V8 is gone.
Note You need to log in before you can comment on or make changes to this bug.