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

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

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

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

Patch Set 1 #

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

Messages

Total messages: 4 (0 generated)
Mads Ager (chromium)
Created Revert of Initial draft - modify ViewportAnchor to know about both inner and outer ...
6 years, 3 months ago (2014-09-24 10:27:56 UTC) #1
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/597113002/1
6 years, 3 months ago (2014-09-24 10:28:34 UTC) #2
commit-bot: I haz the power
Committed patchset #1 (id:1) as 182580
6 years, 3 months ago (2014-09-24 10:29:24 UTC) #3
Mads Ager (chromium)
6 years, 3 months ago (2014-09-24 11:14:21 UTC) #4
Message was sent while issue was closed.
A revert of this CL (patchset #1 id:1) has been created in
https://codereview.chromium.org/597683007/ by ager@chromium.org.

The reason for reverting is: Reapplying, reverting this one shouldn't be
necessary..

Powered by Google App Engine
This is Rietveld 408576698