Bug 109666

Summary:

[V8] Generate wrapper methods for custom getters/setters

Product:

WebKit

Reporter:

Kentaro Hara <haraken>

Component:

WebCore JavaScript

Assignee:

Kentaro Hara <haraken>

Status:

RESOLVED INVALID

Severity:

Normal

CC:

abarth, andersca, atwilson, dglazkov, japhet, webkit.review.bot

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Patch	none
patch for landing	none

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	Details Formatted Diff Diff
patch for landing (9.56 KB, patch) 2013-02-13 05:57 PST, Kentaro Hara	no flags	Details Formatted Diff Diff
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.