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

cc/trees/property_tree.cc

Index: cc/trees/property_tree.cc
diff --git a/cc/trees/property_tree.cc b/cc/trees/property_tree.cc
index acc34795448ed974e952394e018b5092d051f8e9..60bdf19ed46e8d46e56a8c632436926b5935b449 100644
--- a/cc/trees/property_tree.cc
+++ b/cc/trees/property_tree.cc
@@ -1591,6 +1591,9 @@ void ScrollTree::UpdateScrollOffsetMapEntry(
       layer_tree_impl->DidUpdateScrollOffset(key);
     }
   } else {
+    changed |=
+        current_scroll_offset(key) !=
+        new_scroll_offset_map->at(key)->Current(property_trees()->is_active);

[ajuma, 2016/10/05 17:19:21]

I wonder if we're just missing a case here. In the old code (before
https://codereview.chromium.org/1736073002), we effectively had 3 cases
(main-to-pending, main-to-active, pending-to-active) but here we only have
to-pending and to-active. In the main-to-active case, what if we first call
PushFromMainThread and then call PushPendingToActive (and look at the return
values from both calls to compute |changed|)? Does that seem equivalent to what
the old LayerImpl::PushScrollOffset did in this case?

[sunxd, 2016/10/05 18:48:26]

I think as long as we overwrite the scroll offset in the map with the one in the
new map, we end up with the following two situation:

1) main to active: the synced scroll offset is overwritten with clobbering
active value;
2) pending to active: the synced scroll offset is unchanged.

Then we push pending value to active. This should be the same behavior as before
the change except that we didn't check if the new value is the same as the
clobbered old value.

[ajuma, 2016/10/05 19:16:05]

Ah, makes sense now, thanks for the explanation! Please put this into a comment,
since the logic is non-obvious.

     layer_id_to_scroll_offset_map_[key] = new_scroll_offset_map->at(key);
     changed |= synced_scroll_offset(key)->PushPendingToActive();
     if (changed) {