Misc. cleanup, ui/ edition.

* Inline temps
* Style guide bans "0.f"
* Don't restate types
* Better Time/TimeDelta unit usage
* Simplify
* Improve comments
* Split separate DCHECK conditions
* Use const/constexpr more
* Eliminate unnecessary heap alloc

Bug: none
Change-Id: I4f7938318d436855f754ea262af268a0a95f414f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2359007
Commit-Queue: Peter Kasting <pkasting@chromium.org>
Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Auto-Submit: Peter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/heads/master@{#798542}

ui/android/edge_effect.cc

@@ -204,9 +204,8 @@ bool EdgeEffect::Update(base::TimeTicks current_time) {
   if (IsFinished())
     return false;
 
-  const base::TimeDelta dt = current_time - start_time_;
-  const double t = std::min(dt / duration_, 1.);
-  const float interp = static_cast<float>(Damp(t, 1.));
+  const double t = std::min((current_time - start_time_) / duration_, 1.0);
+  const float interp = static_cast<float>(Damp(t, 1.0));
 
   glow_alpha_ = Lerp(glow_alpha_start_, glow_alpha_finish_, interp);
   glow_scale_y_ = Lerp(glow_scale_y_start_, glow_scale_y_finish_, interp);
@@ -222,8 +221,8 @@ bool EdgeEffect::Update(base::TimeTicks current_time) {
         glow_alpha_start_ = glow_alpha_;
         glow_scale_y_start_ = glow_scale_y_;
 
-        glow_alpha_finish_ = 0.f;
-        glow_scale_y_finish_ = 0.f;
+        glow_alpha_finish_ = 0.0f;
+        glow_scale_y_finish_ = 0.0f;
         break;
       case STATE_PULL:
         state_ = STATE_PULL_DECAY;
@@ -234,8 +233,8 @@ bool EdgeEffect::Update(base::TimeTicks current_time) {
         glow_scale_y_start_ = glow_scale_y_;
 
         // After pull, the glow should fade to nothing.
-        glow_alpha_finish_ = 0.f;
-        glow_scale_y_finish_ = 0.f;
+        glow_alpha_finish_ = 0.0f;
+        glow_scale_y_finish_ = 0.0f;
         break;
       case STATE_PULL_DECAY:
         state_ = STATE_RECEDE;

ui/base/prediction/linear_resampling.cc

@@ -13,19 +13,16 @@ namespace ui {
 namespace {
 // Minimum time difference between last two consecutive events before attempting
 // to resample.
-constexpr base::TimeDelta kResampleMinDelta =
-    base::TimeDelta::FromMilliseconds(2);
+constexpr auto kResampleMinDelta = base::TimeDelta::FromMilliseconds(2);
 // Maximum time to predict forward from the last event, to avoid predicting too
 // far into the future. This time is further bounded by 50% of the last time
 // delta.
-constexpr base::TimeDelta kResampleMaxPrediction =
-    base::TimeDelta::FromMilliseconds(8);
+constexpr auto kResampleMaxPrediction = base::TimeDelta::FromMilliseconds(8);
 // Align events to a few milliseconds before frame_time. This is to make the
 // resampling either doing interpolation or extrapolating a closer future time
 // so that resampled result is more accurate and has less noise. This adds some
 // latency during resampling but a few ms should be fine.
-constexpr base::TimeDelta kResampleLatency =
-    base::TimeDelta::FromMilliseconds(5);
+constexpr auto kResampleLatency = base::TimeDelta::FromMilliseconds(5);
 
 // Get position at |sample_time| by linear interpolate/extrapolate a and b.
 inline gfx::PointF lerp(const InputPredictor::InputData& a,

ui/compositor/layer_animator_unittest.cc

@@ -1210,7 +1210,7 @@ TEST(LayerAnimatorTest, StartTogetherSetsLastStepTime) {
   // should be enormous. Arbitrarily choosing 1 minute as the threshold,
   // though a much smaller value would probably have sufficed.
   delta = base::TimeTicks::Now() - animator->last_step_time();
-  EXPECT_GT(60.0, delta.InSecondsF());
+  EXPECT_LT(delta, base::TimeDelta::FromMinutes(1));
 }
 
 //-------------------------------------------------------
@@ -3503,17 +3503,15 @@ TEST(LayerAnimatorTest,
   Layer root;
   compositor->SetRootLayer(&root);
 
-  constexpr base::TimeDelta kAnimationDuration =
-      base::TimeDelta::FromMilliseconds(50);
+  constexpr auto kAnimationDuration = base::TimeDelta::FromMilliseconds(50);
 
   // Draw enough frames so that missing the start frame number would cause the
   // reporter to always report 100% smoothness. 4 times of the expected
   // animation frames because somehow the refresh rate changes from 60fps to
   // 200fps when reporting.
-  const float frame_interval =
-      base::Time::kMillisecondsPerSecond / compositor->refresh_rate();
   const int kStartFrameNumber =
-      base::ClampFloor(kAnimationDuration.InMillisecondsF() / frame_interval) *
+      base::ClampFloor(kAnimationDuration.InSecondsF() *
+                       compositor->refresh_rate()) *
       4;
   while (compositor->activated_frame_count() < kStartFrameNumber) {
     compositor->ScheduleFullRedraw();

ui/events/gestures/physics_based_fling_curve.cc

@@ -4,6 +4,8 @@
 
 #include "ui/events/gestures/physics_based_fling_curve.h"
 
+#include <cmath>
+
 namespace {
 
 // These constants are defined based on UX experiment.
@@ -15,8 +17,8 @@ const float kDefaultPixelDeceleration = 2300.0f;
 const float kMaxCurveDurationForFling = 2.0f;
 const float kDefaultPhysicalDeceleration = 2.7559e-5f;  // inch/ms^2.
 
-inline gfx::Vector2dF GetPositionAtTime(const gfx::Vector2dF& end_point,
-                                        double progress) {
+inline gfx::Vector2dF GetPositionAtValue(const gfx::Vector2dF& end_point,
+                                         double progress) {
   return gfx::ScaleVector2d(end_point, progress);
 }
 
@@ -27,12 +29,9 @@ inline gfx::Vector2dF GetVelocityAtTime(const gfx::Vector2dF& current_offset,
 }
 
 float GetOffset(float velocity, float deceleration, float duration) {
-  float position =
+  const float position =
       (std::abs(velocity) - deceleration * duration * 0.5) * duration;
-  if (velocity < 0.0f)
-    return -position;
-
-  return position;
+  return std::copysign(position, velocity);
 }
 
 gfx::Vector2dF GetDecelerationInPixelsPerMs2(
@@ -101,7 +100,8 @@ PhysicsBasedFlingCurve::PhysicsBasedFlingCurve(
       bezier_(p1_.x(), p1_.y(), p2_.x(), p2_.y()),
       previous_time_delta_(base::TimeDelta()) {
   DCHECK(!velocity.IsZero());
-  CHECK(!std::isnan(velocity.x()) && !std::isnan(velocity.y()));
+  DCHECK(!std::isnan(velocity.x()));
+  DCHECK(!std::isnan(velocity.y()));
 }
 
 PhysicsBasedFlingCurve::~PhysicsBasedFlingCurve() = default;
@@ -111,38 +111,33 @@ bool PhysicsBasedFlingCurve::ComputeScrollOffset(base::TimeTicks time,
                                                  gfx::Vector2dF* velocity) {
   DCHECK(offset);
   DCHECK(velocity);
-  base::TimeDelta elapsed_time = time - start_timestamp_;
+
+  const base::TimeDelta elapsed_time = time - start_timestamp_;
   if (elapsed_time < base::TimeDelta()) {
     *offset = gfx::Vector2dF();
     *velocity = gfx::Vector2dF();
     return true;
   }
 
-  bool still_active = true;
-  double x = elapsed_time / curve_duration_;
-  if (x < 1.0f) {
-    double progress = bezier_.Solve(x);
-    *offset = GetPositionAtTime(distance_, progress);
+  const double x = elapsed_time / curve_duration_;
+  const bool still_active = x < 1.0f;
+  if (still_active) {
+    const double progress = bezier_.Solve(x);
+    *offset = GetPositionAtValue(distance_, progress);
     *velocity = GetVelocityAtTime(*offset, prev_offset_,
                                   elapsed_time - previous_time_delta_);
     prev_offset_ = *offset;
     previous_time_delta_ = elapsed_time;
-    still_active = true;
   } else {
-    // At the end of animation, we should have travel distance equal to
-    // distance_
+    // At the end of animation, we should have traveled a distance equal to
+    // |distance_|.
     *offset = distance_;
     *velocity = gfx::Vector2dF();
-    still_active = false;
   }
 
   return still_active;
 }
 
-// This method calculate the curve duration and generate the control points for
-// bezier curve based on velocity and |distance_|. It calculate the slope based
-// on the input velocity (initial velocity), curve duration and |distance_|.
-// Slope is then used to configure the value of control points for curve.
 base::TimeDelta
 PhysicsBasedFlingCurve::CalculateDurationAndConfigureControlPoints(
     const gfx::Vector2dF& velocity) {

ui/events/gestures/physics_based_fling_curve.h

@@ -52,6 +52,9 @@ class EVENTS_BASE_EXPORT PhysicsBasedFlingCurve : public GestureCurve {
   // increases the upper bound of the scroll distance for a fling.
   constexpr static int kDefaultBoundsMultiplier = 3;
 
+  // Calculates the curve duration and generates the control points for a bezier
+  // curve. The slope is based on the input initial |velocity|, calculated curve
+  // duration, and |distance_|. Returns the duration.
   base::TimeDelta CalculateDurationAndConfigureControlPoints(
       const gfx::Vector2dF& velocity);
 

ui/gfx/animation/linear_animation.cc

@@ -45,8 +45,7 @@ double LinearAnimation::GetCurrentValue() const {
 
 void LinearAnimation::SetCurrentValue(double new_value) {
   new_value = base::ClampToRange(new_value, 0.0, 1.0);
-  base::TimeDelta time_delta = base::TimeDelta::FromMicroseconds(
-      static_cast<int64_t>(duration_.InMicroseconds() * (new_value - state_)));
+  const base::TimeDelta time_delta = duration_ * (new_value - state_);
   SetStartTime(start_time() - time_delta);
   state_ = new_value;
 }
@@ -72,18 +71,13 @@ void LinearAnimation::SetDuration(base::TimeDelta duration) {
                ? animation_duration_scale
                : 1.0;
   }();
-  duration_ = duration * duration_scale_factor;
-  if (duration_ < timer_interval())
-    duration_ = timer_interval();
+  duration_ = std::max(duration * duration_scale_factor, timer_interval());
   if (is_animating())
     SetStartTime(container()->last_tick_time());
 }
 
 void LinearAnimation::Step(base::TimeTicks time_now) {
-  base::TimeDelta elapsed_time = time_now - start_time();
-  state_ = elapsed_time / duration_;
-  if (state_ >= 1.0)
-    state_ = 1.0;
+  state_ = std::min((time_now - start_time()) / duration_, 1.0);
 
   AnimateToState(state_);
 

ui/gfx/paint_throbber.cc

@@ -95,12 +95,12 @@ void PaintThrobberSpinningWithStartAngle(
   // The sweep angle ranges from -270 to 270 over 1333ms. CSS
   // animation timing functions apply in between key frames, so we have to
   // break up the 1333ms into two keyframes (-270 to 0, then 0 to 270).
-  const double arc_progress = (elapsed_time % kArcTime) / kArcTime;
+  const double elapsed_ratio = elapsed_time / kArcTime;
+  const int64_t sweep_frame = base::ClampFloor<int64_t>(elapsed_ratio);
+  const double arc_progress = elapsed_ratio - sweep_frame;
   // This tween is equivalent to cubic-bezier(0.4, 0.0, 0.2, 1).
   double sweep = kMaxArcSize *
                  Tween::CalculateValue(Tween::FAST_OUT_SLOW_IN, arc_progress);
-  const int64_t sweep_frame =
-      base::ClampFloor<int64_t>(elapsed_time / kArcTime);
   if (sweep_frame % 2 == 0)
     sweep -= kMaxArcSize;
 
@@ -160,13 +160,13 @@ void PaintThrobberSpinningAfterWaiting(Canvas* canvas,
   if (waiting_state->arc_time_offset.is_zero()) {
     for (int64_t arc_ms = 0; arc_ms <= kArcTime.InMillisecondsRoundedUp();
          ++arc_ms) {
-      double arc_size_progress =
-          std::min(1.0, arc_ms / kArcTime.InMillisecondsF());
+      const base::TimeDelta arc_time =
+          std::min(base::TimeDelta::FromMilliseconds(arc_ms), kArcTime);
       if (kMaxArcSize * Tween::CalculateValue(Tween::FAST_OUT_SLOW_IN,
-                                              arc_size_progress) >=
+                                              arc_time / kArcTime) >=
           waiting_sweep) {
         // Add kArcTime to sidestep the |sweep_keyframe == 0| offset below.
-        waiting_state->arc_time_offset = kArcTime * (arc_size_progress + 1);
+        waiting_state->arc_time_offset = kArcTime + arc_time;
         break;
       }
     }

ui/gfx/skia_vector_animation.cc

@@ -82,8 +82,7 @@ void SkiaVectorAnimation::SetAnimationObserver(
 }
 
 base::TimeDelta SkiaVectorAnimation::GetAnimationDuration() const {
-  return base::TimeDelta::FromMilliseconds(
-      std::floor(SkScalarToFloat(skottie_->duration()) * 1000.f));
+  return base::TimeDelta::FromSecondsD(skottie_->duration());
 }
 
 gfx::Size SkiaVectorAnimation::GetOriginalSize() const {
@@ -148,13 +147,11 @@ float SkiaVectorAnimation::GetCurrentProgress() const {
       DCHECK(timer_control_);
       return timer_control_->GetNormalizedEndOffset();
     case PlayState::kPaused:
-      if (timer_control_) {
-        return timer_control_->GetNormalizedCurrentCycleProgress();
-      } else {
-        // It may be that the timer hasn't been initialized which may happen if
-        // the animation was paused while it was in |kScheculePlay| state.
-        return scheduled_start_offset_ / GetAnimationDuration();
-      }
+      // It may be that the timer hasn't been initialized, which may happen if
+      // the animation was paused while it was in the kSchedulePlay state.
+      return timer_control_
+                 ? timer_control_->GetNormalizedCurrentCycleProgress()
+                 : (scheduled_start_offset_ / GetAnimationDuration());
     case PlayState::kSchedulePlay:
     case PlayState::kPlaying:
     case PlayState::kScheduleResume:

ui/gfx/skia_vector_animation.h

@@ -185,7 +185,7 @@ class GFX_EXPORT SkiaVectorAnimation {
     // Time duration from 0 which marks the beginning of a cycle.
     const base::TimeDelta start_offset_;
 
-    // Time duration  from 0 which marks the end of a cycle.
+    // Time duration from 0 which marks the end of a cycle.
     const base::TimeDelta end_offset_;
 
     // Time duration for one cycle. This is essentially a cache of the

ui/gfx/skia_vector_animation_unittest.cc

@@ -151,7 +151,7 @@ class SkiaVectorAnimationTest : public testing::Test {
     return test_clock_.NowTicks() - ticks;
   }
 
-  const base::TimeTicks NowTicks() const { return test_clock_.NowTicks(); }
+  base::TimeTicks NowTicks() const { return test_clock_.NowTicks(); }
 
   double GetTimerStartOffset() const {
     return animation_->timer_control_->GetNormalizedStartOffset();
@@ -276,7 +276,7 @@ TEST_F(SkiaVectorAnimationTest, StopLinearAnimation) {
                   kAdvance / kAnimationDuration);
 
   animation_->Stop();
-  EXPECT_FLOAT_EQ(animation_->GetCurrentProgress(), 0.f);
+  EXPECT_FLOAT_EQ(animation_->GetCurrentProgress(), 0.0f);
   EXPECT_TRUE(IsStopped());
 }
 
@@ -410,7 +410,7 @@ TEST_F(SkiaVectorAnimationTest, PlayLoopAnimation) {
 
   EXPECT_FLOAT_EQ(animation_->GetCurrentProgress(), 0);
   EXPECT_FLOAT_EQ(GetTimerStartOffset(), 0);
-  EXPECT_FLOAT_EQ(GetTimerEndOffset(), 1.f);
+  EXPECT_FLOAT_EQ(GetTimerEndOffset(), 1.0f);
 
   EXPECT_EQ(GetTimerTotalDuration(), kAnimationDuration);
 
@@ -552,7 +552,6 @@ TEST_F(SkiaVectorAnimationTest, PausingLoopAnimation) {
 }
 
 TEST_F(SkiaVectorAnimationTest, PlayThrobbingAnimation) {
-
   TestAnimationObserver observer;
   animation_->SetAnimationObserver(&observer);
 
@@ -570,7 +569,7 @@ TEST_F(SkiaVectorAnimationTest, PlayThrobbingAnimation) {
 
   EXPECT_FLOAT_EQ(animation_->GetCurrentProgress(), 0);
   EXPECT_FLOAT_EQ(GetTimerStartOffset(), 0);
-  EXPECT_FLOAT_EQ(GetTimerEndOffset(), 1.f);
+  EXPECT_FLOAT_EQ(GetTimerEndOffset(), 1.0f);
 
   EXPECT_EQ(GetTimerTotalDuration(), kAnimationDuration);
 
@@ -588,7 +587,7 @@ TEST_F(SkiaVectorAnimationTest, PlayThrobbingAnimation) {
   EXPECT_EQ(TimeDeltaSince(GetTimerPreviousTick()),
             kAnimationDuration - kAdvance);
   animation_->Paint(canvas(), NowTicks(), animation_->GetOriginalSize());
-  EXPECT_FLOAT_EQ(animation_->GetCurrentProgress(), 1.f);
+  EXPECT_FLOAT_EQ(animation_->GetCurrentProgress(), 1.0f);
   EXPECT_TRUE(IsPlaying());
   EXPECT_FALSE(observer.animation_cycle_ended());
 
@@ -777,10 +776,9 @@ TEST_F(SkiaVectorAnimationTest, PausingThrobbingAnimation) {
   EXPECT_TRUE(IsPlaying());
 }
 
+// Test to see if the race condition is handled correctly. It may happen that we
+// pause the video before it even starts playing.
 TEST_F(SkiaVectorAnimationTest, PauseBeforePlay) {
-  // Test to see if the race condition is handled correctly. It may happen that
-  // we pause the video before it even starts playing.
-
   TestAnimationObserver observer;
   animation_->SetAnimationObserver(&observer);
 
@@ -810,38 +808,32 @@ TEST_F(SkiaVectorAnimationTest, PauseBeforePlay) {
 }
 
 TEST_F(SkiaVectorAnimationTest, PaintTest) {
-  std::unique_ptr<gfx::Canvas> canvas(new gfx::Canvas(
-      gfx::Size(kAnimationWidth, kAnimationHeight), 1.f, false));
+  gfx::Canvas canvas(gfx::Size(kAnimationWidth, kAnimationHeight), 1.f, false);
 
   AdvanceClock(base::TimeDelta::FromMilliseconds(300));
 
   animation_->Start(SkiaVectorAnimation::Style::kLinear);
-  animation_->Paint(canvas.get(), NowTicks(), animation_->GetOriginalSize());
+  animation_->Paint(&canvas, NowTicks(), animation_->GetOriginalSize());
 
   AdvanceClock(base::TimeDelta::FromMilliseconds(50));
-  animation_->Paint(canvas.get(), NowTicks(), animation_->GetOriginalSize());
-  SkBitmap bitmap = canvas->GetBitmap();
-  IsAllSameColor(SK_ColorGREEN, bitmap);
+  animation_->Paint(&canvas, NowTicks(), animation_->GetOriginalSize());
+  IsAllSameColor(SK_ColorGREEN, canvas.GetBitmap());
 
   AdvanceClock(base::TimeDelta::FromMilliseconds(2450));
-  animation_->Paint(canvas.get(), NowTicks(), animation_->GetOriginalSize());
-  bitmap = canvas->GetBitmap();
-  IsAllSameColor(SK_ColorGREEN, bitmap);
+  animation_->Paint(&canvas, NowTicks(), animation_->GetOriginalSize());
+  IsAllSameColor(SK_ColorGREEN, canvas.GetBitmap());
 
   AdvanceClock(base::TimeDelta::FromMilliseconds(50));
-  animation_->Paint(canvas.get(), NowTicks(), animation_->GetOriginalSize());
-  bitmap = canvas->GetBitmap();
-  IsAllSameColor(SK_ColorBLUE, bitmap);
+  animation_->Paint(&canvas, NowTicks(), animation_->GetOriginalSize());
+  IsAllSameColor(SK_ColorBLUE, canvas.GetBitmap());
 
   AdvanceClock(base::TimeDelta::FromMilliseconds(1000));
-  animation_->Paint(canvas.get(), NowTicks(), animation_->GetOriginalSize());
-  bitmap = canvas->GetBitmap();
-  IsAllSameColor(SK_ColorBLUE, bitmap);
+  animation_->Paint(&canvas, NowTicks(), animation_->GetOriginalSize());
+  IsAllSameColor(SK_ColorBLUE, canvas.GetBitmap());
 
   AdvanceClock(base::TimeDelta::FromMilliseconds(1400));
-  animation_->Paint(canvas.get(), NowTicks(), animation_->GetOriginalSize());
-  bitmap = canvas->GetBitmap();
-  IsAllSameColor(SK_ColorBLUE, bitmap);
+  animation_->Paint(&canvas, NowTicks(), animation_->GetOriginalSize());
+  IsAllSameColor(SK_ColorBLUE, canvas.GetBitmap());
 }
 
 }  // namespace gfx

ui/touch_selection/touch_handle.cc

@@ -21,17 +21,17 @@ constexpr auto kFadeDuration = base::TimeDelta::FromMilliseconds(200);
 
 // Maximum amount of travel for a fade sequence. This avoids handle "ghosting"
 // when the handle is moving rapidly while the fade is active.
-const double kFadeDistanceSquared = 20.f * 20.f;
+constexpr double kFadeDistanceSquared = 20.0f * 20.0f;
 
 // Avoid using an empty touch rect, as it may fail the intersection test event
 // if it lies within the other rect's bounds.
-const float kMinTouchMajorForHitTesting = 1.f;
+constexpr float kMinTouchMajorForHitTesting = 1.0f;
 
 // The maximum touch size to use when computing whether a touch point is
 // targetting a touch handle. This is necessary for devices that misreport
 // touch radii, preventing inappropriately largely touch sizes from completely
 // breaking handle dragging behavior.
-const float kMaxTouchMajorForHitTesting = 36.f;
+constexpr float kMaxTouchMajorForHitTesting = 36.0f;
 
 // Note that the intersection region is boundary *exclusive*.
 bool RectIntersectsCircle(const gfx::RectF& rect,
@@ -236,18 +236,17 @@ bool TouchHandle::Animate(base::TimeTicks frame_time) {
 
   DCHECK(enabled_);
 
-  float time_u = 1.f - (fade_end_time_ - frame_time) / kFadeDuration;
-  float position_u = (focus_bottom_ - fade_start_position_).LengthSquared() /
-                     kFadeDistanceSquared;
-  float u = std::max(time_u, position_u);
-  SetAlpha(is_visible_ ? u : 1.f - u);
+  const float time_u = 1.0f - (fade_end_time_ - frame_time) / kFadeDuration;
+  const float position_u =
+      (focus_bottom_ - fade_start_position_).LengthSquared() /
+      kFadeDistanceSquared;
+  const float u = std::max(time_u, position_u);
+  SetAlpha(is_visible_ ? u : 1.0f - u);
 
-  if (u >= 1.f) {
+  if (u >= 1)
     EndFade();
-    return false;
-  }
 
-  return true;
+  return u < 1;
 }
 
 gfx::RectF TouchHandle::GetVisibleBounds() const {