Bug 83771 - Move viewport meta handling til the web process side
Summary: Move viewport meta handling til the web process side
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Kenneth Rohde Christiansen
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-04-12 05:00 PDT by Kenneth Rohde Christiansen
Modified: 2012-04-12 07:43 PDT (History)
9 users (show)

See Also:


Attachments
Patch (26.46 KB, patch)
2012-04-12 05:01 PDT, Kenneth Rohde Christiansen
no flags Details | Formatted Diff | Diff
Patch (28.58 KB, patch)
2012-04-12 06:07 PDT, Kenneth Rohde Christiansen
hausmann: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Kenneth Rohde Christiansen 2012-04-12 05:00:00 PDT
SSIA
Comment 1 Kenneth Rohde Christiansen 2012-04-12 05:01:01 PDT
Created attachment 136878 [details]
Patch
Comment 2 WebKit Review Bot 2012-04-12 05:03:45 PDT
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 3 Jocelyn Turcotte 2012-04-12 05:27:08 PDT
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 4 zalan 2012-04-12 05:32:53 PDT
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?
Comment 5 Kenneth Rohde Christiansen 2012-04-12 05:35:06 PDT
(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.
Comment 6 Kenneth Rohde Christiansen 2012-04-12 06:07:37 PDT
Created attachment 136890 [details]
Patch
Comment 7 zalan 2012-04-12 06:16:00 PDT
lgtm
Comment 8 Simon Hausmann 2012-04-12 06:59:34 PDT
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
Comment 9 Kenneth Rohde Christiansen 2012-04-12 07:43:52 PDT
Landed in 113974