[webkit-reviews] review denied: [Bug 97799] Web Inspector: [Device Metrics] Remove the gutter overlay moving its functionality into the InspectorOverlay : [Attachment 166160] Patch

[bugzilla-daemon at webkit.org]

Pavel Feldman <pfeldman at chromium.org> has denied Alexander Pavlov (apavlov)
<apavlov at chromium.org>'s request for review:
Bug 97799: Web Inspector: [Device Metrics] Remove the gutter overlay moving its
functionality into the InspectorOverlay
https://bugs.webkit.org/show_bug.cgi?id=97799

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

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


> Source/WebCore/inspector/InspectorController.h:96
> +    void deviceMetricsOverrideUpdated(bool);

This should not be exposed on the InspectorController.

> Source/WebCore/inspector/InspectorOverlay.cpp:199
> +    , m_deviceMetricsOverrideEnabled(false)

Is webViewSize not enough?

> Source/WebCore/inspector/InspectorOverlay.cpp:209
> +    if (m_pausedInDebuggerMessage.isNull() && !m_highlightNode &&
!m_highlightRect && !m_deviceMetricsOverrideEnabled)

I would use non-empty webViewSize as a flag.

> Source/WebCore/inspector/InspectorOverlay.cpp:250
> +void InspectorOverlay::deviceMetricsOverrideUpdated(bool enabled)

I don't think you need this method.

> Source/WebCore/inspector/InspectorOverlay.cpp:368
> +    gutterData->setBoolean("shouldShow", m_deviceMetricsOverrideEnabled);

You don't need this flag.