Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(117)

Issue 597683007: Revert of Revert of Initial draft - modify ViewportAnchor to know about both inner and outer viewports. (Closed)

Created:
6 years, 2 months ago by Mads Ager (chromium)
Modified:
6 years, 2 months ago
Base URL:
https://chromium.googlesource.com/chromium/blink.git@master
Project:
blink
Visibility:
Public.

Description

Revert of Revert of Initial draft - modify ViewportAnchor to know about both inner and outer viewports. (patchset #1 id:1 of https://codereview.chromium.org/597113002/) Reason for revert: Reapplying, reverting this one shouldn't be necessary. Original issue's description: > Revert of Initial draft - modify ViewportAnchor to know about both inner and outer viewports. (patchset #11 id:200001 of https://codereview.chromium.org/556703005/) > > Reason for revert: > The new test is failing on mac. Reverting for now so it can be investigated. > > [ FAILED ] PinchViewportTest.TestResizeAfterHorizontalScroll (39 ms) > [1471/1471] PinchViewportTest.TestResizeAfterHorizontalScroll (39 ms) > Retrying 2 tests (retry #3) > [ RUN ] PinchViewportTest.TestResizeAfterVerticalScroll > ../../third_party/WebKit/Source/web/tests/PinchViewportTest.cpp:251: Failure > Value of: (pinchViewport.visibleRect().size()).width() > Actual: 45.94595 > Expected: (FloatSize(50, 25)).width() > Which is: 50 > ../../third_party/WebKit/Source/web/tests/PinchViewportTest.cpp:251: Failure > Value of: (pinchViewport.visibleRect().size()).height() > Actual: 22.972975 > Expected: (FloatSize(50, 25)).height() > Which is: 25 > ../../third_party/WebKit/Source/web/tests/PinchViewportTest.cpp:253: Failure > Value of: (frame()->view()->scrollPosition()).y() > Actual: 638 > Expected: (IntPoint(0, 625)).y() > Which is: 625 > ../../third_party/WebKit/Source/web/tests/PinchViewportTest.cpp:254: Failure > Value of: (pinchViewport.location()).y() > Actual: 62 > Expected: (FloatPoint(0, 75)).y() > Which is: 75 > > Original issue's description: > > Fix pinch virtual viewport position after resize. > > > > Associate the viewport anchor with the inner viewport. > > Adjust the inner and outer viewport positions after resize > > such that they both remain in their allowed range and the > > inner viewport origin scales proportionally within the > > outer viewport. > > > > As a small cleanup, made the method ScrollView::scrollTo() protected. > > > > BUG=364108 > > > > Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=182365 > > TBR=leviw@chromium.org,aelias@chromium.org,bokan@chromium.org,timav@google.com,timav@chromium.org > NOTREECHECKS=true > NOTRY=true > BUG=364108 > > Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=182580 TBR=leviw@chromium.org,aelias@chromium.org,bokan@chromium.org,timav@google.com,timav@chromium.org NOTREECHECKS=true NOTRY=true BUG=364108 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=182586

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+310 lines, -29 lines) Patch
M Source/core/frame/PinchViewport.h View 1 chunk +1 line, -0 lines 0 comments Download
M Source/platform/scroll/ScrollView.h View 2 chunks +3 lines, -3 lines 0 comments Download
M Source/web/ViewportAnchor.h View 2 chunks +15 lines, -5 lines 0 comments Download
M Source/web/ViewportAnchor.cpp View 4 chunks +86 lines, -15 lines 0 comments Download
M Source/web/WebViewImpl.h View 1 chunk +4 lines, -0 lines 0 comments Download
M Source/web/WebViewImpl.cpp View 4 chunks +55 lines, -6 lines 0 comments Download
M Source/web/tests/PinchViewportTest.cpp View 2 chunks +120 lines, -0 lines 0 comments Download
A Source/web/tests/data/200-by-800-viewport.html View 1 chunk +26 lines, -0 lines 0 comments Download

Messages

Total messages: 2 (0 generated)
Mads Ager (chromium)
Created Revert of Revert of Initial draft - modify ViewportAnchor to know about both inner ...
6 years, 2 months ago (2014-09-24 11:14:22 UTC) #1
Mads Ager (chromium)
6 years, 2 months ago (2014-09-24 11:49:33 UTC) #2
Message was sent while issue was closed.
A revert of this CL (patchset #1 id:1) has been created in
https://codereview.chromium.org/601633003/ by ager@chromium.org.

The reason for reverting is: Nope: I did have to revert this one.
webkit_unit_test failures on mac are back..

Powered by Google App Engine
This is Rietveld 408576698