Commit eb643a6b authored by David Bokan's avatar David Bokan Committed by Commit Bot

Revert "Improve rotation check on viewport resize"

This reverts commit 432553b5.

Reason for revert: width == height isn't enough since the system bar affects only height. Broke 887064, 887330, 887899

Original change's description:
> Improve rotation check on viewport resize
>
> Since rotation used to be supported only on Android, where windows can't
> be resized, we made the assumption that if the width changes it must be
> a rotation.
>
> This changes in ChromeOS where entering tablet mode enables rotation but
> also resizes the window (when entering tablet mode). This caused
> inappropriate rotation anchoring.
>
> The underlying issue has been fixed in other patches linked to this bug,
> the rotation trigger should be improved to check that the width and
> height are swapped.
>
> Bug: 831473
> Change-Id: I0acd39d16319d8cb7819faac4c4ce54b6f7a2e46
> Reviewed-on: https://chromium-review.googlesource.com/1224711
> Reviewed-by: Dave Tapuska <dtapuska@chromium.org>
> Commit-Queue: David Bokan <bokan@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#591430}

TBR=bokan@chromium.org,dtapuska@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 831473,887899,887064,887330
Change-Id: Ib0f3cad366a80506effc62fcc352e1c75f2fcd55
Reviewed-on: https://chromium-review.googlesource.com/1239726
Commit-Queue: David Bokan <bokan@chromium.org>
Reviewed-by: default avatarDavid Bokan <bokan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#593536}
parent 8d006e4a
......@@ -1515,8 +1515,7 @@ void WebViewImpl::ResizeWithBrowserControls(
bool is_rotation =
GetPage()->GetSettings().GetMainFrameResizesAreOrientationChanges() &&
size_.width && ContentsSize().Width() && new_size.width == size_.height &&
new_size.height == size_.width &&
size_.width && ContentsSize().Width() && new_size.width != size_.width &&
!fullscreen_controller_->IsFullscreenOrTransitioning();
size_ = new_size;
......
......@@ -25,9 +25,7 @@ class RotationViewportAnchorTest : public SimTest {
}
};
// This tests that the rotation anchor doesn't make any changes to scroll
// when nothing on the page changes.
TEST_F(RotationViewportAnchorTest, SimpleAbsolutePositionNoOpRotation) {
TEST_F(RotationViewportAnchorTest, SimpleAbsolutePosition) {
WebView().Resize(WebSize(400, 600));
SimRequest request("https://example.com/test.html", "text/html");
LoadURL("https://example.com/test.html");
......@@ -119,60 +117,6 @@ TEST_F(RotationViewportAnchorTest, PositionRelativeToViewportSize) {
EXPECT_EQ(expected_offset.Y(), layout_viewport->GetScrollOffset().Height());
}
// Ensure that only a size change that looks like a rotation (i.e. width and
// height are swapped) causes the rotation viewport anchoring behavior. Other
// resizes don't cause anchoring.
TEST_F(RotationViewportAnchorTest, OnlyRotationEngagesAnchor) {
WebView().Resize(WebSize(100, 600));
SimRequest request("https://example.com/test.html", "text/html");
LoadURL("https://example.com/test.html");
request.Complete(R"HTML(
<!DOCTYPE html>
<style>
body {
width: 10000px;
height: 10000px;
margin: 0px;
}
#target {
width: 50px;
height: 50px;
position: absolute;
left: 500%;
top: 500%;
}
</style>
<div id="target"></div>
)HTML");
Compositor().BeginFrame();
Document& document = GetDocument();
ScrollableArea* layout_viewport = document.View()->LayoutViewport();
IntPoint target_position(5 * WebView().Size().width,
5 * WebView().Size().height);
// Place the target at the top-center of the viewport. This is where the
// rotation anchor finds the node to anchor to.
layout_viewport->SetScrollOffset(
ScrollOffset(target_position.X() - WebView().Size().width / 2 + 25,
target_position.Y()),
kProgrammaticScroll);
ScrollOffset expected_offset = layout_viewport->GetScrollOffset();
// Change just the width - we shouldn't consider this a rotation so the
// expectation is that the scroll offset won't change.
WebView().Resize(WebSize(600, 600));
Compositor().BeginFrame();
EXPECT_EQ(expected_offset.Width(),
layout_viewport->GetScrollOffset().Width());
EXPECT_EQ(expected_offset.Height(),
layout_viewport->GetScrollOffset().Height());
}
} // namespace
} // namespace blink
......@@ -98,12 +98,7 @@ class CORE_EXPORT ImageDocument final : public HTMLDocument {
ShrinkToFitMode shrink_to_fit_mode_;
FRIEND_TEST_ALL_PREFIXES(ImageDocumentViewportTest, ZoomForDSFScaleImage);
FRIEND_TEST_ALL_PREFIXES(ImageDocumentViewportTest,
DivWidthWithZoomForDSFSmallerThanView);
FRIEND_TEST_ALL_PREFIXES(ImageDocumentViewportTest,
DivWidthWithZoomForDSFLargerThanView);
FRIEND_TEST_ALL_PREFIXES(ImageDocumentViewportTest,
DivWidthWithZoomForDSFMuchLargerThanView);
FRIEND_TEST_ALL_PREFIXES(ImageDocumentViewportTest, DivWidthWithZoomForDSF);
};
DEFINE_DOCUMENT_TYPE_CASTS(ImageDocument);
......
......@@ -348,15 +348,11 @@ TEST_F(ImageDocumentViewportTest, ZoomForDSFScaleImage) {
// Tests that with zoom factor for device scale factor, image with different
// size fit in the viewport correctly.
TEST_F(ImageDocumentViewportTest, DivWidthWithZoomForDSFSmallerThanView) {
TEST_F(ImageDocumentViewportTest, DivWidthWithZoomForDSF) {
v8::HandleScope handle_scope(v8::Isolate::GetCurrent());
SimRequest request("https://example.com/test.jpg", "image/jpeg");
LoadURL("https://example.com/test.jpg");
// Viewport is 100px (px == CSS pixels) width and height.
WebView().SetZoomFactorForDeviceScaleFactor(2.f);
WebView().Resize(IntSize(200, 200));
Vector<unsigned char> jpeg = JpegImage();
Vector<char> data = Vector<char>();
data.Append(jpeg.data(), jpeg.size());
......@@ -364,8 +360,11 @@ TEST_F(ImageDocumentViewportTest, DivWidthWithZoomForDSFSmallerThanView) {
HTMLImageElement* img = GetDocument().ImageElement();
WebView().SetZoomFactorForDeviceScaleFactor(2.f);
// Image smaller then webview size, visual viewport is not zoomed, and image
// will be centered in the viewport.
WebView().Resize(IntSize(200, 200));
Compositor().BeginFrame();
EXPECT_EQ(50u, img->width());
EXPECT_EQ(50u, img->height());
......@@ -376,25 +375,10 @@ TEST_F(ImageDocumentViewportTest, DivWidthWithZoomForDSFSmallerThanView) {
DOMRect* rect = img->getBoundingClientRect();
EXPECT_EQ(25, rect->x());
EXPECT_EQ(25, rect->y());
}
TEST_F(ImageDocumentViewportTest, DivWidthWithZoomForDSFLargerThanView) {
v8::HandleScope handle_scope(v8::Isolate::GetCurrent());
SimRequest request("https://example.com/test.jpg", "image/jpeg");
LoadURL("https://example.com/test.jpg");
WebView().SetZoomFactorForDeviceScaleFactor(2.f);
WebView().Resize(IntSize(50, 50));
Vector<unsigned char> jpeg = JpegImage();
Vector<char> data = Vector<char>();
data.Append(jpeg.data(), jpeg.size());
request.Complete(data);
HTMLImageElement* img = GetDocument().ImageElement();
// Image wider than webview size, image should fill the visual viewport, and
// visual viewport zoom out to 0.5.
WebView().Resize(IntSize(50, 50));
Compositor().BeginFrame();
EXPECT_EQ(50u, img->width());
EXPECT_EQ(50u, img->height());
......@@ -402,27 +386,10 @@ TEST_F(ImageDocumentViewportTest, DivWidthWithZoomForDSFLargerThanView) {
EXPECT_EQ(0.5f, GetVisualViewport().Scale());
EXPECT_EQ(50, GetVisualViewport().Width());
EXPECT_EQ(50, GetVisualViewport().Height());
}
TEST_F(ImageDocumentViewportTest, DivWidthWithZoomForDSFMuchLargerThanView) {
v8::HandleScope handle_scope(v8::Isolate::GetCurrent());
SimRequest request("https://example.com/test.jpg", "image/jpeg");
LoadURL("https://example.com/test.jpg");
WebView().SetZoomFactorForDeviceScaleFactor(2.f);
WebView().Resize(IntSize(4, 20));
Vector<unsigned char> jpeg = JpegImage();
Vector<char> data = Vector<char>();
data.Append(jpeg.data(), jpeg.size());
request.Complete(data);
HTMLImageElement* img = GetDocument().ImageElement();
// Image wider than webview size, image should fill the visual viewport, and
// visual viewport zoom out to 0.5.
// When image is more than 10X wider than webview, shrink the image to fit the
// width of the screen.
WebView().Resize(IntSize(4, 20));
Compositor().BeginFrame();
EXPECT_EQ(20u, img->width());
EXPECT_EQ(20u, img->height());
......@@ -430,7 +397,7 @@ TEST_F(ImageDocumentViewportTest, DivWidthWithZoomForDSFMuchLargerThanView) {
EXPECT_EQ(0.1f, GetVisualViewport().Scale());
EXPECT_EQ(20, GetVisualViewport().Width());
EXPECT_EQ(100, GetVisualViewport().Height());
DOMRect* rect = img->getBoundingClientRect();
rect = img->getBoundingClientRect();
EXPECT_EQ(0, rect->x());
EXPECT_EQ(40, rect->y());
}
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment