| Summary: | Give StylePropertySet a proper mutability flag. | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Andreas Kling <kling> | ||||||
| Component: | CSS | Assignee: | Andreas Kling <kling> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | cmarcelo, koivisto, macpherson, menard, webkit.review.bot | ||||||
| Priority: | P2 | ||||||||
| Version: | 528+ (Nightly build) | ||||||||
| Hardware: | Unspecified | ||||||||
| OS: | Unspecified | ||||||||
| Bug Depends on: | 87750 | ||||||||
| Bug Blocks: | |||||||||
| Attachments: |
|
||||||||
|
Description
Andreas Kling
2012-05-28 12:36:36 PDT
Created attachment 144398 [details]
Patch
Attachment 144398 [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/CSSStyleRule.cpp:64: One line control clauses should not use braces. [whitespace/braces] [4]
Total errors found: 1 in 9 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 144398 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=144398&action=review > Source/WebCore/css/StylePropertySet.h:56 > + static PassRefPtr<StylePropertySet> createMutable(StylePropertyVector& properties) I think we want adopt in this function name for the same reason we have it in the other function name, because the function adopts a property vector. Comment on attachment 144398 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=144398&action=review > Source/WebCore/css/StyleRule.h:123 > StylePropertySet* properties() const { return m_properties.get(); } > + StylePropertySet* mutableProperties(); Could properties() made return a const? We already do that for the immutable element properties. Comment on attachment 144398 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=144398&action=review > Source/WebCore/css/StyleRule.cpp:188 > +StylePropertySet* StyleRule::mutableProperties() > +{ > + if (m_properties && !m_properties->isMutable()) > + m_properties = m_properties->copy(); > + return m_properties.get(); > +} At least the copy constructor assumes that m_properties can't be null and looking at the parser I don't think it ever can be. Created attachment 144526 [details]
Also patch
Attachment 144526 [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/CSSStyleRule.cpp:64: One line control clauses should not use braces. [whitespace/braces] [4]
Total errors found: 1 in 24 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 144526 [details] Also patch View in context: https://bugs.webkit.org/attachment.cgi?id=144526&action=review > Source/WebCore/css/StyleRule.cpp:240 > + if (m_properties && !m_properties->isMutable()) Still don't need null check here. Committed r118761: <http://trac.webkit.org/changeset/118761> |