Bug 103908

Summary: JSC: visitChildren() not automatically generated for uncustomized EventTarget interfaces
Product: WebKit Reporter: Michael Pruett <michael>
Component: New BugsAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, darin, ggaren, haraken, japhet, jsbell, ossy, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
ggaren: review+
Patch
ggaren: review+
Patch none

Michael Pruett
Reported 2012-12-03 10:36:14 PST
CodeGeneratorJS.pm fails to generate visitChildren() for EventTarget interfaces with no functions. Examples of such interfaces are IDBOpenDBRequest and IDBVersionChangeRequest.
Attachments
Patch (1.93 KB, patch)
2012-12-03 11:01 PST, Michael Pruett 		no flags
DetailsFormatted DiffDiff
Patch (4.43 KB, patch)
2012-12-03 11:57 PST, Michael Pruett 		ggaren: review+
DetailsFormatted DiffDiff
Patch (4.47 KB, patch)
2012-12-03 13:21 PST, Michael Pruett 		ggaren: review+
DetailsFormatted DiffDiff
Patch (4.46 KB, patch)
2012-12-03 16:02 PST, Michael Pruett 		no flags
DetailsFormatted DiffDiff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
Michael Pruett
Comment 1 2012-12-03 11:01:43 PST
Created attachment 177288 [details] Patch
Michael Pruett
Comment 2 2012-12-03 11:57:10 PST
Created attachment 177300 [details] Patch
Geoffrey Garen
Comment 3 2012-12-03 13:12:19 PST
Comment on attachment 177300 [details] Patch Would be good to explain why this is needed in your ChangeLog. For example, is this only required to support a new set of classes, or are you fixing a bug in some existing classes?
Michael Pruett
Comment 4 2012-12-03 13:18:25 PST
(In reply to comment #3) > (From update of attachment 177300 [details]) > Would be good to explain why this is needed in your ChangeLog. For example, is this only required to support a new set of classes, or are you fixing a bug in some existing classes? This change is needed to support the classes IDBOpenDBRequest and IDBVersionChagngeRequest, which are not currently enabled for JSC. These interfaces have the EventTarget attribute but contain no members other than event listeners, and there's no reason for these interfaces to have custom implementations for visitChildren(). There seem to be no other interfaces for which this test fails, but I see no reason for the test I removed (i.e. $numFunctions > 0 || $numCachedAttributes > 0) to remain.
Michael Pruett
Comment 5 2012-12-03 13:21:26 PST
Created attachment 177314 [details] Patch I've updated the ChangeLog to mention the interfaces for which this change is necessary.
Geoffrey Garen
Comment 6 2012-12-03 13:22:50 PST
Comment on attachment 177314 [details] Patch r=me
WebKit Review Bot
Comment 7 2012-12-03 15:03:12 PST
Comment on attachment 177314 [details] Patch Rejecting attachment 177314 [details] from commit-queue. Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2 Last 500 characters of output: ripts/update-webkit line 152. Failed to run "['Tools/Scripts/update-webkit', '--chromium', '--force-update']" exit_code: 2 Updating OpenSource From http://git.chromium.org/external/Webkit 76cfc20..41a9297 HEAD -> origin/HEAD error: Ref refs/remotes/origin/master is at 41a92978811a093ec854e669c98e2c578bb1b3a1 but expected 76cfc203424f158e9f12fb85dfd4a4491fd1462e ! 76cfc20..41a9297 master -> origin/master (unable to update local ref) Died at Tools/Scripts/update-webkit line 152. Full output: http://queues.webkit.org/results/15106662
Kentaro Hara
Comment 8 2012-12-03 15:49:41 PST
It looks like the patch is conflicting with the WebKit trunk. Please rebase your patch with the latest WebKit, fill "Geoffrey Garen" in the reviewer field in ChangeLog, and reupload it.
Michael Pruett
Comment 9 2012-12-03 16:02:21 PST
Created attachment 177361 [details] Patch
Kentaro Hara
Comment 10 2012-12-03 16:02:47 PST
Comment on attachment 177361 [details] Patch Thanks!
WebKit Review Bot
Comment 11 2012-12-03 23:49:31 PST
Comment on attachment 177361 [details] Patch Rejecting attachment 177361 [details] from commit-queue. Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2 Last 500 characters of output: ripts/update-webkit line 152. Failed to run "['Tools/Scripts/update-webkit', '--chromium', '--force-update']" exit_code: 2 Updating OpenSource From http://git.chromium.org/external/Webkit ef0ca77..8cbb25f HEAD -> origin/HEAD error: Ref refs/remotes/origin/master is at 8cbb25f3febaaee8d69fbea1e5b98dd0173b71a9 but expected ef0ca77a554ed2a07b001ee4935266d6301b2bf8 ! ef0ca77..8cbb25f master -> origin/master (unable to update local ref) Died at Tools/Scripts/update-webkit line 152. Full output: http://queues.webkit.org/results/15098929
WebKit Review Bot
Comment 12 2012-12-04 00:07:25 PST
Comment on attachment 177361 [details] Patch Clearing flags on attachment: 177361 Committed r136482: <http://trac.webkit.org/changeset/136482>
WebKit Review Bot
Comment 13 2012-12-04 00:07:30 PST
All reviewed patches have been landed. Closing bug.
Csaba Osztrogonác
Comment 14 2012-12-04 01:33:52 PST
(In reply to comment #12) > (From update of attachment 177361 [details]) > Clearing flags on attachment: 177361 > > Committed r136482: <http://trac.webkit.org/changeset/136482> It broke the bindings tests. Could you fix it?
Kentaro Hara
Comment 15 2012-12-04 01:36:07 PST
(In reply to comment #14) > It broke the bindings tests. Could you fix it? Sorry. Fixed in r136490.
Note You need to log in before you can comment on or make changes to this bug.