Bug 71218 - Kill StyleBase.
Summary: Kill StyleBase.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Andreas Kling
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-10-31 05:34 PDT by Andreas Kling
Modified: 2011-12-04 05:39 PST (History)
7 users (show)

See Also:


Attachments
Le Patch (46.50 KB, patch)
2011-10-31 06:29 PDT, Andreas Kling
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Andreas Kling 2011-10-31 05:34:13 PDT
Let's get rid of StyleBase and let CSSRule, StyleSheet and friends stand on their own legs!
Comment 1 Andreas Kling 2011-10-31 06:29:58 PDT
Created attachment 113044 [details]
Le Patch
Comment 2 Early Warning System Bot 2011-10-31 06:41:42 PDT
Comment on attachment 113044 [details]
Le Patch

Attachment 113044 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/10236953
Comment 3 WebKit Review Bot 2011-10-31 06:48:10 PDT
Comment on attachment 113044 [details]
Le Patch

Attachment 113044 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/10237913
Comment 4 Antti Koivisto 2011-10-31 06:49:09 PDT
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.
Comment 5 Andreas Kling 2011-10-31 07:13:31 PDT
Committed r98853: <http://trac.webkit.org/changeset/98853>
Comment 6 Luke Macpherson 2011-10-31 10:00:10 PDT
I like it. Thanks!
Comment 7 Csaba Osztrogonác 2011-10-31 15:56:20 PDT
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?
Comment 8 Andreas Kling 2011-10-31 17:11:31 PDT
(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! :)
Comment 9 Csaba Osztrogonác 2011-10-31 17:46:44 PDT
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.
Comment 10 Csaba Osztrogonác 2011-10-31 18:01:11 PDT
Where is WTF_USE_LIBXML2 defined? I can't see it anywhere.
Comment 11 Csaba Osztrogonác 2011-10-31 18:11:02 PDT
(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?
Comment 12 Andreas Kling 2011-10-31 18:42:21 PDT
(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
Comment 13 Csaba Osztrogonác 2011-11-02 00:45:32 PDT
(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.
Comment 14 Yael 2011-11-02 17:22:24 PDT
(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.
Comment 15 Csaba Osztrogonác 2011-11-03 06:08:09 PDT
(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.
Comment 16 Andreas Kling 2011-11-03 06:17:05 PDT
(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.
Comment 17 Andreas Kling 2011-12-04 05:39:04 PST
No build issue remains.