Re-introduce ScrollY trace event.

cc/layers/layer_impl.cc

 // Copyright 2012 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.
 
 #include "cc/layers/layer_impl.h"
 
 #include "base/json/json_reader.h"
 #include "base/strings/stringprintf.h"
 #include "base/trace_event/trace_event.h"
 #include "base/trace_event/trace_event_argument.h"
@@ skipping 1304 matching lines @@
   if (scrollbar_layer->orientation() == HORIZONTAL) {
     float visible_ratio = clip_rect.width() / scroll_rect.width();
     scrollbar_needs_animation |=
         scrollbar_layer->SetCurrentPos(current_offset.x());
     scrollbar_needs_animation |=
         scrollbar_layer->SetMaximum(scroll_rect.width() - clip_rect.width());
     scrollbar_needs_animation |=
         scrollbar_layer->SetVisibleToTotalLengthRatio(visible_ratio);
   } else {
     float visible_ratio = clip_rect.height() / scroll_rect.height();
-    scrollbar_needs_animation |=
+    bool y_offset_did_change =
         scrollbar_layer->SetCurrentPos(current_offset.y());
+    scrollbar_needs_animation |= y_offset_did_change;
     scrollbar_needs_animation |=
         scrollbar_layer->SetMaximum(scroll_rect.height() - clip_rect.height());
     scrollbar_needs_animation |=
         scrollbar_layer->SetVisibleToTotalLengthRatio(visible_ratio);
+    // Get the current_offset_.y() value for a sanity-check on scrolling

[enne (OOO), 2015/03/24 17:53:21]

This comment seems unnecessary.

[wjmaclean, 2015/03/24 18:21:40]

Don't we want to indicate why we're interested in recording the scroll offsets?
I've edited the comment to make it briefer and indicate current_offset isn't a
member variable, but it can go away entirely if you like.

+    // benchmark metrics. Specifically, we want to make sure
+    // BasicMouseWheelSmoothScrollGesture has proper scroll curves.
+    if (y_offset_did_change && layer_tree_impl()->IsActiveTree()) {
+      TRACE_COUNTER_ID1("gpu", "scroll_offset_y", this->id(),

[enne (OOO), 2015/03/24 17:53:21]

This should be a cc trace, I think?

Should we only do this on the main scrolling layer, or does every scrolling
layer get a trace?

[wjmaclean, 2015/03/24 18:21:40]

Done.
The previous implementation seems to have reported for every scrolling layer
(presumably this is why the id() was previously included).

+paulirish@

Paul, did you just want this trace for the main scrolling layer? (If so, does
that mean both inner/outer viewports for pinch-zoom?)

+                        current_offset.y());
+    }
   }
   if (scrollbar_needs_animation) {
     layer_tree_impl()->set_needs_update_draw_properties();
     // TODO(wjmaclean) The scrollbar animator for the pinch-zoom scrollbars
     // should activate for every scroll on the main frame, not just the
     // scrolls that move the pinch virtual viewport (i.e. trigger from
     // either inner or outer viewport).
     if (scrollbar_animation_controller_) {
       // Non-overlay scrollbars shouldn't trigger animations.
       if (scrollbar_layer->is_overlay_scrollbar())
@@ skipping 277 matching lines @@
   gfx::Transform scaled_draw_transform =
       draw_properties_.target_space_transform;
   scaled_draw_transform.Scale(SK_MScalar1 / scale, SK_MScalar1 / scale);
   gfx::Size scaled_content_bounds =
       gfx::ToCeiledSize(gfx::ScaleSize(content_bounds(), scale));
   return MathUtil::MapEnclosingClippedRect(scaled_draw_transform,
                                            gfx::Rect(scaled_content_bounds));
 }
 
 }  // namespace cc