Add sticky nodes affected by inner and outer viewport bounds deltas.

Add sticky nodes affected by inner and outer viewport bounds deltas.

We need to additionally add sticky nodes affected by the outer viewport bounds, which is the common case on Android. Sticky nodes are not moved unconditionally by viewport bounds deltas though since they have constraints on their movement.

BUG=649111
TEST=LayerTreeHostCommonTest.StickyPositionBottomInnerViewportDelta,LayerTreeHostCommonTest.StickyPositionBottomOuterViewportDelta
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel

Committed: https://crrev.com/a283bedb5251108878e9cd71539464641eeec21b
Cr-Commit-Position: refs/heads/master@{#428708}

[flackr, 2016-10-26 14:33:57]

Description was changed from

==========
Add sticky nodes affected by inner and outer viewport bounds deltas.

We used to compute the clip size by accounting for the top control delta
but cc has special plumbing to move nodes affected by top controls which
do the adjustment at the correct time. We were applying the adjustment
too late which caused jittering while hiding/showing top controls.

BUG=649111
TEST=LayerTreeHostCommonTest.StickyPositionBottomHideTopControls
==========

to

==========
Add sticky nodes affected by inner and outer viewport bounds deltas.

We used to compute the clip size by accounting for the top control delta
but cc has special plumbing to move nodes affected by top controls which
do the adjustment at the correct time. We were applying the adjustment
too late which caused jittering while hiding/showing top controls.

BUG=649111
TEST=LayerTreeHostCommonTest.StickyPositionBottomHideTopControls
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

[flackr, 2016-10-26 14:36:24]

flackr@chromium.org changed reviewers:
+ ajuma@chromium.org

[flackr, 2016-10-26 14:36:25]

Hey,

Can you take a look? This removes including the top control delta when computing
the sticky position and instead uses the same plumbing that fixed position uses
to tell cc to reposition the nodes when the top controls are showing / hiding.
This fixes the jittering during top control hiding / showing. The correctness is
tested by the existing top control hiding test.

Thanks,
Rob

[ajuma, 2016-10-26 14:44:49]

On 2016/10/26 14:36:25, flackr wrote:
> Hey,
> 
> Can you take a look? This removes including the top control delta when
computing
> the sticky position and instead uses the same plumbing that fixed position
uses
> to tell cc to reposition the nodes when the top controls are showing / hiding.
> This fixes the jittering during top control hiding / showing. The correctness
is
> tested by the existing top control hiding test.
> 
> Thanks,
> Rob

The fix looks good, but would it be possible to add a test that would have
failed because of the jittering that was happening before this change?

[flackr, 2016-10-28 16:48:11]

Description was changed from

==========
Add sticky nodes affected by inner and outer viewport bounds deltas.

We used to compute the clip size by accounting for the top control delta
but cc has special plumbing to move nodes affected by top controls which
do the adjustment at the correct time. We were applying the adjustment
too late which caused jittering while hiding/showing top controls.

BUG=649111
TEST=LayerTreeHostCommonTest.StickyPositionBottomHideTopControls
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

to

==========
Add sticky nodes affected by inner and outer viewport bounds deltas.

We used to compute the clip size by accounting for the top control delta
but cc has special plumbing to move nodes affected by top controls which
do the adjustment at the correct time. We were applying the adjustment
too late which caused jittering while hiding/showing top controls.

BUG=649111
TEST=LayerTreeHostCommonTest.StickyPositionBottomInnerViewportDelta,LayerTreeHostCommonTest.StickyPositionBottomOuterViewportDelta
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

[flackr, 2016-10-28 16:49:08]

Hey,

As we discussed, I realized we still have to handle the bounds adjustment
differently for sticky since you can hit the sticky extents (see updates to the
unit tests which test for this). The jitter on android was simply because I was
only adding to inner viewport, but adding to outer viewport affected nodes seems
to have resolved this issue.

PTAL, thanks!

[ajuma, 2016-10-28 17:10:41]

Thanks, lgtm

https://codereview.chromium.org/2451183002/diff/20001/cc/trees/property_tree_...
File cc/trees/property_tree_builder.cc (right):

https://codereview.chromium.org/2451183002/diff/20001/cc/trees/property_tree_...
cc/trees/property_tree_builder.cc:712:
.AddNodeAffectedByInnerViewportBoundsDelta(node->id);
As discussed, please rename TransformNode::affected_by_.... to
TransformNode::moved_by...., since these are no longer symmetric with the the
nodes_affected_by... sets. Please also add a comment about this.

[flackr, 2016-10-28 17:12:11]

Description was changed from

==========
Add sticky nodes affected by inner and outer viewport bounds deltas.

We used to compute the clip size by accounting for the top control delta
but cc has special plumbing to move nodes affected by top controls which
do the adjustment at the correct time. We were applying the adjustment
too late which caused jittering while hiding/showing top controls.

BUG=649111
TEST=LayerTreeHostCommonTest.StickyPositionBottomInnerViewportDelta,LayerTreeHostCommonTest.StickyPositionBottomOuterViewportDelta
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

to

==========
Add sticky nodes affected by inner and outer viewport bounds deltas.

We need to additionally add sticky nodes affected by the outer viewport bounds,
which is the common case on Android. Sticky nodes are not moved unconditionally
by viewport bounds deltas though since they have constraints on their movement.

BUG=649111
TEST=LayerTreeHostCommonTest.StickyPositionBottomInnerViewportDelta,LayerTreeHostCommonTest.StickyPositionBottomOuterViewportDelta
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

[flackr, 2016-10-28 17:24:51]

https://codereview.chromium.org/2451183002/diff/20001/cc/trees/property_tree_...
File cc/trees/property_tree_builder.cc (right):

https://codereview.chromium.org/2451183002/diff/20001/cc/trees/property_tree_...
cc/trees/property_tree_builder.cc:712:
.AddNodeAffectedByInnerViewportBoundsDelta(node->id);
On 2016/10/28 at 17:10:41, ajuma wrote:
> As discussed, please rename TransformNode::affected_by_.... to
TransformNode::moved_by...., since these are no longer symmetric with the the
nodes_affected_by... sets. Please also add a comment about this.

Done.

[flackr, 2016-10-28 17:24:54]

The CQ bit was checked by flackr@chromium.org

[flackr, 2016-10-28 17:24:54]

The patchset sent to the CQ was uploaded after l-g-t-m from ajuma@chromium.org
Link to the patchset: https://codereview.chromium.org/2451183002/#ps40001
(title: "Rename affected_by_*_bounds_delta_* to moved_by_*_bounds_delta_* and
add comment.")

[commit-bot: I haz the power, 2016-10-28 17:25:17]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-28 17:27:06]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-28 17:27:07]

Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[flackr, 2016-10-28 18:09:50]

The CQ bit was checked by flackr@chromium.org

[flackr, 2016-10-28 18:09:50]

The patchset sent to the CQ was uploaded after l-g-t-m from ajuma@chromium.org
Link to the patchset: https://codereview.chromium.org/2451183002/#ps60001
(title: "Merge with master.")

[commit-bot: I haz the power, 2016-10-28 18:10:14]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-28 20:24:22]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-28 20:24:23]

Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[flackr, 2016-10-31 13:55:02]

The CQ bit was checked by flackr@chromium.org

[commit-bot: I haz the power, 2016-10-31 13:55:15]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-31 14:50:12]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-10-31 14:52:00]

Description was changed from

==========
Add sticky nodes affected by inner and outer viewport bounds deltas.

We need to additionally add sticky nodes affected by the outer viewport bounds,
which is the common case on Android. Sticky nodes are not moved unconditionally
by viewport bounds deltas though since they have constraints on their movement.

BUG=649111
TEST=LayerTreeHostCommonTest.StickyPositionBottomInnerViewportDelta,LayerTreeHostCommonTest.StickyPositionBottomOuterViewportDelta
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

to

==========
Add sticky nodes affected by inner and outer viewport bounds deltas.

We need to additionally add sticky nodes affected by the outer viewport bounds,
which is the common case on Android. Sticky nodes are not moved unconditionally
by viewport bounds deltas though since they have constraints on their movement.

BUG=649111
TEST=LayerTreeHostCommonTest.StickyPositionBottomInnerViewportDelta,LayerTreeHostCommonTest.StickyPositionBottomOuterViewportDelta
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel

Committed: https://crrev.com/a283bedb5251108878e9cd71539464641eeec21b
Cr-Commit-Position: refs/heads/master@{#428708}
==========

[commit-bot: I haz the power, 2016-10-31 14:52:01]

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