Bug 71218 – Kill StyleBase.

• Status: RESOLVED
• Resolution: FIXED
• Component: CSS

[Andreas Kling]

Let's get rid of StyleBase and let CSSRule, StyleSheet and friends stand on their own legs!

[Andreas Kling]

Created attachment 113044 [details]
Le Patch

[Early Warning System Bot]

Comment on attachment 113044 [details]
Le Patch

Attachment 113044 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/10236953

[WebKit Review Bot]

Comment on attachment 113044 [details]
Le Patch

Attachment 113044 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/10237913

[Antti Koivisto]

Comment on attachment 113044 [details]
Le Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=113044&action=review

r=me, very nice!

> Source/WebCore/css/CSSStyleSheet.cpp:254
> +    for (CSSStyleSheet* sheet = this; sheet; sheet = sheet->parentStyleSheet()) {
> +        if (Node* ownerNode = sheet->ownerNode())
> +            return ownerNode->document();

Wonder if the ownerNode is always found from the root. If so, this could be a root search + assertion that ownerNode is null in child stylesheets.

[Andreas Kling]

Committed r98853: <http://trac.webkit.org/changeset/98853>

[Luke Macpherson]

I like it. Thanks!

[Csaba Osztrogonác]

It broke the debug build on the Qt bots:

../../WebCore/debug/libwebcore.a(XSLStyleSheetQt.o): In function `~XSLStyleSheet':
/ramdisk/qt-linux-32-debug/build/WebKitBuild/Debug/WebCore/../../../Source/WebCore/xml/XSLStyleSheetQt.cpp:45: undefined reference to `WebCore::XSLImportRule::parentStyleSheet() const'
/ramdisk/qt-linux-32-debug/build/WebKitBuild/Debug/WebCore/../../../Source/WebCore/xml/XSLStyleSheetQt.cpp:45: undefined reference to `WebCore::XSLImportRule::parentStyleSheet() const'
/ramdisk/qt-linux-32-debug/build/WebKitBuild/Debug/WebCore/../../../Source/WebCore/xml/XSLStyleSheetQt.cpp:45: undefined reference to `WebCore::XSLImportRule::parentStyleSheet() const'

Could you fix the build please?

[Andreas Kling]

(In reply to comment #7)
> It broke the debug build on the Qt bots:
> 
> ../../WebCore/debug/libwebcore.a(XSLStyleSheetQt.o): In function `~XSLStyleSheet':
> /ramdisk/qt-linux-32-debug/build/WebKitBuild/Debug/WebCore/../../../Source/WebCore/xml/XSLStyleSheetQt.cpp:45: undefined reference to `WebCore::XSLImportRule::parentStyleSheet() const'
> /ramdisk/qt-linux-32-debug/build/WebKitBuild/Debug/WebCore/../../../Source/WebCore/xml/XSLStyleSheetQt.cpp:45: undefined reference to `WebCore::XSLImportRule::parentStyleSheet() const'
> /ramdisk/qt-linux-32-debug/build/WebKitBuild/Debug/WebCore/../../../Source/WebCore/xml/XSLStyleSheetQt.cpp:45: undefined reference to `WebCore::XSLImportRule::parentStyleSheet() const'
> 
> Could you fix the build please?

I'll fix it first thing tomorrow dude, in the office! :)

[Csaba Osztrogonác]

It seems that WTF_USE_LIBXML2 is false in WebCore.pro. This function is declared in the header, but the cpp file isn't linked.

[Csaba Osztrogonác]

Where is WTF_USE_LIBXML2 defined? I can't see it anywhere.

[Csaba Osztrogonác]

(In reply to comment #10)
> Where is WTF_USE_LIBXML2 defined? I can't see it anywhere.

Checking this non-existent define introduced in http://trac.webkit.org/changeset/89516 . Yael, Andreas have you got any idea what is it?

[Andreas Kling]

(In reply to comment #11)
> (In reply to comment #10)
> > Where is WTF_USE_LIBXML2 defined? I can't see it anywhere.
> 
> Checking this non-existent define introduced in http://trac.webkit.org/changeset/89516 . Yael, Andreas have you got any idea what is it?

Hm, looks like WebCore/xml/XSLImportRule.cpp should always be built if ENABLE_XSLT; independent of USE_LIBXML2

[Csaba Osztrogonác]

(In reply to comment #12)
> (In reply to comment #11)
> > (In reply to comment #10)
> > > Where is WTF_USE_LIBXML2 defined? I can't see it anywhere.
> > 
> > Checking this non-existent define introduced in http://trac.webkit.org/changeset/89516 . Yael, Andreas have you got any idea what is it?
> 
> Hm, looks like WebCore/xml/XSLImportRule.cpp should always be built if ENABLE_XSLT; independent of USE_LIBXML2

But there isn't any WTF_USE_LIBXML2 define, so USE(LIBXML2) is always false.

FYI: A workaround landed in http://trac.webkit.org/changeset/98933.

[Yael]

(In reply to comment #13)
> (In reply to comment #12)
> > (In reply to comment #11)
> > > (In reply to comment #10)
> > > > Where is WTF_USE_LIBXML2 defined? I can't see it anywhere.
> > > 
> > > Checking this non-existent define introduced in http://trac.webkit.org/changeset/89516 . Yael, Andreas have you got any idea what is it?
> > 
> > Hm, looks like WebCore/xml/XSLImportRule.cpp should always be built if ENABLE_XSLT; independent of USE_LIBXML2
> 
> But there isn't any WTF_USE_LIBXML2 define, so USE(LIBXML2) is always false.
> 
> FYI: A workaround landed in http://trac.webkit.org/changeset/98933.

WTF_USE_LIBXML2 isn't defined because I don't have a solution for mac and Windows. Enabling it requires libxml2 and libxslt.

[Csaba Osztrogonác]

(In reply to comment #12)
> (In reply to comment #11)
> > (In reply to comment #10)
> > > Where is WTF_USE_LIBXML2 defined? I can't see it anywhere.
> > 
> > Checking this non-existent define introduced in http://trac.webkit.org/changeset/89516 . Yael, Andreas have you got any idea what is it?
> 
> Hm, looks like WebCore/xml/XSLImportRule.cpp should always be built if ENABLE_XSLT; independent of USE_LIBXML2

It would be better than the landed hacky workaround.

[Andreas Kling]

(In reply to comment #15)
> (In reply to comment #12)
> > (In reply to comment #11)
> > > (In reply to comment #10)
> > > > Where is WTF_USE_LIBXML2 defined? I can't see it anywhere.
> > > 
> > > Checking this non-existent define introduced in http://trac.webkit.org/changeset/89516 . Yael, Andreas have you got any idea what is it?
> > 
> > Hm, looks like WebCore/xml/XSLImportRule.cpp should always be built if ENABLE_XSLT; independent of USE_LIBXML2
> 
> It would be better than the landed hacky workaround.

Yep, not sure why that was never fixed; I told Renata it was the wrong solution. Anyhow, no reason to revert that change, the method should remain inline.

[Andreas Kling]

No build issue remains.