[webkit-reviews] review granted: [Bug 171013] Expose obscured insets to web content (as "safe area insets") : [Attachment 307521] Patch

[bugzilla-daemon at webkit.org]

Wenson Hsieh <wenson_hsieh at apple.com> has granted Tim Horton
<thorton at apple.com>'s request for review:
Bug 171013: Expose obscured insets to web content (as "safe area insets")
https://bugs.webkit.org/show_bug.cgi?id=171013

Attachment 307521: Patch

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




--- Comment #4 from Wenson Hsieh <wenson_hsieh at apple.com> ---
Comment on attachment 307521
  --> https://bugs.webkit.org/attachment.cgi?id=307521
Patch

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

r=me

> Source/WebCore/dom/ConstantPropertyMap.cpp:96
> +void ConstantPropertyMap::didChangeObscuredInsets()

imo, updateObscuredInsetsFromPage would be a more informative name here, or
maybe just updateObscuredInsets, and then pass a const ref to the insets
around.

> Source/WebCore/page/Page.cpp:2292
> +void Page::setObscuredInsets(FloatBoxExtent insets)

Nit - I think this can be a const &

> Source/WebCore/page/Page.h:333
> +    WEBCORE_EXPORT void setObscuredInsets(FloatBoxExtent);

Nit - const FloatBoxExtent&?

> Source/WebCore/page/Page.h:692
>      // This is only used for history scroll position restoration.

Is this comment still valid? (now that it's referring to
m_enclosedInScrollableAncestorView)