Changeset 152433 in webkit


Ignore:
Timestamp:
Jul 5, 2013 7:19:41 PM (11 years ago)
Author:
timothy_horton@apple.com
Message:

Fix r152265: FrameView's pagination mode is only one of two, and the logic was totally wrong
​https://bugs.webkit.org/show_bug.cgi?id=118439
<rdar://problem/14366120>

Reviewed by Anders Carlsson.

  • page/FrameView.cpp:

(WebCore::FrameView::maximumScrollPosition):

  • Test both Page and FrameView's pagination modes.
  • *Don't* clamp negatives to zero if we are paginated with scroll offset set,

like the original changelog claimed, but the code did the opposite.

  • Don't clamp in *all* pagination modes; with 'direction: RTL' it's still possible

to have a negative maximum position in LTR/TB pagination modes.

  • TestWebKitAPI/Tests/WebKit2/ResizeReversePaginatedWebView.cpp:

(TestWebKitAPI::didLayout):
(TestWebKitAPI::TEST):

  • Make use of EXPECT_JS_EQ instead of manually doing JavaScript stuff.
  • Assert that we got the right number of pages for sanity.
  • Reduce the page gap size so that DrawingAreaImpl doesn't try to allocate

so much memory that SharedMemory asserts and makes the test time out.

  • Use didFirstLayoutAfterSuppressedIncrementalRendering instead of Paint

because paint doesn't fire if the window is offscreen.

Location:
trunk
Files:
4 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r152430 r152433  
     12013-07-05  Tim Horton  <timothy_horton@apple.com>
     2
     3        Fix r152265: FrameView's pagination mode is only one of two, and the logic was totally wrong
     4        https://bugs.webkit.org/show_bug.cgi?id=118439
     5        <rdar://problem/14366120>
     6
     7        Reviewed by Anders Carlsson.
     8
     9        * page/FrameView.cpp:
     10        (WebCore::FrameView::maximumScrollPosition):
     11        - Test both Page and FrameView's pagination modes.
     12        - *Don't* clamp negatives to zero if we are paginated with scroll offset set,
     13            like the original changelog claimed, but the code did the opposite.
     14        - Don't clamp in *all* pagination modes; with 'direction: RTL' it's still possible
     15            to have a negative maximum position in LTR/TB pagination modes.
     16
    1172013-07-05  Sam Weinig  <sam@webkit.org>
    218

  • trunk/Source/WebCore/page/FrameView.cpp

    r152425 r152433  
    16541654    IntPoint maximumOffset(contentsWidth() - visibleWidth() - scrollOrigin().x(), totalContentsSize().height() - visibleHeight() - scrollOrigin().y());
    16551655
    1656     // With reverse pagination modes, we can have a negative maximum scroll position.
    1657     if (m_pagination.mode == Pagination::BottomToTopPaginated
    1658         || m_pagination.mode == Pagination::RightToLeftPaginated
     1656    // With pagination enabled, we can have a negative maximum scroll position.
     1657    if ((m_frame->page()->pagination().mode == Pagination::Unpaginated && m_pagination.mode == Pagination::Unpaginated)
    16591658        || scrollOrigin() == IntPoint::zero())
    16601659        maximumOffset.clampNegativeToZero();

  • trunk/Tools/ChangeLog

    r152425 r152433  
     12013-07-05  Tim Horton  <timothy_horton@apple.com>
     2
     3        Fix r152265: FrameView's pagination mode is only one of two, and the logic was totally wrong
     4        https://bugs.webkit.org/show_bug.cgi?id=118439
     5        <rdar://problem/14366120>
     6
     7        Reviewed by Anders Carlsson.
     8
     9        * TestWebKitAPI/Tests/WebKit2/ResizeReversePaginatedWebView.cpp:
     10        (TestWebKitAPI::didLayout):
     11        (TestWebKitAPI::TEST):
     12        - Make use of EXPECT_JS_EQ instead of manually doing JavaScript stuff.
     13        - Assert that we got the right number of pages for sanity.
     14        - Reduce the page gap size so that DrawingAreaImpl doesn't try to allocate
     15            so much memory that SharedMemory asserts and makes the test time out.
     16        - Use didFirstLayoutAfterSuppressedIncrementalRendering instead of Paint
     17            because paint doesn't fire if the window is offscreen.
     18
    1192013-07-05  Tim Horton  <timothy_horton@apple.com>
    220

  • trunk/Tools/TestWebKitAPI/Tests/WebKit2/ResizeReversePaginatedWebView.cpp

    r152265 r152433  
    2525
    2626#include "config.h"
     27#include "JavaScriptTest.h"
    2728#include "PlatformUtilities.h"
    2829#include "PlatformWebView.h"
    … …  
    3839
    3940static const unsigned pageLength = 100;
    40 static const unsigned pageGap = 1000;
    41 
    42 static void didRunJavaScript(WKSerializedScriptValueRef resultSerializedScriptValue, WKErrorRef error, void* context)
    43 {
    44     JSGlobalContextRef scriptContext = JSGlobalContextCreate(0);
    45     ASSERT_NOT_NULL(scriptContext);
    46 
    47     ASSERT_NOT_NULL(resultSerializedScriptValue);
    48     JSValueRef returnValue = WKSerializedScriptValueDeserialize(resultSerializedScriptValue, scriptContext, 0);
    49     double scrollPosition = JSValueToNumber(scriptContext, returnValue, 0);
    50 
    51     ASSERT_EQ(-20900.0, scrollPosition);
    52 
    53     testDone = true;
    54 }
     41static const unsigned pageGap = 100;
     42static const unsigned expectedPageCount = 20;
    5543
    5644static void didLayout(WKPageRef page, WKLayoutMilestones milestones, WKTypeRef, const void* clientInfo)
    5745{
    58     if (milestones & kWKDidFirstPaintAfterSuppressedIncrementalRendering) {
     46    if (milestones & kWKDidFirstLayoutAfterSuppressedIncrementalRendering) {
    5947        PlatformWebView* webView = (PlatformWebView*)clientInfo;
     48
    6049        unsigned pageCount = WKPageGetPageCount(page);
     50        EXPECT_EQ(expectedPageCount, pageCount);
     51
    6152        webView->resizeTo((pageLength * pageCount) + (pageGap * (pageCount - 1)), 500);
     53        EXPECT_JS_EQ(page, "window.scrollX", "-3800");
    6254
    63         WKRetainPtr<WKStringRef> javaScriptString(AdoptWK, WKStringCreateWithUTF8CString("window.scrollX"));
    64         WKPageRunJavaScriptInMainFrame(page, javaScriptString.get(), 0, didRunJavaScript);
     55        testDone = true;
    6556    }
    6657}
    … …  
    7970    WKPageSetPageLoaderClient(webView.page(), &loaderClient);
    8071
    81     WKPageListenForLayoutMilestones(webView.page(), kWKDidFirstPaintAfterSuppressedIncrementalRendering);
     72    WKPageListenForLayoutMilestones(webView.page(), kWKDidFirstLayoutAfterSuppressedIncrementalRendering);
    8273
    8374    WKPageGroupRef pageGroup =  WKPageGetPageGroup(webView.page());
Note: See TracChangeset for help on using the changeset viewer.