Bug 82847 - Add mechanism for mapping from StyleRules back to fully constructed CSSStyleRules
Summary: Add mechanism for mapping from StyleRules back to fully constructed CSSStyleR...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks: 77745
  Show dependency treegraph
 
Reported: 2012-04-01 12:28 PDT by Antti Koivisto
Modified: 2012-04-11 20:52 PDT (History)
5 users (show)

See Also:


Attachments
patch (17.04 KB, patch)
2012-04-01 12:52 PDT, Antti Koivisto
kling: review-
Details | Formatted Diff | Diff
with bug url (17.06 KB, patch)
2012-04-01 13:09 PDT, Antti Koivisto
kling: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Antti Koivisto 2012-04-01 12:28:57 PDT
Inspector is using CSSStyleSelector to calculate the CSS rules matched by a given element and expects to be able to walk the parent chain. After 82728 the stylesheet object tree won't have parent pointers and we are going to need another mechanism to support this.
Comment 1 Antti Koivisto 2012-04-01 12:52:24 PDT
Created attachment 135012 [details]
patch
Comment 2 Andreas Kling 2012-04-01 12:55:04 PDT
Comment on attachment 135012 [details]
patch

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

> Source/WebCore/ChangeLog:3
> +        Add mechanism for mapping from StyleRules back to fully constructed CSSStyleRules 

NO BUG NUMBER R- BRO
Comment 3 Antti Koivisto 2012-04-01 13:09:38 PDT
Created attachment 135013 [details]
with bug url
Comment 4 WebKit Review Bot 2012-04-01 13:11:18 PDT
Attachment 135013 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1
Source/WebCore/css/CSSStyleSelector.cpp:2927:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Total errors found: 1 in 5 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 5 Andreas Kling 2012-04-01 13:17:18 PDT
Comment on attachment 135013 [details]
with bug url

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

> Source/WebCore/css/CSSStyleSelector.cpp:292
> +static CSSStyleSheet* fullscreenStyleSheet;

fullScreenStylesheet;

> Source/WebCore/css/CSSStyleSelector.h:495
> +    HashMap<StyleRule*, RefPtr<CSSStyleRule> >  m_styleRuleToCSSOMWrapperMap;

Style: Two spaces after >.
Comment 6 Andreas Kling 2012-04-01 13:17:31 PDT
Comment on attachment 135013 [details]
with bug url

TD;DR
Comment 7 Sam Weinig 2012-04-01 19:25:52 PDT
Would it make sense to just switch the inspector to using the internal representation?
Comment 8 Antti Koivisto 2012-04-02 03:55:17 PDT
(In reply to comment #7)
> Would it make sense to just switch the inspector to using the internal representation?

Sure but that's a separate refactoring. It also doesn't solve this particular problem.
Comment 9 Andreas Kling 2012-04-02 04:03:38 PDT
Comment on attachment 135013 [details]
with bug url

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

r=me. Let's do this thing.

> Source/WebCore/css/CSSStyleSelector.cpp:2905
> +    const Vector<RefPtr<StyleSheet> >& styleSheets = document->styleSheets()->vector();
> +    for (unsigned i = 0; i < styleSheets.size(); ++i) {

You could use the iteration helpers of StyleSheetList directly here instead of grabbing the Vector. NABD.

> Source/WebCore/css/CSSStyleSelector.h:247
> +    CSSStyleRule* ensureFullCSSOMWrapperForStyleRule(StyleRule*);

As you mentioned, this could do with a scarier name, e.g ensureFullCSSOMWrapperForInspectorWhileUsingTerabytesOfMemory().
Comment 10 Antti Koivisto 2012-04-02 06:09:15 PDT
http://trac.webkit.org/changeset/112858