[webkit-reviews] review granted: [Bug 213242] Web Inspector: Add setScreenSizeOverride API to the Page agent : [Attachment 411419] Patch

[bugzilla-daemon at webkit.org]

Devin Rousso <drousso at apple.com> has granted Carlos Garcia Campos
<cgarcia at igalia.com>'s request for review:
Bug 213242: Web Inspector: Add setScreenSizeOverride API to the Page agent
https://bugs.webkit.org/show_bug.cgi?id=213242

Attachment 411419: Patch

https://bugs.webkit.org/attachment.cgi?id=411419&action=review




--- Comment #30 from Devin Rousso <drousso at apple.com> ---
Comment on attachment 411419
  --> https://bugs.webkit.org/attachment.cgi?id=411419
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=411419&action=review

r=me with a few fixes

> Source/WebCore/page/Frame.cpp:1069
> +    if (!m_overrideScreenSize.isZero())

I think `isEmpty` is probably enough (and simpler), but I don't have a strong
preference.

> Source/WebCore/page/Frame.h:380
> +    Optional<int> m_overrideScreenWidth;
> +    Optional<int> m_overrideScreenHeight;

these don't appear to be necessary anymore

> Source/WebInspectorUI/UserInterface/Base/Main.js:2290
> +	       screenSizeValueInput.value = screenSizeValueInput.placeholder =
WI._overridenDeviceScreenSize || window.screen.width + "x" +
window.screen.height;

NIT: I'd add `(` `)` around the stuff after the `||` so it's explicitly clear