Bug 118408

Summary: Introduce DECLARE_FORWARDING_ATTRIBUTE_EVENT_LISTENER() macro
Product: WebKit Reporter: Chris Dumez <cdumez>
Component: SVGAssignee: Chris Dumez <cdumez>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, darin, d-r, eflews.bot, esprehn+autocc, fmalita, gtk-ews, gyuyoung.kim, kangil.han, krit, laszlo.gombos, mikhail.pozdnyakov, pdr, philn, rniwa, rwlbuis, schenney, webkit-ews, xan.lopez, zimmermann
Priority: P2 Keywords: BlinkMergeCandidate
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 107386    
Attachments:
Description Flags
Patch
none
Patch none

Chris Dumez
Reported 2013-07-05 00:43:21 PDT
Split DEFINE_FORWARDING_ATTRIBUTE_EVENT_LISTENER() macro into separate DECLARE and DEFINE macros to reduce the number of header includes in SVG. This is a prerequirement to merging SVGStyledElement into SVGElement (To avoid header inclusion cycle). Corresponding Blink revision: https://src.chromium.org/viewvc/blink?revision=153530&view=revision
Attachments
Patch (17.42 KB, patch)
2013-07-05 00:56 PDT, Chris Dumez 		no flags
DetailsFormatted DiffDiff
Patch (17.42 KB, patch)
2013-07-05 01:16 PDT, Chris Dumez 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Chris Dumez
Comment 1 2013-07-05 00:56:36 PDT
Created attachment 206125 [details] Patch
WebKit Commit Bot
Comment 2 2013-07-05 00:58:28 PDT
Attachment 206125 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCore/dom/EventTarget.h', u'Source/WebCore/svg/SVGElementInstance.cpp', u'Source/WebCore/svg/SVGElementInstance.h']" exit_code: 1 Source/WebCore/dom/EventTarget.h:176: preprocessor directives (e.g., #ifdef, #define, #import) should never be indented. [whitespace/indent] [4] Source/WebCore/dom/EventTarget.h:180: preprocessor directives (e.g., #ifdef, #define, #import) should never be indented. [whitespace/indent] [4] Source/WebCore/dom/EventTarget.h:182: More than one command on the same line in if [whitespace/parens] [4] Total errors found: 3 in 4 files If any of these errors are false positives, please file a bug against check-webkit-style.
Early Warning System Bot
Comment 3 2013-07-05 01:05:58 PDT
Comment on attachment 206125 [details] Patch Attachment 206125 [details] did not pass qt-ews (qt): Output: http://webkit-queues.appspot.com/results/1007407
Early Warning System Bot
Comment 4 2013-07-05 01:06:10 PDT
Comment on attachment 206125 [details] Patch Attachment 206125 [details] did not pass qt-wk2-ews (qt-wk2): Output: http://webkit-queues.appspot.com/results/942936
EFL EWS Bot
Comment 5 2013-07-05 01:09:29 PDT
Comment on attachment 206125 [details] Patch Attachment 206125 [details] did not pass efl-ews (efl): Output: http://webkit-queues.appspot.com/results/1004414
kov's GTK+ EWS bot
Comment 6 2013-07-05 01:15:52 PDT
Comment on attachment 206125 [details] Patch Attachment 206125 [details] did not pass gtk-ews (gtk): Output: http://webkit-queues.appspot.com/results/1037051
Chris Dumez
Comment 7 2013-07-05 01:16:54 PDT
Created attachment 206126 [details] Patch
WebKit Commit Bot
Comment 8 2013-07-05 01:19:24 PDT
Attachment 206126 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCore/dom/EventTarget.h', u'Source/WebCore/svg/SVGElementInstance.cpp', u'Source/WebCore/svg/SVGElementInstance.h']" exit_code: 1 Source/WebCore/dom/EventTarget.h:176: preprocessor directives (e.g., #ifdef, #define, #import) should never be indented. [whitespace/indent] [4] Source/WebCore/dom/EventTarget.h:180: preprocessor directives (e.g., #ifdef, #define, #import) should never be indented. [whitespace/indent] [4] Source/WebCore/dom/EventTarget.h:182: More than one command on the same line in if [whitespace/parens] [4] Total errors found: 3 in 4 files If any of these errors are false positives, please file a bug against check-webkit-style.
Mikhail Pozdnyakov
Comment 9 2013-07-05 01:26:53 PDT
Comment on attachment 206126 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=206126&action=review > Source/WebCore/svg/SVGElementInstance.h:76 > + Document* ownerDocument() const; why? now it is not implicitly inlined. > Source/WebCore/svg/SVGElementInstance.h:155 > + virtual Node* toNode(); same here.
Chris Dumez
Comment 10 2013-07-05 01:31:02 PDT
Comment on attachment 206126 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=206126&action=review >> Source/WebCore/svg/SVGElementInstance.h:76 >> + Document* ownerDocument() const; > > why? now it is not implicitly inlined. I know but I need to stop including SVGElement.h in this file to prevent a cycle in my next patch. Here the casting from SVGElement* to Document* is an issue with a forward declaration. >> Source/WebCore/svg/SVGElementInstance.h:155 >> + virtual Node* toNode(); > > same here. Well, this one is virtual anyway.
Chris Dumez
Comment 11 2013-07-05 01:33:54 PDT
Comment on attachment 206126 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=206126&action=review >>> Source/WebCore/svg/SVGElementInstance.h:76 >>> + Document* ownerDocument() const; >> >> why? now it is not implicitly inlined. > > I know but I need to stop including SVGElement.h in this file to prevent a cycle in my next patch. Here the casting from SVGElement* to Document* is an issue with a forward declaration. Sorry, casting for the one below. This one is using SVGElement API.
Mikhail Pozdnyakov
Comment 12 2013-07-05 01:37:47 PDT
(In reply to comment #10) > (From update of attachment 206126 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=206126&action=review > > >> Source/WebCore/svg/SVGElementInstance.h:76 > >> + Document* ownerDocument() const; > > > > why? now it is not implicitly inlined. > > I know but I need to stop including SVGElement.h in this file to prevent a cycle in my next patch. Here the casting from SVGElement* to Document* is an issue with a forward declaration. Agree, that makes sense. > > >> Source/WebCore/svg/SVGElementInstance.h:155 > >> + virtual Node* toNode(); > > > > same here. > > Well, this one is virtual anyway. still it can be inlined if compiler knows the exact class of the object.
Philip Rogers
Comment 13 2013-07-08 07:51:04 PDT
Comment on attachment 206126 [details] Patch Fantastic! This gets us so much closer to axing SVGStyledElement :) Not marking as cq+ yet in case Mikhail has further comments.
WebKit Commit Bot
Comment 14 2013-07-08 08:37:41 PDT
Comment on attachment 206126 [details] Patch Clearing flags on attachment: 206126 Committed r152451: <http://trac.webkit.org/changeset/152451>
WebKit Commit Bot
Comment 15 2013-07-08 08:37:44 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.