Clamp visible loading progress to a smaller range

Prevents the loading animation from looking stuck. 0% looked buggy, like
we wouldn't even start loading. 100% looks similarly buggy, if we're
100% done we should not be stuck in a loading state.

Bug: chromium:908920
Change-Id: I280a9f7ce9f7db3920c41add9220a5ed4604b900
Reviewed-on: https://chromium-review.googlesource.com/c/1351103
Commit-Queue: Peter Boström <pbos@chromium.org>
Reviewed-by: Sidney San Martín <sdy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#611485}

chrome/browser/ui/views/tabs/tab_icon.cc

@@ -463,15 +463,14 @@ void TabIcon::SetNetworkState(TabNetworkState network_state,
     }
 
     if (old_state == TabNetworkState::kLoading) {
+      // Rewind the progress timer back to currently displayed progress bar so
+      // we don't miss the end of the animation.
+      RewindLoadingProgressTimerIfNecessary(target_loading_progress_);
       target_loading_progress_ = 1.0;
       // Start fading in placeholder favicon if no favicon has loaded so far.
       const base::TimeTicks now = clock_->NowTicks();
       if (!favicon_fade_in_animation_)
         favicon_fade_in_animation_ = now;
-
-      // Rewind the progress timer back to 100% if necessary. This prevents
-      // parts of the fade-out animation to be skipped.
-      RewindLoadingProgressTimerIfNecessary(1.0f);
     }
 
     if (network_state_ == TabNetworkState::kWaiting) {
@@ -482,15 +481,23 @@ void TabIcon::SetNetworkState(TabNetworkState network_state,
     SchedulePaint();
   }
 
-  // The loading progress looks really weird if it ever jumps backwards, so make
-  // sure it only increases.
-  if (network_state_ == TabNetworkState::kLoading &&
-      target_loading_progress_ < load_progress) {
-    DCHECK(loading_progress_timer_);
-
-    RewindLoadingProgressTimerIfNecessary(target_loading_progress_);
-
-    target_loading_progress_ = load_progress;
+  if (network_state_ == TabNetworkState::kLoading) {
+    // Interpolate loading progress to a narrower range to prevent us from
+    // looking stuck doing nothing at 0% or at 100% but still not finishing.
+    constexpr float kLoadingProgressStart = 0.3;
+    constexpr float kLoadingProgressEnd = 0.7;
+    load_progress =
+        kLoadingProgressStart +
+        load_progress * (kLoadingProgressEnd - kLoadingProgressStart);
+
+    // The loading progress looks really weird if it ever jumps backwards, so
+    // make sure it only increases.
+    if (target_loading_progress_ < load_progress) {
+      DCHECK(loading_progress_timer_);
+
+      RewindLoadingProgressTimerIfNecessary(target_loading_progress_);
+      target_loading_progress_ = load_progress;
+    }
   }
 }
 

chrome/browser/ui/views/tabs/tab_unittest.cc

@@ -718,17 +718,22 @@ TEST_F(TabTest, LoadingProgressMonotonicallyIncreases) {
   data.network_state = TabNetworkState::kLoading;
   data.load_progress = 0.2;
   tab.SetData(data);
-  EXPECT_FLOAT_EQ(0.2, GetLoadingProgress(icon));
+  float initial_reported_progress = GetLoadingProgress(icon);
+  // Reported progress should interpolate to something between itself and 1.0.
+  EXPECT_GE(initial_reported_progress, 0.2);
+  EXPECT_LT(initial_reported_progress, 1.0);
 
   // Decrease load progress, icon's load progress should not change.
   data.load_progress = 0.1;
   tab.SetData(data);
-  EXPECT_FLOAT_EQ(0.2, GetLoadingProgress(icon));
+  EXPECT_FLOAT_EQ(initial_reported_progress, GetLoadingProgress(icon));
 
   // Though increasing it should be respected.
   data.load_progress = 0.5;
   tab.SetData(data);
-  EXPECT_FLOAT_EQ(0.5, GetLoadingProgress(icon));
+  // A higher load progress should be interpolate to larger value (less than 1).
+  EXPECT_GT(GetLoadingProgress(icon), initial_reported_progress);
+  EXPECT_LT(GetLoadingProgress(icon), 1.0);
 }
 
 TEST_F(TabTest, LoadingProgressGoesTo100PercentAfterLoadingIsDone) {
@@ -745,7 +750,9 @@ TEST_F(TabTest, LoadingProgressGoesTo100PercentAfterLoadingIsDone) {
   data.network_state = TabNetworkState::kLoading;
   data.load_progress = 0.2;
   tab.SetData(data);
-  EXPECT_FLOAT_EQ(0.2, GetLoadingProgress(icon));
+  // Reported progress should interpolate to something between itself and 1.0.
+  EXPECT_GE(GetLoadingProgress(icon), 0.2);
+  EXPECT_LT(GetLoadingProgress(icon), 1.0);
 
   // Finish loading. Regardless of reported |data.load_progress|, load_progress
   // should be drawn at 100%.