| Summary: | Move viewport meta handling til the web process side | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Kenneth Rohde Christiansen <kenneth> | ||||||
| Component: | WebKit2 | Assignee: | Kenneth Rohde Christiansen <kenneth> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | cgarcia, gustavo, hausmann, jturcotte, menard, mrobinson, webkit.review.bot, zalan, zoltan | ||||||
| Priority: | P2 | ||||||||
| Version: | 528+ (Nightly build) | ||||||||
| Hardware: | Unspecified | ||||||||
| OS: | Unspecified | ||||||||
| Attachments: |
|
||||||||
|
Description
Kenneth Rohde Christiansen
2012-04-12 05:00:00 PDT
Created attachment 136878 [details]
Patch
Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See http://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API Comment on attachment 136878 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=136878&action=review > Source/WebKit2/UIProcess/API/qt/qquickwebview.cpp:-710 > - WebCore::restrictMinimumScaleFactorToViewportSize(attr, availableSize); > - WebCore::restrictScaleFactorToInitialScaleIfNotUserScalable(attr); What was the use of those two calls? They disappeared now. Comment on attachment 136878 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=136878&action=review > Source/WebKit2/UIProcess/API/qt/qquickwebview.cpp:213 > } I'd swap updateViewportSize() and drawingAres()->setSize(). updateViewportSize() calls _q_contentViewportChanged() which sets the visiblerect on the drawingArea. There could be some side effect to set the visiblerect, when the size is not yet set properly. > Source/WebKit2/WebProcess/WebCoreSupport/WebChromeClient.cpp:-715 > - Viewport property change callback is now bundled with TILED_BACKING_STORE. Is it intentional? > Source/WebKit2/WebProcess/WebPage/WebPage.cpp:885 > } WebChromeClient::dispatchViewportPropertiesDidChange() and WebPage::setViewportSize() are very lookalike now. Wouldn't be better to move the common code to a dedicated function? (In reply to comment #3) > (From update of attachment 136878 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=136878&action=review > > > Source/WebKit2/UIProcess/API/qt/qquickwebview.cpp:-710 > > - WebCore::restrictMinimumScaleFactorToViewportSize(attr, availableSize); > > - WebCore::restrictScaleFactorToInitialScaleIfNotUserScalable(attr); > > What was the use of those two calls? They disappeared now. True and that might need to be revisited later. Currently a lot of additional code in Grob were due to us calling these (the implicit/explicit). That code were related to fit to width. Created attachment 136890 [details]
Patch
lgtm Comment on attachment 136890 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=136890&action=review > Source/WebKit2/ChangeLog:9 > + we now do everything on the web process side, and just sends sends -> send Landed in 113974 |