bokan@ can you check how VisualViewport::setScale() affects pixel dump of layout tests?

We do reset scale to 1 in Internals::resetToConsistentState().

I've seen this before when looking into a different test...or perhaps is was this exact one. I couldn't figure out what was going on then. I may take a closer look but I probably wont have the time to do so in the next week or two.

Added some log and found that the restoration of setScale() doesn't restore the offset (leaving 400,300 for the next test).

Some findings, but no actual progress yet.

- Save the attached fixed-scroll-simple.html into LayoutTests/paint/invalidation;
- run-webkit-tests --child-processes=1 paint/invalidation/fixed-right-bottom-in-page-scale.html paint/invalidation/fixed-scroll-simple.html

The test produced pixel result like the attached png file. It seems that the viewport still had an offset of (
The pixel result of the test shows that the viewport has an offset of (400, 300).

I tried to add the following in Internals::resetToConsistentState():

    page->frameHost().visualViewport().setLocation(FloatPoint());

and it did set the visual viewport's location to 0, 0, but this didn't fix the issue. The pixel result still has an offset.

bokan@ do you have any suggestions to debug/fix the issue?

Deleted: fixed-scroll-simple.html 1.1 KB

fixed-scroll-simple.html 1.1 KB View Download

Deleted: fixed-scroll-simple-actual.png 2.8 KB

	fixed-scroll-simple-actual.png 2.8 KB View Download

Bisected to:
https://codereview.chromium.org/1736073002

It seems that sometimes the scroll offset is stored somewhere and not synched with cc::Layer::SetScrollOffset().

(Sorry, I put a wrong bug number in the CL so this didn't appear in this bug automatically.)

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2b39360fd0dbcab3bdaa50335978b404c3255126

commit 2b39360fd0dbcab3bdaa50335978b404c3255126
Author: wangxianzhu <wangxianzhu@chromium.org>
Date: Wed Sep 28 17:08:41 2016

Disable paint/invalidation/fixed-right-bottom-in-page-scale.html

It causes the next test to fail.

BUG=  648875  <= should be 648785
TBR=sunxd@chromium.org
NOTRY=true

Review-Url: https://codereview.chromium.org/2379563003
Cr-Commit-Position: refs/heads/master@{#421549}

[modify] https://crrev.com/2b39360fd0dbcab3bdaa50335978b404c3255126/third_party/WebKit/LayoutTests/TestExpectations

It seems in the second test, cc::Layer::SetScrollOffset() is only called for outer viewport scroll layer but not inner viewport scroll layer.

bokan@, what's the correct way to reset the scroll offset of the inner viewport layer?

from the compositor, cc::Layer::SetScrollOffset on the inner viewport layer should work but maybe we should add a reset() method to cc::Viewport that does that and resets the page scale factor

I think there is at least something wrong here:
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/frame/RootFrameViewport.cpp?rcl=0&l=227

If we run the second test after completing the first, here, we have primary's scroll offset being (400, 300) but maximumScrollPositionDouble being (0, 0). This makes us skip updating the inner viewport scroll layer.

More precisely, I think on the blink side, the scroll offset is reset to 0 but the cc does not, which is exactly what bokan@ suggests.

So if I understand you correctly, Blink already thinks its at (0, 0) so it doesn't bother updating the compositor, right? I see here:

https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/frame/FrameView.cpp?sq=package:chromium&dr=CSs&rcl=1475220003&l=3501

If, after clamping, we don't change position, we early-out, before calling PaintLayerCompositor::frameViewDidScroll. Similarly in VisualViewport::didSetScaleOrLocation we only call scrollableAreaScrollLayerDidChange if the offset changed. Do we know whether it's the VisualViewport or FrameView that's not updated on the compositor? The other thing to check would be what causes the scroll offset to reset in Blink but not update the compositor.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/41781b5c4d26bc86779212a9dea21e36d0566622

commit 41781b5c4d26bc86779212a9dea21e36d0566622
Author: sunxd <sunxd@chromium.org>
Date: Fri Oct 14 16:41:23 2016

cc: Update scroll offset before property tree scrolling and animation.

In LayerTreeHostInProcess::FinishCommitOnImplThread, scroll offset in
scroll tree was updated after updating scrolling and animation.

Previously we actively update scroll offset in transform tree when we
later pushing scroll offset map from main to impl. So transform node's
scroll offset is always up-to-date.

However in LayoutTests, we sometimes do not create pending tree. When
pushing directly from main to active, we fail to detect change if the
following criteria are met:

1) current_scroll_offset != new_scroll_offset;
2) new_scroll_offset.pending_value == new_scroll_offset.active_value.

This results in not updating the inner viewport scroll layer when we use
the same renderer and reset page scale factor.

BUG= 648785 
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel

Review-Url: https://codereview.chromium.org/2393213002
Cr-Commit-Position: refs/heads/master@{#425353}

[modify] https://crrev.com/41781b5c4d26bc86779212a9dea21e36d0566622/cc/trees/layer_tree_host_in_process.cc
[modify] https://crrev.com/41781b5c4d26bc86779212a9dea21e36d0566622/cc/trees/layer_tree_host_unittest_scroll.cc
[modify] https://crrev.com/41781b5c4d26bc86779212a9dea21e36d0566622/third_party/WebKit/LayoutTests/TestExpectations