Create PageScaleAnimation after ImplTree is activated.

Create PageScaleAnimation after ImplTree is activated.

Since https://codereview.chromium.org/615953005/ the PageScaleAnimation
was created on the pending tree during a commit. This led to crbug.com/428327
since PageScaleAnimation is passed in the TotalScrollOffset of the viewport.
The viewport scroll offset on the pending tree will be different than the
offset on the active tree.

This patch effectively reverts most of https://codereview.chromium.org/615953005/.
Instead, the PendingPageScaleAnimation struct is given to the pending tree. Upon,
activation, this information is used in LayerTreeHostImpl to construct a new
PageScaleAnimation as before, using the active tree's state.

BUG=428327
Committed: https://crrev.com/fcdbc18c93053642b3cb8b143e6b2bf7f521dedb
Cr-Commit-Position: refs/heads/master@{#305292}

[bokan, 2014-10-31 20:31:48]

bokan@chromium.org changed reviewers:
+ aelias@chromium.org

[bokan, 2014-10-31 20:31:49]

PTAL, it's looks like a big patch for a merge, but much of it is just moving
code around and undoing the previous patch. The issue I was seeing was that
activating and pushing the pending tree was causing the inner scroll layer on
the pending tree to get total scroll offset (0, 0), due to the
pending_twin->SetScrollDelta call in SetScrollOffsetAndDelta. I'm not sure why
that's happening, but it seems that we should always be using the active tree to
construct the PageScaleAnimation.

[bokan, 2014-10-31 20:40:01]

On 2014/10/31 20:31:49, bokan wrote:
> PTAL, it's looks like a big patch for a merge, but much of it is just moving
> code around and undoing the previous patch. The issue I was seeing was that
> activating and pushing the pending tree was causing the inner scroll layer on
> the pending tree to get total scroll offset (0, 0), due to the
> pending_twin->SetScrollDelta call in SetScrollOffsetAndDelta. I'm not sure why
> that's happening, but it seems that we should always be using the active tree
to
> construct the PageScaleAnimation.

Also, I'm still working on a test but I thought it might be a good idea to land
this first so it has some time to bake since we'll want to merge it.

[aelias_OOO_until_Jul13, 2014-10-31 21:18:03]

Hmm, the feedback from the pending_twin scroll delta push seems bogus in any
case.  That code looks intended to be executed during impl scrolls, and it seems
like a coincidence that it also happens during commits.  If those lines were
suppressed during commits, would that also fix this bug?

[bokan, 2014-11-03 15:40:09]

On 2014/10/31 21:18:03, aelias wrote:
> Hmm, the feedback from the pending_twin scroll delta push seems bogus in any
> case.  That code looks intended to be executed during impl scrolls, and it
seems
> like a coincidence that it also happens during commits.  If those lines were
> suppressed during commits, would that also fix this bug?

Sure does, everything else looks like it works ok too. Do we want to remove it
altogether or just during tree-activation?

I still think this patch is worth landing, but we can do that after we fix this
bug on the 39 branch.

[aelias_OOO_until_Jul13, 2014-11-03 19:27:44]

OK, lgtm to land this if you think it's a cleanup.  I'm fine with cherry-picking
this to 39 as well if that's more convenient.

You can upload a separate patch for the other fix, it should be reviewed by
enne@.  It should only be disabled during commits because it is useful to update
the pending tree's scroll offset during impl scrolls for the pending tree to
have an accurate idea of what it needs to raster to cover the viewport.  It
would lead to more checkerboarding during scroll + invalidation scenarios if we
removed it completely.

[bokan, 2014-11-21 20:59:08]

The CQ bit was checked by bokan@chromium.org

[commit-bot: I haz the power, 2014-11-21 20:59:49]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/692213002/20001

[commit-bot: I haz the power, 2014-11-21 21:53:40]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2014-11-21 21:54:23]

Patchset 2 (id:??) landed as
https://crrev.com/fcdbc18c93053642b3cb8b143e6b2bf7f521dedb
Cr-Commit-Position: refs/heads/master@{#305292}