Bug 118408 - Introduce DECLARE_FORWARDING_ATTRIBUTE_EVENT_LISTENER() macro
Summary: Introduce DECLARE_FORWARDING_ATTRIBUTE_EVENT_LISTENER() macro
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: SVG (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Chris Dumez
URL:
Keywords: BlinkMergeCandidate
Depends on:
Blocks: 107386
  Show dependency treegraph
 
Reported: 2013-07-05 00:43 PDT by Chris Dumez
Modified: 2013-07-08 08:37 PDT (History)
20 users (show)

See Also:


Attachments
Patch (17.42 KB, patch)
2013-07-05 00:56 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (17.42 KB, patch)
2013-07-05 01:16 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Dumez 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
Comment 1 Chris Dumez 2013-07-05 00:56:36 PDT
Created attachment 206125 [details]
Patch
Comment 2 WebKit Commit Bot 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.
Comment 3 Early Warning System Bot 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
Comment 4 Early Warning System Bot 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
Comment 5 EFL EWS Bot 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
Comment 6 kov's GTK+ EWS bot 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
Comment 7 Chris Dumez 2013-07-05 01:16:54 PDT
Created attachment 206126 [details]
Patch
Comment 8 WebKit Commit Bot 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.
Comment 9 Mikhail Pozdnyakov 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.
Comment 10 Chris Dumez 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.
Comment 11 Chris Dumez 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.
Comment 12 Mikhail Pozdnyakov 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.
Comment 13 Philip Rogers 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.
Comment 14 WebKit Commit Bot 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>
Comment 15 WebKit Commit Bot 2013-07-08 08:37:44 PDT
All reviewed patches have been landed.  Closing bug.