Bug 78257

Created attachment 126383 [details] patch

Comment on attachment 126383 [details] patch Attachment 126383 [details] did not pass qt-ews (qt): Output: http://queues.webkit.org/results/11473424

Created attachment 126402 [details] try to fix qt build, other cleanups

Attachment 126402 [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/CSSStyleDeclaration.h:39: More than one command on the same line [whitespace/newline] [4] Total errors found: 1 in 20 files If any of these errors are false positives, please file a bug against check-webkit-style.

Comment on attachment 126402 [details] try to fix qt build, other cleanups Attachment 126402 [details] did not pass qt-ews (qt): Output: http://queues.webkit.org/results/11484444

Comment on attachment 126402 [details] try to fix qt build, other cleanups View in context: https://bugs.webkit.org/attachment.cgi?id=126402&action=review TD;DR > Source/WebCore/css/StylePropertySet.cpp:50 > + PropertySetCSSStyleDeclaration(StylePropertySet* propertySet) : m_propertySet(propertySet) { } explicit > Source/WebCore/css/StylePropertySet.h:170 > - mutable RefPtr<PropertySetCSSStyleDeclaration> m_cssStyleDeclaration; > + mutable OwnPtr<PropertySetCSSStyleDeclaration> m_cssStyleDeclaration; We should move this to a hashmap and have a bit for "has CSSOM object"

(In reply to comment #6) > TD;DR TD? > explicit How explicit? > We should move this to a hashmap and have a bit for "has CSSOM object" Sure Sherlock, but not in this patch.

Created attachment 126444 [details] still fixing qt build

Attachment 126444 [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/CSSStyleDeclaration.h:39: More than one command on the same line [whitespace/newline] [4] Total errors found: 1 in 21 files If any of these errors are false positives, please file a bug against check-webkit-style.

(In reply to comment #7) > (In reply to comment #6) > > TD;DR > > TD? Too drunk; didn't review. :/ > > explicit > > How explicit? I meant you should make this constructor explicit.

Comment on attachment 126444 [details] still fixing qt build View in context: https://bugs.webkit.org/attachment.cgi?id=126444&action=review > Source/WebCore/css/CSSComputedStyleDeclaration.h:48 > +class CSSComputedStyleDeclaration : public CSSStyleDeclaration, private WTF::RefCountedBase { Why do we need to inherit from RefCountedBase here? Shadowing ref() and deref() in RefCountedBase with overridden virtuals from CSSStyleDeclaration is a little confusing and the ChangeLog doesn't explain what's going on. > Source/WebCore/css/StylePropertySet.cpp:50 > + PropertySetCSSStyleDeclaration(StylePropertySet* propertySet) : m_propertySet(propertySet) { } explicit PropertySetCSSStyleDeclaration(StylePropertySet* propertySet) : m_propertySet(propertySet) { } Because you're worth it.

Created attachment 126525 [details] refcount by hand instead of calling to RefCountedBase

(In reply to comment #11) > > Source/WebCore/css/StylePropertySet.cpp:50 > > + PropertySetCSSStyleDeclaration(StylePropertySet* propertySet) : m_propertySet(propertySet) { } > > explicit PropertySetCSSStyleDeclaration(StylePropertySet* propertySet) : m_propertySet(propertySet) { } > Because you're worth it. I don't see what value that would add.

Attachment 126525 [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/CSSStyleDeclaration.h:39: More than one command on the same line [whitespace/newline] [4] Total errors found: 1 in 21 files If any of these errors are false positives, please file a bug against check-webkit-style.