Add a cache of LayerImpl's viewport layer type

cc/layers/layer_impl.h

Index: cc/layers/layer_impl.h
diff --git a/cc/layers/layer_impl.h b/cc/layers/layer_impl.h
index 4d2e85310fb23b08bb6659ef7414bade89188fda..f278444a9a418e1c8e73e209c8e3bbdeef08e81d 100644
--- a/cc/layers/layer_impl.h
+++ b/cc/layers/layer_impl.h
@@ -70,6 +70,16 @@ enum DrawMode {
   DRAW_MODE_RESOURCELESS_SOFTWARE
 };
 
+enum ViewportLayerType {
+  NOT_VIEWPORT_LAYER,
+  INNER_VIEWPORT_CONTAINER,
+  OUTER_VIEWPORT_CONTAINER,
+  INNER_VIEWPORT_SCROLL,
+  OUTER_VIEWPORT_SCROLL,
+  // If values are added or removed, ensure LayerImpl::viewport_layer_type_ is

[enne (OOO), 2017/05/09 17:08:29]

Can you static assert this instead of leaving a comment? Maybe down below saying
static_assert(2<<bitfieldsize > LAST_VIEWPORT_LAYER_TYPE)?

[pdr., 2017/05/09 22:20:07]

Done, I went with:
  static_assert(OUTER_VIEWPORT_SCROLL< (1u << 3),
                "enough bits for ViewportLayerType (viewport_layer_type_)");

+  // updated to use more or less bits as needed.
+};
+
 class CC_EXPORT LayerImpl {
  public:
   static std::unique_ptr<LayerImpl> Create(LayerTreeImpl* tree_impl, int id) {
@@ -264,12 +274,22 @@ class CC_EXPORT LayerImpl {
   // of pixels) due to use of single precision float.
   gfx::SizeF BoundsForScrolling() const;
 
-  // Viewport bounds delta are used for viewport layers and accounts for changes
-  // in the viewport layers from browser controls and page scale factors. These
-  // deltas are only set on the active tree.
+  // Viewport bounds delta are only used for viewport layers and account for
+  // changes in the viewport layers from browser controls and page scale
+  // factors. These deltas are only set on the active tree.
   void SetViewportBoundsDelta(const gfx::Vector2dF& bounds_delta);
   gfx::Vector2dF ViewportBoundsDelta() const;
 
+  void SetViewportLayerType(ViewportLayerType type) {
+    // Once set as a viewport layer type, the viewport type should not change.

[enne (OOO), 2017/05/09 17:08:29]

Can you leave a "why" here as well? Or a quick pointer at which code assumes
this?

[pdr., 2017/05/09 22:20:07]

This appears to be how the system works (we don't shuffle around viewport
layers) but I haven't seen that stated anywhere.

The new code in this patch doesn't unset the viewport layer types on the old
layers if a different layer becomes the viewport layer, so that would break if
this assumption changes. Do you think it would be useful to include that info
here as a comment? Or did you mean to include something else here?

[enne (OOO), 2017/05/09 22:31:52]

Hmm ok ok.  This is fine as-is.

+    DCHECK(viewport_layer_type() == NOT_VIEWPORT_LAYER ||
+           viewport_layer_type() == type);
+    viewport_layer_type_ = type;
+  }
+  ViewportLayerType viewport_layer_type() const {
+    return static_cast<ViewportLayerType>(viewport_layer_type_);
+  }
+
   void SetCurrentScrollOffset(const gfx::ScrollOffset& scroll_offset);
   gfx::ScrollOffset CurrentScrollOffset() const;
 
@@ -503,6 +523,8 @@ class CC_EXPORT LayerImpl {
   // (all content was complete).
   bool was_ever_ready_since_last_transform_animation_ : 1;
 
+  unsigned viewport_layer_type_ : 3;  // ViewportLayerType

[enne (OOO), 2017/05/09 17:08:29]

s/unsigned/uint8_t/

[pdr., 2017/05/09 22:20:07]

Done

+
   Region non_fast_scrollable_region_;
   Region touch_event_handler_region_;
   SkColor background_color_;