Bug 103003

Summary: Re-virtualize CSSRule.
Product: WebKit Reporter: Andreas Kling <kling>
Component: CSSAssignee: Andreas Kling <kling>
Status: RESOLVED FIXED    
Severity: Normal CC: allan.jensen, cmarcelo, eric.carlson, feature-media-reviews, koivisto, macpherson, menard, ojan, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch andersca: review+

Andreas Kling
Reported 2012-11-21 18:24:41 PST
CSSRule was split into CSSRule (CSSOM wrapper) and StyleRule (internal data), and keeping it lean is far less important as it's only instantiated on CSSOM API access. We should make it virtual again to make it more hackable and less error prone.
Attachments
Patch (55.97 KB, patch)
2012-11-21 18:34 PST, Andreas Kling
no flags
Patch (55.95 KB, patch)
2012-11-21 18:39 PST, Andreas Kling
andersca: review+
Andreas Kling
Comment 1 2012-11-21 18:34:08 PST
Andreas Kling
Comment 2 2012-11-21 18:39:47 PST
Anders Carlsson
Comment 3 2012-11-21 19:05:23 PST
Comment on attachment 175561 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=175561&action=review > Source/WebCore/css/StyleResolver.cpp:2573 > - if (cssRule->isImportRule()) > + if (cssRule->type() == CSSRule::IMPORT_RULE) > collectCSSOMWrappers(wrapperMap, static_cast<CSSImportRule*>(cssRule)->styleSheet()); > - else if (cssRule->isMediaRule()) > + else if (cssRule->type() == CSSRule::MEDIA_RULE) > collectCSSOMWrappers(wrapperMap, static_cast<CSSMediaRule*>(cssRule)); > #if ENABLE(CSS_REGIONS) > - else if (cssRule->isRegionRule()) > + else if (cssRule->type() == CSSRule::WEBKIT_REGION_RULE) > collectCSSOMWrappers(wrapperMap, static_cast<WebKitCSSRegionRule*>(cssRule)); > #endif > - else if (cssRule->isStyleRule()) { > + else if (cssRule->type() == CSSRule::STYLE_RULE) { > CSSStyleRule* cssStyleRule = static_cast<CSSStyleRule*>(cssRule); > wrapperMap.add(cssStyleRule->styleRule(), cssStyleRule); > } This looks like it should be a switch statement instead.
Andreas Kling
Comment 4 2012-11-21 19:47:31 PST
Antti Koivisto
Comment 5 2012-11-22 01:42:24 PST
Comment on attachment 175560 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=175560&action=review > Source/WebCore/css/CSSStyleRule.cpp:129 > + ASSERT(rule->isStyleRule()); > + m_styleRule = static_cast<StyleRule*>(rule); Would be good to add asStyleFooRule() casting functions at some point.
Note You need to log in before you can comment on or make changes to this bug.