New issue
Advanced search Search tips

Issue 773371 link

Starred by 1 user

Issue metadata

Status: Duplicate
Merged: issue 755405
Owner:
Closed: Oct 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

[root layer scrolls] mobile tests with small content get layout size 980x735

Project Member Reported by szager@chromium.org, Oct 10 2017

Issue description

If the page contents don't overflow 800x600, then the page's layout size should be 800x600, not the maximum constraint-derived size of 980x735.

Partial list of failing tests:

compositing/contents-opaque/body-background-painted.html
compositing/contents-opaque/layer-transform.html
compositing/contents-opaque/visibility-hidden.html
compositing/filters/sw-layer-overlaps-hw-shadow.html
compositing/filters/sw-nested-shadow-overlaps-hw-nested-shadow.html
compositing/filters/sw-shadow-overlaps-hw-layer.html
compositing/filters/sw-shadow-overlaps-hw-shadow.html
compositing/force-compositing-mode/force-composite-empty.html

 

Comment 1 by skobes@google.com, Oct 10 2017

Hmm, without a meta viewport I would expect us to use the fallback width of 980px, regardless of content.

Comment 2 by szager@chromium.org, Oct 12 2017

Even if the content is smaller than 800x600?  Below is a sample layer tree dump for a page that only has a 200x200 div.  It's weird that the scrolling contents layer is 980x735 but the main graphics layer is 800x600.

{
  "layers": [
    {
      "name": "LayoutView #document",
      "bounds": [800, 600],
      "backgroundColor": "#D3D3D3"
    },
    {
      "name": "Scrolling Layer",
      "bounds": [800, 600],
      "drawsContent": false
    },
    {
      "name": "Scrolling Contents Layer",
      "bounds": [980, 735],
      "contentsOpaque": true,
      "backgroundColor": "#D3D3D3"
    },
    {
      "name": "LayoutBlockFlow HTML",
      "bounds": [980, 735],
      "backgroundColor": "#D3D3D3"
    },
    {
      "name": "LayoutBlockFlow (positioned) DIV id='underbody'",
      "bounds": [200, 200],
      "contentsOpaque": true,
      "backgroundColor": "#008000",
      "transform": 1
    },
    {
      "name": "LayoutBlockFlow HTML (foreground) Layer",
      "bounds": [980, 735]
    }
  ],
  "transforms": [
    {
      "id": 1,
      "transform": [
        [1, 0, 0, 0],
        [0, 1, 0, 0],
        [0, 0, 1, 0],
        [8, 8, 0, 1]
      ]
    }
  ]
}

Comment 3 by skobes@chromium.org, Oct 12 2017

Isn't LocalFrameView::layout_size_ established before the content is laid out?  I think it will have width = 980 on mobile.  As a result, LayoutView::frame_rect_ (ICB) would be 980, and its LayoutOverflowRect() will be 980 as well (if there is no overflow).

But that layer tree looks wrong because I would expect the LayoutView's main GraphicsLayer and Scrolling Layer to also be 980, reflecting the size of the layout viewport.

However, these tests don't seem to be enabling mobile viewport settings.  Do you mean they're only failing on Android bots?

Comment 4 by szager@chromium.org, Oct 12 2017

layout_size_ is now set after the page content has been laid out, in LayoutView::UpdateAfterLayout.  If the page content is smaller than 800x600, I don't know why the ICB would be set to 980x735.

These tests are only failing on Android bots.

Comment 5 by szager@chromium.org, Oct 12 2017

For context, here is how the layout size gets set:

LayoutView::UpdateAfterLayout
 ChromeClientImpl::ResizeAfterLayout
  WebViewImpl::ResizeAfterLayout
   WebViewImpl::RefreshPageScaleFactorAfterLayout
    WebViewImpl::UpdatePageDefinedViewportConstraints
     WebViewImpl::UpdateMainFrameLayoutSize
      LocalFrameView::UpdateLayoutSize

Comment 6 by bokan@chromium.org, Oct 16 2017

I haven't succeeded running tests locally yet, due to issue 567947 presumably. However, opening one of the mentioned tests in a local Chrome build, the LayoutView and scrolling layers are also 980, as I'd exepect. So my guess is this works correctly but that we're taking the layer snapshot in the test before we do the "post layout resize". It may just be we have to fix the tests.

I'm still trying to get the tests to run but I might have to reimage my device which will take some time. It may be that we just have to fix the tests to delay the snapshot a bit.

Comment 7 by bokan@chromium.org, Oct 16 2017

Status: Started (was: Assigned)

Comment 8 by bokan@chromium.org, Oct 23 2017

Mergedinto: 755405
Status: Duplicate (was: Started)
As szager@ pointed out, this is likely related to issue 755405 rather than root-layer-scrolls.

Comment 9 by skobes@chromium.org, Oct 23 2017

Blocking: -417782

Sign in to add a comment