[webkit-reviews] review granted: [Bug 133162] [iOS][WK2] Add support for minimal-ui viewports : [Attachment 231846] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Wed May 21 18:25:45 PDT 2014
Simon Fraser (smfr) <simon.fraser at apple.com> has granted Benjamin Poulain
<benjamin at webkit.org>'s request for review:
Bug 133162: [iOS][WK2] Add support for minimal-ui viewports
https://bugs.webkit.org/show_bug.cgi?id=133162
Attachment 231846: Patch
https://bugs.webkit.org/attachment.cgi?id=231846&action=review
------- Additional Comments from Simon Fraser (smfr) <simon.fraser at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=231846&action=review
> Source/WebCore/ChangeLog:20
> + When the first frame is rendered, the WebKit layer calls
pageWillRenderFirstFrame() which freeze
freezes
> Source/WebKit2/ChangeLog:8
> + In the WebKit2 layers, we have two part to minimal-ui.
two parts
> Source/WebKit2/ChangeLog:10
> + An other part is freezing the state on the first frame.
Another
> Source/WebCore/page/ViewportConfiguration.h:86
> + void pageWillRenderFirstFrame();
"frame" is ambiguous (with Frame/iframe). Maybe didFirstRender or something?
Can you just leverage a layout milestone?
> Source/WebCore/page/ViewportConfiguration.h:118
> + bool m_firstFrameCommited;
Name should be consistent with pageWillRenderFirstFrame. "Committing" is a
WK2/compositing notion, and should not be used here.
> Source/WebKit2/UIProcess/API/Cocoa/WKWebViewPrivate.h:103
> + at property (nonatomic, setter=_setLargestUnobscuredSizeOverride:) CGSize
_largestUnobscuredSizeOverride;
Not sure what override means.
> Source/WebKit2/UIProcess/WebPageProxy.h:602
> + void setLargestUnobscuredSize(const WebCore::FloatSize&);
largest -> maximum?
> Source/WebKit2/WebProcess/WebPage/ios/WebPageIOS.mm:1930
> + if (m_hasRenderedContentAfterDidCommitLoad)
> + return;
> +
> + m_hasRenderedContentAfterDidCommitLoad = true;
> + m_viewportConfiguration.pageWillRenderFirstFrame();
Don't really like this statefulness in another place. We should use layout
milestones for this.
> Source/WebKit2/WebProcess/WebPage/mac/RemoteLayerTreeDrawingArea.mm:249
> + m_webPage->willFlushLayers();
> +
> m_webPage->layoutIfNeeded();
I think you should call willFlush after layoutIfNeeded (in case it tries to
query layout).
More information about the webkit-reviews
mailing list