[iOS][WK2] Add support for minimal-ui viewports
Created attachment 231846 [details] Patch
Comment on attachment 231846 [details] Patch 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 > +@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).
Comment on attachment 231846 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=231846&action=review > Source/WebCore/ChangeLog:14 > + Initialy, the page is initialized with resetMinimalUI() and start with a normal layout. typo "initially" > Source/WebKit2/ChangeLog:8 > + In the WebKit2 layers, we have two part to minimal-ui. typo "parts" > Source/WebKit2/ChangeLog:10 > + An other part is freezing the state on the first frame. typo "another" > Source/WebCore/page/ViewportConfiguration.cpp:117 > +void ViewportConfiguration::pageWillRenderFirstFrame() I have already registered my objection to this part :D
Created attachment 231849 [details] Patch
<rdar://problem/16283914>
Created attachment 231930 [details] Patch
Created attachment 231933 [details] Patch
Comment on attachment 231933 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=231933&action=review > Source/WebCore/ChangeLog:14 > + Initially, the page is initialized with resetMinimalUI() and start with a normal layout. starts with > Source/WebCore/ChangeLog:18 > + layout size. Any layout after that take into account the minimal-ui. takes minimal-ui into account. > Source/WebCore/page/ViewportConfiguration.cpp:120 > + if (m_pageDidFinishDocumentLoad) > + return; No point having these lines. > Source/WebCore/page/ViewportConfiguration.cpp:286 > int ViewportConfiguration::layoutWidth() const This should return LayoutUnits at some point? > Source/WebKit2/UIProcess/API/Cocoa/WKWebViewPrivate.h:99 > +// Define the smallest size a page take with a regular. With a regular what? > Source/WebKit2/WebProcess/WebPage/WebPage.h:253 > + void didFinishDocumentLoad(WebFrame*); > void didFinishLoad(WebFrame*); What's the difference? Is the first like DOMContentLoaded? > Source/WebKit2/WebProcess/WebPage/ios/WebPageIOS.mm:1992 > resetTextAutosizingBeforeLayoutIfNeeded(m_viewportConfiguration.minimumLayoutSize(), minimumLayoutSize); > + resetTextAutosizingBeforeLayoutIfNeeded(m_viewportConfiguration.minimumLayoutSizeForMinimalUI(), minimumLayoutSizeForMinimalUI); Do we have to call resetTextAutosizingBeforeLayoutIfNeeded twice?
Thanks for the review! (In reply to comment #8) > > Source/WebCore/page/ViewportConfiguration.cpp:286 > > int ViewportConfiguration::layoutWidth() const > > This should return LayoutUnits at some point? No idea. Do we plan to support non-integer document sizes? > > Source/WebKit2/WebProcess/WebPage/WebPage.h:253 > > + void didFinishDocumentLoad(WebFrame*); > > void didFinishLoad(WebFrame*); > > What's the difference? Is the first like DOMContentLoaded? Exactly :) > > Source/WebKit2/WebProcess/WebPage/ios/WebPageIOS.mm:1992 > > resetTextAutosizingBeforeLayoutIfNeeded(m_viewportConfiguration.minimumLayoutSize(), minimumLayoutSize); > > + resetTextAutosizingBeforeLayoutIfNeeded(m_viewportConfiguration.minimumLayoutSizeForMinimalUI(), minimumLayoutSizeForMinimalUI); > > Do we have to call resetTextAutosizingBeforeLayoutIfNeeded twice? We could add some code to do it, but it should not matter much, the layout on resize is so expensive this will be negligible.
Committed r169245: <http://trac.webkit.org/changeset/169245>