[webkit-reviews] review denied: [Bug 67846] Web Inspector: refactor decorations management code in SourceFrame. : [Attachment 106866] Patch

[bugzilla-daemon at webkit.org]

Pavel Feldman <pfeldman at chromium.org> has denied Pavel Podivilov
<podivilov at chromium.org>'s request for review:
Bug 67846: Web Inspector: refactor decorations management code in SourceFrame.
https://bugs.webkit.org/show_bug.cgi?id=67846

Attachment 106866: Patch
https://bugs.webkit.org/attachment.cgi?id=106866&action=review

------- Additional Comments from Pavel Feldman <pfeldman at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=106866&action=review


I think bind/unbind adds unnecessary complexity.

> Source/WebCore/inspector/front-end/SourceFrame.js:135
> +		   messageDecoration.bind(this._textViewer);

I think you should:

1) define an @interface for TextViewerDecoration
2) Make TextViewer receive this new type in addDecoration(lineNumber,
decoration)
3) For (2), create a generic StyleTextViewerDecoration in order to workaround
the TextViewer::addDecoration's typeof === "string"
4) do this._textViewer.addDecoration(lineNumber, messageDecoration) on this
line

It would also be great if you could add a test that covers both: pre-load and
post-load cases :)