Patch to follow.
Created attachment 115765 [details] Patch
Attachment 115765 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/insp..." exit_code: 1 Source/WebCore/inspector/InspectorCSSAgent.cpp:240: The parameter type should use PassRefPtr instead of RefPtr. [readability/pass_ptr] [5] Source/WebCore/inspector/InspectorCSSAgent.h:67: The parameter type should use PassRefPtr instead of RefPtr. [readability/pass_ptr] [5] Total errors found: 2 in 12 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 115765 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=115765&action=review > Source/WebCore/inspector/Inspector.json:1438 > "type": "object", I don't see why we need this class - you should return these from getStylesFromNode. > Source/WebCore/inspector/InspectorCSSAgent.cpp:256 > + // Early return to avoid unnecessary style recalcs. This early return confuses me - we should treat all the flags equally. > Source/WebCore/inspector/InspectorCSSAgent.cpp:262 > bool needStyleRecalc = element != m_lastElementWithPseudoState || forcePseudoState != m_lastPseudoState; I would just add a check here to not recalc in case of inline query. > Source/WebCore/inspector/InspectorCSSAgent.cpp:311 > + parentStyle->setObject("inlineStyle", styleSheet->buildObjectForStyle(styleSheet->styleForId(InspectorCSSId(styleSheet->id(), 0)))); So asking for inherited styles will return all: inline, matched, etc? This does not sound right. > Source/WebCore/inspector/front-end/CSSStyleModel.js:88 > + if ("computedStyle" in payload) Here and below, use "if (payload.computedStyle)" for compilability. > Source/WebCore/inspector/front-end/CSSStyleModel.js:150 > + CSSAgent.getStylesForNode(nodeId, [], WebInspector.CSSStyleModel.StyleFlags.COMPUTED, callback.bind(null, userCallback)); Computed style should take forced pseudo classes into consideration.
Comment on attachment 115765 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=115765&action=review > Source/WebCore/inspector/front-end/CSSStyleModel.js:59 > + INLINE: 1 << 0, > + COMPUTED: 1 << 1, > + MATCHED: 1 << 2, > + ATTRIBUTES: 1 << 3, > + PSEUDO: 1 << 4, > + INHERITED: 1 << 5, > + ALL: (1 << 6) - 1 We don't use ALLCAPS style for enums. You didn't do it in InspectorCSSAgent.h. There should also be a comment about this needing to match InspectorCSSAgent.h.
Comment on attachment 115765 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=115765&action=review > Source/WebCore/ChangeLog:8 > + * inspector/Inspector.json: Do we have any rules about bumping protocol version when removing methods (or any other compatability change)?
Created attachment 115813 [details] Patch
Attachment 115813 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/insp..." exit_code: 1 Source/WebCore/inspector/InspectorCSSAgent.cpp:240: The parameter type should use PassRefPtr instead of RefPtr. [readability/pass_ptr] [5] Source/WebCore/inspector/InspectorCSSAgent.cpp:240: The parameter type should use PassRefPtr instead of RefPtr. [readability/pass_ptr] [5] Source/WebCore/inspector/InspectorCSSAgent.cpp:240: The parameter type should use PassRefPtr instead of RefPtr. [readability/pass_ptr] [5] Source/WebCore/inspector/InspectorCSSAgent.cpp:297: The parameter type should use PassRefPtr instead of RefPtr. [readability/pass_ptr] [5] Source/WebCore/inspector/InspectorCSSAgent.cpp:297: The parameter type should use PassRefPtr instead of RefPtr. [readability/pass_ptr] [5] Source/WebCore/inspector/InspectorCSSAgent.cpp:311: The parameter type should use PassRefPtr instead of RefPtr. [readability/pass_ptr] [5] Source/WebCore/inspector/InspectorCSSAgent.h:67: The parameter type should use PassRefPtr instead of RefPtr. [readability/pass_ptr] [5] Source/WebCore/inspector/InspectorCSSAgent.h:68: The parameter type should use PassRefPtr instead of RefPtr. [readability/pass_ptr] [5] Source/WebCore/inspector/InspectorCSSAgent.h:68: The parameter type should use PassRefPtr instead of RefPtr. [readability/pass_ptr] [5] Source/WebCore/inspector/InspectorCSSAgent.h:69: The parameter type should use PassRefPtr instead of RefPtr. [readability/pass_ptr] [5] Source/WebCore/inspector/InspectorCSSAgent.h:69: The parameter type should use PassRefPtr instead of RefPtr. [readability/pass_ptr] [5] Source/WebCore/inspector/InspectorCSSAgent.h:69: The parameter type should use PassRefPtr instead of RefPtr. [readability/pass_ptr] [5] Total errors found: 12 in 13 files If any of these errors are false positives, please file a bug against check-webkit-style.
> Do we have any rules about bumping protocol version when removing methods (or any other compatability change)? CSS domain is currently marked as "hidden" as a whole. We don't generate documentation for it. Breaking backwards compatibility for published domains results in a build error generated by generate-inspector-protocol-version script. See http://code.google.com/chrome/devtools/docs/remote-debugging.html#protocol for public compatibility definition.
@Timothy: this is one step toward cleaning up the CSS domain to prepare it for opening to the public.
Comment on attachment 115813 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=115813&action=review > Source/WebCore/inspector/InspectorCSSAgent.cpp:317 > + unsigned forcePseudoState = computePseudoClassMask(forcedPseudoClasses ? forcedPseudoClasses->get() : 0); This logic is not trivial. Please do not copy-paste it. > Source/WebCore/inspector/front-end/AuditRules.js:746 > + function inlineCallback(targetResult, inlineStyle, styleAttributes) You can define targetResult as a variable in the getStyles scope. > LayoutTests/inspector/styles/styles-new-API.html:53 > + function computedCallback(error, computedStyle) Why do we need this?
(In reply to comment #10) > (From update of attachment 115813 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=115813&action=review > > > Source/WebCore/inspector/InspectorCSSAgent.cpp:317 > > + unsigned forcePseudoState = computePseudoClassMask(forcedPseudoClasses ? forcedPseudoClasses->get() : 0); > > This logic is not trivial. Please do not copy-paste it. Done. > > Source/WebCore/inspector/front-end/AuditRules.js:746 > > + function inlineCallback(targetResult, inlineStyle, styleAttributes) > > You can define targetResult as a variable in the getStyles scope. Done. > > LayoutTests/inspector/styles/styles-new-API.html:53 > > + function computedCallback(error, computedStyle) > > Why do we need this? Do you mean I can remove the entire test (styles-new-API.html) altogether? It's a bare API unit test, not an integration test.
Created attachment 116061 [details] Patch
Attachment 116061 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/insp..." exit_code: 1 Source/WebCore/inspector/InspectorCSSAgent.cpp:251: The parameter type should use PassRefPtr instead of RefPtr. [readability/pass_ptr] [5] Source/WebCore/inspector/InspectorCSSAgent.cpp:251: The parameter type should use PassRefPtr instead of RefPtr. [readability/pass_ptr] [5] Source/WebCore/inspector/InspectorCSSAgent.cpp:251: The parameter type should use PassRefPtr instead of RefPtr. [readability/pass_ptr] [5] Source/WebCore/inspector/InspectorCSSAgent.cpp:303: The parameter type should use PassRefPtr instead of RefPtr. [readability/pass_ptr] [5] Source/WebCore/inspector/InspectorCSSAgent.cpp:303: The parameter type should use PassRefPtr instead of RefPtr. [readability/pass_ptr] [5] Source/WebCore/inspector/InspectorCSSAgent.cpp:317: The parameter type should use PassRefPtr instead of RefPtr. [readability/pass_ptr] [5] Source/WebCore/inspector/InspectorCSSAgent.h:67: The parameter type should use PassRefPtr instead of RefPtr. [readability/pass_ptr] [5] Source/WebCore/inspector/InspectorCSSAgent.h:68: The parameter type should use PassRefPtr instead of RefPtr. [readability/pass_ptr] [5] Source/WebCore/inspector/InspectorCSSAgent.h:68: The parameter type should use PassRefPtr instead of RefPtr. [readability/pass_ptr] [5] Source/WebCore/inspector/InspectorCSSAgent.h:69: The parameter type should use PassRefPtr instead of RefPtr. [readability/pass_ptr] [5] Source/WebCore/inspector/InspectorCSSAgent.h:69: The parameter type should use PassRefPtr instead of RefPtr. [readability/pass_ptr] [5] Source/WebCore/inspector/InspectorCSSAgent.h:69: The parameter type should use PassRefPtr instead of RefPtr. [readability/pass_ptr] [5] Total errors found: 12 in 13 files If any of these errors are false positives, please file a bug against check-webkit-style.
Committed r100900: <http://trac.webkit.org/changeset/100900>