[webkit-reviews] review denied: [Bug 50227] Web Inspector: Enable switching between revisions of stylesheets : [Attachment 75131] [PATCH] Suggested solution

[bugzilla-daemon at webkit.org]

Yury Semikhatsky <yurys at chromium.org> has denied Alexander Pavlov (apavlov)
<apavlov at chromium.org>'s request for review:
Bug 50227: Web Inspector: Enable switching between revisions of stylesheets
https://bugs.webkit.org/show_bug.cgi?id=50227

Attachment 75131: [PATCH] Suggested solution
https://bugs.webkit.org/attachment.cgi?id=75131&action=review

------- Additional Comments from Yury Semikhatsky <yurys at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=75131&action=review

> WebCore/inspector/front-end/CSSStyleModel.js:168
> +	   function callback(styleSheetId, href, content)

Just use styleSheetId from the outer scope without passing it as additional
explicit parameter, this way you would avoid callback.bind call below.

> WebCore/inspector/front-end/CSSStyleModel.js:221
> +    return payload ? new WebInspector.CSSStyleDeclaration(payload) : null;

The contract should be that the caller checks for null payload because
otherwise we have two null checks instead of one, first one in the parse method
and the second one at the call site where we check for null parsed value.

> WebCore/inspector/front-end/CSSStyleModel.js:369
> +    return payload ? new WebInspector.CSSRule(payload) : null;

ditto

> WebCore/inspector/front-end/CSSStyleModel.js:410
> +    if (!payload)

ditto

> WebCore/inspector/front-end/CSSStyleModel.js:530
> +    return payload ? new WebInspector.CSSStyleSheet(payload) : null;

ditto