Bug 95557 - [Qt] ASSERT(m_useFixedLayout) in WebPage when using QRawWebView without setting fixed layout
Summary: [Qt] ASSERT(m_useFixedLayout) in WebPage when using QRawWebView without setti...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Qt (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Balazs Kelemen
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-08-31 04:44 PDT by Balazs Kelemen
Modified: 2012-08-31 10:09 PDT (History)
9 users (show)

See Also:


Attachments
Patch (2.13 KB, patch)
2012-08-31 04:50 PDT, Balazs Kelemen
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Balazs Kelemen 2012-08-31 04:44:56 PDT
This has been revealed in https://bugs.webkit.org/show_bug.cgi?id=90394.
At first I was thinking that this is simply a matter of a missing call to WebPageProxy::setUseFixedLayout.
But if I made this call, than I could not use qrawwebview for layout tests because it always sets the viewport size to 980x800. I was trying setFixedLayoutSize but it does not help. In WebPage::sendViewportAttributesChanged the given size argument is adjusted in accordance to the viewport tag specification (WebCore::computeViewportAttributes). It is strange to me that calling this function with default desktop arguments (devicewidth/height==viewportSize.width()/height(), args=default, minimumWidth=viewportSize.width(), scale=1, viewportSize=800x600) adjust the size to 980x800 but I don't know the specification.
In order to fix this I see two option:
 1. introduce a setting in WebPage/~Proxy that enables/disables handling the viewport attributes (qrawwebview is missing the UI side of it anyway)
 2. let WTR with qrawwebview run in non fixed layout mode and don't call webpageproxy->setViewportSize.
 3. find out how can we set a custom viewport size in fixed layout mode

3 seems to be the best but I had no success with that, so for now I would like to go with 2.
Comment 1 Balazs Kelemen 2012-08-31 04:50:04 PDT
Created attachment 161665 [details]
Patch
Comment 2 Kenneth Rohde Christiansen 2012-08-31 05:41:10 PDT
Comment on attachment 161665 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=161665&action=review

> Source/WebKit2/UIProcess/API/qt/raw/qrawwebview.cpp:334
> +    // Note: it's up to our client to decide if it wants fixed layout.
> +    if (d->m_webPageProxy->useFixedLayout())
> +        d->m_webPageProxy->setViewportSize(size);

whether*. Anyway does that commetn add any value?

> Source/WebKit2/UIProcess/API/qt/raw/qrawwebview.cpp:339
> +        WebKit::LayerTreeCoordinatorProxy* coordinator = drawingArea->layerTreeCoordinatorProxy();
> +        if (!coordinator)
> +            return;
> +        coordinator->setContentsSize(WebCore::FloatSize(size.width(), size.height()));

That could be written in two lines instead of 4
Comment 3 Balazs Kelemen 2012-08-31 06:04:19 PDT
(In reply to comment #2)
> (From update of attachment 161665 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=161665&action=review
> 
> > Source/WebKit2/UIProcess/API/qt/raw/qrawwebview.cpp:334
> > +    // Note: it's up to our client to decide if it wants fixed layout.
> > +    if (d->m_webPageProxy->useFixedLayout())
> > +        d->m_webPageProxy->setViewportSize(size);
> 
> whether*. Anyway does that commetn add any value?

Not much, I can remove it.

> 
> > Source/WebKit2/UIProcess/API/qt/raw/qrawwebview.cpp:339
> > +        WebKit::LayerTreeCoordinatorProxy* coordinator = drawingArea->layerTreeCoordinatorProxy();
> > +        if (!coordinator)
> > +            return;
> > +        coordinator->setContentsSize(WebCore::FloatSize(size.width(), size.height()));
> 
> That could be written in two lines instead of 4

Than I need an else+return, so it would still be 4 lines :)
Comment 4 Kenneth Rohde Christiansen 2012-08-31 06:09:52 PDT
Comment on attachment 161665 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=161665&action=review

>>> Source/WebKit2/UIProcess/API/qt/raw/qrawwebview.cpp:339
>>> +        coordinator->setContentsSize(WebCore::FloatSize(size.width(), size.height()));
>> 
>> That could be written in two lines instead of 4
> 
> Than I need an else+return, so it would still be 4 lines :)

Ah you are right

else if (WebKit::LayerTreeCoordinatorProxy* coordinator = drawingArea->layerTreeCoordinatorProxy())
    coordinator->
else
    return;
Comment 5 Noam Rosenthal 2012-08-31 06:28:30 PDT
Comment on attachment 161665 [details]
Patch

Please remove comment before committing as Kenneth suggests.
Comment 6 Balazs Kelemen 2012-08-31 10:09:18 PDT
Landed in http://trac.webkit.org/changeset/127275