[webkit-reviews] review denied: [Bug 91632] Web Inspector: extracting HighlightInfo from HighlightData in DOMNodeHighlighter : [Attachment 155462] Patch

[bugzilla-daemon at webkit.org]

Pavel Feldman <pfeldman at chromium.org> has denied Sergey Rogulenko
<rogulenko at google.com>'s request for review:
Bug 91632: Web Inspector: extracting HighlightInfo from HighlightData in
DOMNodeHighlighter
https://bugs.webkit.org/show_bug.cgi?id=91632

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

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


Looks mostly good, few nits inline.

> Source/WebCore/inspector/DOMNodeHighlighter.h:60
> +struct HighlightData {

1) I don't think you need highlight data type.
2) I think it is fine to have rect and node assigned at the same time

> Source/WebCore/inspector/InspectorDOMAgent.cpp:1016
> +    if (!highlightInspectorObject)

This should never happen, you should return ErrorString for such cases,

> Source/WebCore/inspector/InspectorDOMAgent.cpp:1021
> +    highlightInspectorObject->getBoolean("showInfo", &showInfo);

I believe we are generating constants for these.