Changeset 141483 in webkit


Ignore:
Timestamp:
Jan 31, 2013 2:02:23 PM (11 years ago)
Author:
aelias@chromium.org
Message:

[chromium] Rework page scale factor limits initialization
https://bugs.webkit.org/show_bug.cgi?id=108446

Reviewed by James Robinson.

When loading a page with viewportEnabled, both the limits
specified by the viewport tag and the content width need to be
considered before we initialize the minimum page scale (and
pageScaleFactor itself usually to the same value). The timing has
proven tricky to get correct.

This patch simplifies the flow by computing the
limits only at the end of layouts and at no other time. In combination
with https://bugs.webkit.org/show_bug.cgi?id=107922 which sets
needsLayout() appropriately, this results in a more robust and easy
to understand sequence.

Fixes FixedLayoutInitializeAtMinimumPageScale test.

  • src/ChromeClientImpl.cpp:

(WebKit::ChromeClientImpl::dispatchViewportPropertiesDidChange):

  • src/WebViewImpl.cpp:

(WebKit::WebViewImpl::WebViewImpl):
(WebKit::WebViewImpl::resize):
(WebKit::WebViewImpl::setPageScaleFactorLimits):
(WebKit::WebViewImpl::computePageScaleFactorLimits):
(WebKit::WebViewImpl::layoutUpdated):
(WebKit::WebViewImpl::didChangeContentsSize):

  • src/WebViewImpl.h:

(WebKit::WebViewImpl::setInitialPageScaleFactor):
(WebViewImpl):

  • tests/WebFrameTest.cpp:
Location:
trunk/Source/WebKit/chromium
Files:
5 edited

Legend:

Unmodified
Added
Removed
  • trunk/Source/WebKit/chromium/ChangeLog

    r141479 r141483  
     12013-01-31  Alexandre Elias  <aelias@chromium.org>
     2
     3        [chromium] Rework page scale factor limits initialization
     4        https://bugs.webkit.org/show_bug.cgi?id=108446
     5
     6        Reviewed by James Robinson.
     7
     8        When loading a page with viewportEnabled, both the limits
     9        specified by the viewport tag and the content width need to be
     10        considered before we initialize the minimum page scale (and
     11        pageScaleFactor itself usually to the same value). The timing has
     12        proven tricky to get correct.
     13
     14        This patch simplifies the flow by computing the
     15        limits only at the end of layouts and at no other time. In combination
     16        with https://bugs.webkit.org/show_bug.cgi?id=107922 which sets
     17        needsLayout() appropriately, this results in a more robust and easy
     18        to understand sequence.
     19
     20        Fixes FixedLayoutInitializeAtMinimumPageScale test.
     21
     22        * src/ChromeClientImpl.cpp:
     23        (WebKit::ChromeClientImpl::dispatchViewportPropertiesDidChange):
     24        * src/WebViewImpl.cpp:
     25        (WebKit::WebViewImpl::WebViewImpl):
     26        (WebKit::WebViewImpl::resize):
     27        (WebKit::WebViewImpl::setPageScaleFactorLimits):
     28        (WebKit::WebViewImpl::computePageScaleFactorLimits):
     29        (WebKit::WebViewImpl::layoutUpdated):
     30        (WebKit::WebViewImpl::didChangeContentsSize):
     31        * src/WebViewImpl.h:
     32        (WebKit::WebViewImpl::setInitialPageScaleFactor):
     33        (WebViewImpl):
     34        * tests/WebFrameTest.cpp:
     35
    1362013-01-31  Aurimas Liutikas  <aurimas@chromium.org>
    237
  • trunk/Source/WebKit/chromium/src/ChromeClientImpl.cpp

    r140286 r141483  
    670670        computed.initialScale *= deviceScaleFactor;
    671671
    672     bool needInitializePageScale = !m_webView->isPageScaleFactorSet();
     672    m_webView->setInitialPageScaleFactor(computed.initialScale);
    673673    m_webView->setFixedLayoutSize(flooredIntSize(computed.layoutSize));
    674674    m_webView->setDeviceScaleFactor(deviceScaleFactor);
    675675    m_webView->setPageScaleFactorLimits(computed.minimumScale, computed.maximumScale);
    676     if (needInitializePageScale)
    677         m_webView->setPageScaleFactorPreservingScrollOffset(computed.initialScale);
    678676#endif
    679677}
  • trunk/Source/WebKit/chromium/src/WebViewImpl.cpp

    r141428 r141483  
    397397    , m_minimumPageScaleFactor(minPageScaleFactor)
    398398    , m_maximumPageScaleFactor(maxPageScaleFactor)
     399    , m_initialPageScaleFactor(-1)
    399400    , m_ignoreViewportTagMaximumScale(false)
    400401    , m_pageScaleFactorIsSet(false)
     
    16611662#if ENABLE(VIEWPORT)
    16621663    if (settings()->viewportEnabled()) {
    1663         // Relayout immediately to obtain the new content width, which is needed
    1664         // to calculate the minimum scale limit.
    1665         view->layout();
    1666         computePageScaleFactorLimits();
     1664        // Relayout immediately to recalculate the minimum scale limit.
     1665        if (view->needsLayout())
     1666            view->layout();
    16671667
    16681668        // When the device rotates:
     
    30483048void WebViewImpl::setPageScaleFactorLimits(float minPageScale, float maxPageScale)
    30493049{
     3050    if (minPageScale == m_pageDefinedMinimumPageScaleFactor && maxPageScale == m_pageDefinedMaximumPageScaleFactor)
     3051        return;
     3052
    30503053    m_pageDefinedMinimumPageScaleFactor = minPageScale;
    30513054    m_pageDefinedMaximumPageScaleFactor = maxPageScale;
    3052     computePageScaleFactorLimits();
     3055
     3056    if (settings()->viewportEnabled()) {
     3057        // If we're in viewport tag mode, we need to layout to obtain the latest
     3058        // contents size and compute the final limits.
     3059        FrameView* view = mainFrameImpl()->frameView();
     3060        if (view)
     3061            view->setNeedsLayout();
     3062    } else {
     3063        // Otherwise just compute the limits immediately.
     3064        computePageScaleFactorLimits();
     3065    }
    30533066}
    30543067
     
    30933106}
    30943107
    3095 bool WebViewImpl::computePageScaleFactorLimits()
     3108void WebViewImpl::computePageScaleFactorLimits()
    30963109{
    30973110    if (m_pageDefinedMinimumPageScaleFactor == -1 || m_pageDefinedMaximumPageScaleFactor == -1)
    3098         return false;
     3111        return;
    30993112
    31003113    if (!mainFrame() || !page() || !page()->mainFrame() || !page()->mainFrame()->view())
    3101         return false;
     3114        return;
    31023115
    31033116    FrameView* view = page()->mainFrame()->view();
     
    31053118    m_minimumPageScaleFactor = min(max(m_pageDefinedMinimumPageScaleFactor, minPageScaleFactor), maxPageScaleFactor);
    31063119    m_maximumPageScaleFactor = max(min(m_pageDefinedMaximumPageScaleFactor, maxPageScaleFactor), minPageScaleFactor);
    3107     if (!m_webSettings->applyDeviceScaleFactorInCompositor()) {
    3108         m_minimumPageScaleFactor *= deviceScaleFactor();
    3109         m_maximumPageScaleFactor *= deviceScaleFactor();
    3110     }
    3111 
    3112     int viewWidthNotIncludingScrollbars = m_size.width;
    3113     if (viewWidthNotIncludingScrollbars && view->verticalScrollbar() && !view->verticalScrollbar()->isOverlayScrollbar())
    3114         viewWidthNotIncludingScrollbars -= view->verticalScrollbar()->width();
    3115     int unscaledContentsWidth = contentsSize().width();
    3116     if (viewWidthNotIncludingScrollbars && unscaledContentsWidth) {
    3117         // Limit page scaling down to the document width.
    3118         m_minimumPageScaleFactor = max(m_minimumPageScaleFactor, static_cast<float>(viewWidthNotIncludingScrollbars) / unscaledContentsWidth);
     3120
     3121    if (settings()->viewportEnabled()) {
     3122        if (!contentsSize().width() || !m_size.width)
     3123            return;
     3124
     3125        // When viewport tag is enabled, the scale needed to see the full
     3126        // content width is the default minimum.
     3127        int viewWidthNotIncludingScrollbars = m_size.width;
     3128        if (view->verticalScrollbar() && !view->verticalScrollbar()->isOverlayScrollbar())
     3129            viewWidthNotIncludingScrollbars -= view->verticalScrollbar()->width();
     3130        m_minimumPageScaleFactor = max(m_minimumPageScaleFactor, static_cast<float>(viewWidthNotIncludingScrollbars) / contentsSize().width());
    31193131        m_maximumPageScaleFactor = max(m_minimumPageScaleFactor, m_maximumPageScaleFactor);
    31203132    }
    31213133    ASSERT(m_minimumPageScaleFactor <= m_maximumPageScaleFactor);
    31223134
    3123     float clampedScale = clampPageScaleFactorToLimits(pageScaleFactor());
     3135    // Initialize and/or clamp the page scale factor if needed.
     3136    float newPageScaleFactor = pageScaleFactor();
     3137    if (!m_pageScaleFactorIsSet && m_initialPageScaleFactor != -1) {
     3138        newPageScaleFactor = m_initialPageScaleFactor;
     3139        m_pageScaleFactorIsSet = true;
     3140    }
     3141    newPageScaleFactor = clampPageScaleFactorToLimits(newPageScaleFactor);
    31243142#if USE(ACCELERATED_COMPOSITING)
    31253143    if (m_layerTreeView)
    3126         m_layerTreeView->setPageScaleFactorAndLimits(clampedScale, m_minimumPageScaleFactor, m_maximumPageScaleFactor);
    3127 #endif
    3128     if (clampedScale != pageScaleFactor()) {
    3129         setPageScaleFactorPreservingScrollOffset(clampedScale);
    3130         return true;
    3131     }
    3132 
    3133     return false;
     3144        m_layerTreeView->setPageScaleFactorAndLimits(newPageScaleFactor, m_minimumPageScaleFactor, m_maximumPageScaleFactor);
     3145#endif
     3146    if (newPageScaleFactor != pageScaleFactor())
     3147        setPageScaleFactorPreservingScrollOffset(newPageScaleFactor);
    31343148}
    31353149
     
    37433757    }
    37443758
     3759    if (settings()->viewportEnabled()) {
     3760        if (!isPageScaleFactorSet()) {
     3761            // If the viewport tag failed to be processed earlier, we need
     3762            // to recompute it now.
     3763            ViewportArguments viewportArguments = mainFrameImpl()->frame()->document()->viewportArguments();
     3764            m_page->chrome()->client()->dispatchViewportPropertiesDidChange(viewportArguments);
     3765        }
     3766
     3767        // Contents size is an input to the page scale limits, so a good time to
     3768        // recalculate is after layout has occurred.
     3769        computePageScaleFactorLimits();
     3770
     3771        // Relayout immediately to avoid violating the rule that needsLayout()
     3772        // isn't set at the end of a layout.
     3773        FrameView* view = mainFrameImpl()->frameView();
     3774        if (view && view->needsLayout())
     3775            view->layout();
     3776    }
     3777
    37453778    m_client->didUpdateLayout();
     3779
    37463780}
    37473781
    37483782void WebViewImpl::didChangeContentsSize()
    37493783{
    3750 #if ENABLE(VIEWPORT)
    3751     if (!settings()->viewportEnabled() || !mainFrameImpl())
    3752         return;
    3753 
    3754     bool mayNeedLayout = false;
    3755     if (!isPageScaleFactorSet()) {
    3756         // If the viewport tag failed to be processed earlier, we need
    3757         // to recompute it now.
    3758         ViewportArguments viewportArguments = mainFrameImpl()->frame()->document()->viewportArguments();
    3759         m_page->chrome()->client()->dispatchViewportPropertiesDidChange(viewportArguments);
    3760         mayNeedLayout = true;
    3761     } else
    3762         mayNeedLayout = computePageScaleFactorLimits();
    3763 
    3764     // didChangeContentsSize() may be called from FrameView::layout; we need to
    3765     // relayout to avoid triggering the assertion that needsLayout() isn't set
    3766     // at the end of a layout.
    3767     FrameView* view = mainFrameImpl()->frameView();
    3768     if (mayNeedLayout && view && view->needsLayout())
    3769         view->layout();
    3770 #endif
    37713784}
    37723785
  • trunk/Source/WebKit/chromium/src/WebViewImpl.h

    r141428 r141483  
    482482    }
    483483
     484    void setInitialPageScaleFactor(float initialPageScaleFactor) { m_initialPageScaleFactor = initialPageScaleFactor; }
    484485    bool ignoreViewportTagMaximumScale() const { return m_ignoreViewportTagMaximumScale; }
    485486
     
    613614
    614615private:
    615     bool computePageScaleFactorLimits();
     616    void computePageScaleFactorLimits();
    616617    float clampPageScaleFactorToLimits(float scale);
    617618    WebCore::IntPoint clampOffsetAtScale(const WebCore::IntPoint& offset, float scale) const;
     
    753754    float m_minimumPageScaleFactor;
    754755    float m_maximumPageScaleFactor;
     756    float m_initialPageScaleFactor;
    755757    bool m_ignoreViewportTagMaximumScale;
    756758    bool m_pageScaleFactorIsSet;
  • trunk/Source/WebKit/chromium/tests/WebFrameTest.cpp

    r141454 r141483  
    287287}
    288288
    289 // Test is disabled because it started failing after r140025.
    290 // See bug for details: webkit.org/b/107206.
    291 TEST_F(WebFrameTest, DISABLED_FixedLayoutInitializeAtMinimumPageScale)
     289TEST_F(WebFrameTest, FixedLayoutInitializeAtMinimumPageScale)
    292290{
    293291    registerMockedHttpURLLoad("fixed_layout.html");
     
    301299    // only becomes available after the load begins.
    302300    WebViewImpl* webViewImpl = static_cast<WebViewImpl*>(FrameTestHelpers::createWebViewAndLoad(m_baseURL + "fixed_layout.html", true, 0, &client));
     301    webViewImpl->settings()->setApplyDeviceScaleFactorInCompositor(true);
     302    webViewImpl->settings()->setApplyPageScaleFactorInCompositor(true);
    303303    webViewImpl->enableFixedLayoutMode(true);
    304304    webViewImpl->settings()->setViewportEnabled(true);
     
    388388}
    389389
    390 // Disabled, pending investigation after http://trac.webkit.org/changeset/141053
    391 TEST_F(WebFrameTest, DISABLED_pageScaleFactorShrinksViewport)
     390TEST_F(WebFrameTest, pageScaleFactorShrinksViewport)
    392391{
    393392    registerMockedHttpURLLoad("fixed_layout.html");
     
    397396    int viewportWidth = 640;
    398397    int viewportHeight = 480;
    399     int viewportWidthMinusScrollbar = 640 - 15;
    400     int viewportHeightMinusScrollbar = 480 - 15;
    401398
    402399    WebViewImpl* webViewImpl = static_cast<WebViewImpl*>(FrameTestHelpers::createWebViewAndLoad(m_baseURL + "fixed_layout.html", true, 0, &client));
     
    408405    webViewImpl->layout();
    409406
     407    WebCore::FrameView* view = webViewImpl->mainFrameImpl()->frameView();
     408    int viewportWidthMinusScrollbar = 640 - (view->verticalScrollbar()->isOverlayScrollbar() ? 0 : 15);
     409    int viewportHeightMinusScrollbar = 480 - (view->horizontalScrollbar()->isOverlayScrollbar() ? 0 : 15);
     410
    410411    webViewImpl->setPageScaleFactor(2, WebPoint());
    411412
    412     WebCore::IntSize unscaledSize = webViewImpl->mainFrameImpl()->frameView()->unscaledVisibleContentSize(true);
     413    WebCore::IntSize unscaledSize = view->unscaledVisibleContentSize(true);
    413414    EXPECT_EQ(viewportWidth, unscaledSize.width());
    414415    EXPECT_EQ(viewportHeight, unscaledSize.height());
    415416
    416     WebCore::IntSize unscaledSizeMinusScrollbar = webViewImpl->mainFrameImpl()->frameView()->unscaledVisibleContentSize(false);
     417    WebCore::IntSize unscaledSizeMinusScrollbar = view->unscaledVisibleContentSize(false);
    417418    EXPECT_EQ(viewportWidthMinusScrollbar, unscaledSizeMinusScrollbar.width());
    418419    EXPECT_EQ(viewportHeightMinusScrollbar, unscaledSizeMinusScrollbar.height());
    419420
    420     WebCore::IntSize scaledSize = webViewImpl->mainFrameImpl()->frameView()->visibleContentRect().size();
     421    WebCore::IntSize scaledSize = view->visibleContentRect().size();
    421422    EXPECT_EQ(ceil(viewportWidthMinusScrollbar / 2.0), scaledSize.width());
    422423    EXPECT_EQ(ceil(viewportHeightMinusScrollbar / 2.0), scaledSize.height());
     
    458459
    459460    WebViewImpl* webViewImpl = static_cast<WebViewImpl*>(FrameTestHelpers::createWebViewAndLoad(m_baseURL + "no_scale_for_you.html", true, 0, &client));
     461    webViewImpl->settings()->setApplyDeviceScaleFactorInCompositor(true);
     462    webViewImpl->settings()->setApplyPageScaleFactorInCompositor(true);
    460463    webViewImpl->enableFixedLayoutMode(true);
    461464    webViewImpl->settings()->setViewportEnabled(true);
     
    465468
    466469    webViewImpl->setIgnoreViewportTagMaximumScale(true);
     470    webViewImpl->layout();
    467471
    468472    EXPECT_EQ(4.0f, webViewImpl->maximumPageScaleFactor());
     
    536540void simulateDoubleTap(WebViewImpl* webViewImpl, WebPoint& point, float& scale)
    537541{
    538     WebPoint scaledPoint(static_cast<int>(point.x * webViewImpl->pageScaleFactor()),
    539         static_cast<int>(point.y * webViewImpl->pageScaleFactor()));
    540     webViewImpl->animateZoomAroundPoint(scaledPoint, WebViewImpl::DoubleTap);
     542    webViewImpl->animateZoomAroundPoint(point, WebViewImpl::DoubleTap);
    541543    webViewImpl->mainFrameImpl()->frameView()->layout();
    542544    scale = webViewImpl->pageScaleFactor();
     
    611613    // minimumPageScale < doubleTapZoomAlreadyLegibleScale < 1
    612614    webView->setPageScaleFactorLimits(0.5f, 4);
     615    webView->layout();
    613616    float doubleTapZoomAlreadyLegibleScale = webViewImpl->minimumPageScaleFactor() * doubleTapZoomAlreadyLegibleRatio;
    614617    setScaleAndScrollAndLayout(webViewImpl, WebPoint(0, 0), (webViewImpl->minimumPageScaleFactor()) * (1 + doubleTapZoomAlreadyLegibleRatio) / 2);
     
    624627    // 1 < minimumPageScale < doubleTapZoomAlreadyLegibleScale
    625628    webView->setPageScaleFactorLimits(1.1f, 4);
     629    webView->layout();
    626630    doubleTapZoomAlreadyLegibleScale = webViewImpl->minimumPageScaleFactor() * doubleTapZoomAlreadyLegibleRatio;
    627631    setScaleAndScrollAndLayout(webViewImpl, WebPoint(0, 0), (webViewImpl->minimumPageScaleFactor()) * (1 + doubleTapZoomAlreadyLegibleRatio) / 2);
     
    637641    // minimumPageScale < 1 < doubleTapZoomAlreadyLegibleScale
    638642    webView->setPageScaleFactorLimits(0.95f, 4);
     643    webView->layout();
    639644    doubleTapZoomAlreadyLegibleScale = webViewImpl->minimumPageScaleFactor() * doubleTapZoomAlreadyLegibleRatio;
    640645    setScaleAndScrollAndLayout(webViewImpl, WebPoint(0, 0), (webViewImpl->minimumPageScaleFactor()) * (1 + doubleTapZoomAlreadyLegibleRatio) / 2);
     
    675680    float legibleScale = textAutosizingFontScaleFactor;
    676681    setScaleAndScrollAndLayout(webViewImpl, WebPoint(0, 0), (webViewImpl->minimumPageScaleFactor()) * (1 + doubleTapZoomAlreadyLegibleRatio) / 2);
     682    float doubleTapZoomAlreadyLegibleScale = webViewImpl->minimumPageScaleFactor() * doubleTapZoomAlreadyLegibleRatio;
    677683    webView->setPageScaleFactorLimits(0.5f, 4);
    678     float doubleTapZoomAlreadyLegibleScale = webViewImpl->minimumPageScaleFactor() * doubleTapZoomAlreadyLegibleRatio;
     684    webView->layout();
    679685    simulateDoubleTap(webViewImpl, doubleTapPoint, scale);
    680686    EXPECT_FLOAT_EQ(legibleScale, scale);
     
    688694    // 1 < textAutosizingFontScaleFactor < minimumPageScale < doubleTapZoomAlreadyLegibleScale
    689695    webView->setPageScaleFactorLimits(1.0f, 4);
     696    webView->layout();
    690697    doubleTapZoomAlreadyLegibleScale = webViewImpl->minimumPageScaleFactor() * doubleTapZoomAlreadyLegibleRatio;
    691698    setScaleAndScrollAndLayout(webViewImpl, WebPoint(0, 0), (webViewImpl->minimumPageScaleFactor()) * (1 + doubleTapZoomAlreadyLegibleRatio) / 2);
     
    701708    // minimumPageScale < 1 < textAutosizingFontScaleFactor < doubleTapZoomAlreadyLegibleScale
    702709    webView->setPageScaleFactorLimits(0.95f, 4);
     710    webView->layout();
    703711    doubleTapZoomAlreadyLegibleScale = webViewImpl->minimumPageScaleFactor() * doubleTapZoomAlreadyLegibleRatio;
    704712    setScaleAndScrollAndLayout(webViewImpl, WebPoint(0, 0), (webViewImpl->minimumPageScaleFactor()) * (1 + doubleTapZoomAlreadyLegibleRatio) / 2);
     
    714722    // minimumPageScale < 1 < doubleTapZoomAlreadyLegibleScale < textAutosizingFontScaleFactor
    715723    webView->setPageScaleFactorLimits(0.9f, 4);
     724    webView->layout();
    716725    doubleTapZoomAlreadyLegibleScale = webViewImpl->minimumPageScaleFactor() * doubleTapZoomAlreadyLegibleRatio;
    717726    setScaleAndScrollAndLayout(webViewImpl, WebPoint(0, 0), (webViewImpl->minimumPageScaleFactor()) * (1 + doubleTapZoomAlreadyLegibleRatio) / 2);
Note: See TracChangeset for help on using the changeset viewer.