Bug 118408 – Introduce DECLARE_FORWARDING_ATTRIBUTE_EVENT_LISTENER() macro

• Status: RESOLVED
• Resolution: FIXED
• Component: SVG

[Chris Dumez]

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

[Chris Dumez]

Created attachment 206125 [details]
Patch

[WebKit Commit Bot]

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 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 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 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 on attachment 206125 [details]
Patch

Attachment 206125 [details] did not pass gtk-ews (gtk):
Output: http://webkit-queues.appspot.com/results/1037051

[Chris Dumez]

Created attachment 206126 [details]
Patch

[WebKit Commit Bot]

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 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 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 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]

(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 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 on attachment 206126 [details]
Patch

Clearing flags on attachment: 206126

Committed r152451: <http://trac.webkit.org/changeset/152451>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.