[webkit-reviews] review granted: [Bug 125239] [iOS] Upstream WebCore/rendering changes : [Attachment 218603] Patch

[bugzilla-daemon at webkit.org]

Simon Fraser (smfr) <simon.fraser at apple.com> has granted Daniel Bates
<dbates at webkit.org>'s request for review:
Bug 125239: [iOS] Upstream WebCore/rendering changes
https://bugs.webkit.org/show_bug.cgi?id=125239

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

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


Pleaes refresh from iOS sources.

I think longer term we should replace #if PLATFORM(IOS) with functional macros,
like #if USE(TOUCH_OVERFLOW_SCROLLING) or whatever.

> Source/WebCore/rendering/InlineTextBox.cpp:1018
> +    float documentScale = renderer().frame().documentScale();

This is stale code.

> Source/WebCore/rendering/RenderBlock.cpp:3569
> +    // On the phone we want to constrain VisiblePositions to the editable
region closest to the input position, so

"On the phone" -> "On iOS"

> Source/WebCore/rendering/RenderLayerCompositor.cpp:2681
> +#if PLATFORM(IOS)
> +float RenderLayerCompositor::minimumDocumentScale() const
> +{
> +    return m_renderView.frameView().frame().minimumDocumentScale();
> +}
> +
> +bool RenderLayerCompositor::allowCompositingLayerVisualDegradation() const
> +{
> +    Page* page = m_renderView.frameView().frame().page();
> +    return !page ||
page->settings().allowCompositingLayerVisualDegradation();
> +}
> +#endif
> +
>  float RenderLayerCompositor::deviceScaleFactor() const
>  {
> +#if PLATFORM(IOS)
> +    return m_renderView.frameView().frame().deviceScaleFactor();
> +#else
>      Page* page = this->page();
>      return page ? page->deviceScaleFactor() : 1;
> +#endif
>  }
>  
>  float RenderLayerCompositor::pageScaleFactor() const
>  {
> +#if PLATFORM(IOS)
> +    return m_renderView.frameView().frame().documentScale();
> +#else
>      Page* page = this->page();
>      return page ? page->pageScaleFactor() : 1;
> +#endif
>  }

Your patch is stale here.

> Source/WebCore/rendering/RenderMenuList.cpp:52
> +#include "LocalizedStrings.h"

Is this necessary?

> Source/WebCore/rendering/RenderObject.cpp:1156
> +// which are annotated with additional state which helps the iPhone draw
selections in its unique way.

the iPhone -> iOS