Add comments describing the cc ScrollNode members

cc/trees/scroll_node.h

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef CC_TREES_SCROLL_NODE_H_
 #define CC_TREES_SCROLL_NODE_H_
 
 #include "cc/base/cc_export.h"
 #include "cc/output/filter_operations.h"
 #include "ui/gfx/geometry/size.h"
 
 namespace base {
 namespace trace_event {
 class TracedValue;
 }  // namespace trace_event
 }  // namespace base
 
 namespace cc {
 
 namespace proto {
 class TreeNode;
 }  // namespace proto
 
 struct CC_EXPORT ScrollNode {
   ScrollNode();
   ScrollNode(const ScrollNode& other);
 
   int id;
   int parent_id;
+
+  // The layer id that corresponds to the layer contents that are scrolled.

[ajuma, 2016/09/07 14:19:23]

Please also mention that, unlike "id" which can change each time we rebuild
property trees, this id is stable.

   int owner_id;

[chrishtr, 2016/09/06 23:40:16]

Rename to owning_layer_id?

[ajuma, 2016/09/07 14:19:23]

Since changing the scroll offset stored at this node can potentially move many
layers, and since there's no hierarchical relationship between these layers in
SPv2, or even now on the compositor thread, if we rename this I'd prefer that
the name emphasizes that this id is stable rather than that it happens to match
a layer id right now (we're working towards removing the assumption that
property tree nodes have "owning" layers). Also whatever terminology we end up
using here should be consistent across node types.

[pdr., 2016/09/07 17:58:11]

It looks like the owner_id name is used for many other trees too. I think it
could make sense to rename them all to stable_id (or similar), but in a separate
patch. Are you okay with deferring this to a later patch Chrishtr?

I've updated the comment to mention that this id is stable.

[chrishtr, 2016/09/07 17:59:45]

cool.

 
+  // This is used to delineate a subtree that should not be scrolled. For

[ajuma, 2016/09/07 14:19:23]

"should not be scrolled independently" maybe?

[pdr., 2016/09/07 17:58:11]

I changed this line to "This is used for subtrees that should not be scrolled
independently." which is cleaner and simpler.

+  // example, when there is a layer that is not scrollable itself but is inside
+  // a scrolling layer.
   bool scrollable;
+
   uint32_t main_thread_scrolling_reasons;
   bool contains_non_fast_scrollable_region;
+
+  // Size of the clipped area, not including non-overlay scrollbars. Overlay
+  // scrollbars do not affect the clipped area.
   gfx::Size scroll_clip_layer_bounds;
+
+  // Bounds of the overflow scrolling area.
   gfx::Size bounds;
+
   bool max_scroll_offset_affected_by_page_scale;
   bool is_inner_viewport_scroll_layer;
   bool is_outer_viewport_scroll_layer;
+
+  // This offset is used when |scrollable| is false and there isn't a transform

[ajuma, 2016/09/07 14:19:23]

"transform node"

[pdr., 2016/09/07 17:58:11]

Done

+  // already present that covers this offset.
   gfx::Vector2dF offset_to_transform_parent;
+
   bool should_flatten;
   bool user_scrollable_horizontal;
   bool user_scrollable_vertical;
   ElementId element_id;
   int transform_id;
   // Number of drawn layers pointing to this node or any of its descendants.
   int num_drawn_descendants;
 
   bool operator==(const ScrollNode& other) const;
 
   void ToProtobuf(proto::TreeNode* proto) const;
   void FromProtobuf(const proto::TreeNode& proto);
   void AsValueInto(base::trace_event::TracedValue* value) const;
 };
 
 }  // namespace cc
 
 #endif  // CC_TREES_SCROLL_NODE_H_