[webkit-reviews] review denied: [Bug 48385] Add WebKit SPI to scale a WebView : [Attachment 71952] Patch

[bugzilla-daemon at webkit.org]

Simon Fraser (smfr) <simon.fraser at apple.com> has denied Beth Dakin
<bdakin at apple.com>'s request for review:
Bug 48385: Add WebKit SPI to scale a WebView
https://bugs.webkit.org/show_bug.cgi?id=48385

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

------- Additional Comments from Simon Fraser (smfr) <simon.fraser at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=71952&action=review

r- because I think this isn't quite right yet.

> WebCore/ChangeLog:5
> +	   Reviewed by NOBODY (OOPS!).
> +
> +	   This patch adds SPI to Mac WebKit that scales the page by the given 


You should file a bugzilla or put a radar number here.

> WebCore/page/Frame.cpp:988
> +    root->setStyle(newStyle.release());

It worries me that this style might get clobbered by something else. I thought
that styleWillChange() was the right place to "fix up" the style, but that
doesn't seem to be the case.

> WebCore/page/Frame.cpp:992
> +    m_pageScaleFactor = scale;
> +    m_pageScaleOrigin = origin;
> +

I don't see m_pageScaleOrigin being used anywhere.

> WebKit/mac/WebView/WebViewData.h:84
> +    float viewScaleFactor;
> +

Do you really need to store the scale in two places (here and Frame)?

> WebKitTools/DumpRenderTree/mac/DumpRenderTreeDraggingInfo.mm:108
> -#if !defined(BUILDING_ON_TIGER) && !defined(BUILDING_ON_LEOPARD) &&
!defined(BUILDING_ON_SNOW_LEOPARD)
> +#if 0

I don't think you want to commit this.