Reland "Disable the new tab-loading animation"

This reverts commit 742b8857.

Reason for revert: Flaky tests should be fixed in r615470.

Bug: chromium:912543, chromium:913135, chromium:913784

Original change's description:
> Revert "Disable the new tab-loading animation"
> 
> This reverts commit 355b8185.
> 
> Reason for revert: Made several tests flaky: https://crbug.com/912543
> 
> Original change's description:
> > Disable the new tab-loading animation
> > 
> > Makes sure that a lot of animation-related code is bypassed when the
> > new-tab-animation flag is off. This should hopefully fix a couple of
> > performance regressions that have not yet been root caused so that they
> > don't go out with M72.
> > 
> > Bug: chromium:912328, chromium:905745, chromium:905918, chromium:910265
> > Change-Id: Id3f131db427eb3ee1618d6c9683fd5e47dc134e8
> > Reviewed-on: https://chromium-review.googlesource.com/c/1364212
> > Reviewed-by: Sidney San Martín <sdy@chromium.org>
> > Commit-Queue: Peter Boström <pbos@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#614199}
> 
> TBR=pbos@chromium.org,sdy@chromium.org
> 
> Change-Id: Ib4c022a255ad085c1716d3559a7f84dcb61c2785
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug: chromium:912328, chromium:905745, chromium:905918, chromium:910265
> Reviewed-on: https://chromium-review.googlesource.com/c/1366359
> Reviewed-by: Jeffrey Yasskin <jyasskin@chromium.org>
> Commit-Queue: Jeffrey Yasskin <jyasskin@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#614440}

TBR=jyasskin@chromium.org,pbos@chromium.org,sdy@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: chromium:912328, chromium:905745, chromium:905918, chromium:910265
Change-Id: I3981455a00f743fae535de4626179dcceab9b2c4
Reviewed-on: https://chromium-review.googlesource.com/c/1372236Reviewed-by: Peter Boström <pbos@chromium.org>
Commit-Queue: Peter Boström <pbos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#615581}

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

@@ -151,6 +151,9 @@ bool TabIcon::ShowingLoadingAnimation() const {
   if (NetworkStateIsAnimated(network_state_))
     return true;
 
+  if (!UseNewLoadingAnimation())
+    return false;
+
   if (LoadingAnimationNeedsRepaint())
     return true;
 
@@ -182,14 +185,20 @@ void TabIcon::SetCanPaintToLayer(bool can_paint_to_layer) {
 }
 
 void TabIcon::StepLoadingAnimation(const base::TimeDelta& elapsed_time) {
-  waiting_state_.elapsed_time = elapsed_time;
+  // The old loading animation only updates elapsed time while it's loading.
+  // This is used as a starting point for PaintThrobberSpinningAfterWaiting().
+  if (UseNewLoadingAnimation() || network_state_ == TabNetworkState::kWaiting)
+    waiting_state_.elapsed_time = elapsed_time;
 
   UpdatePendingAnimationState();
 
   if (LoadingAnimationNeedsRepaint())
     SchedulePaint();
 
-  RefreshLayer();
+  // TODO(pbos): Revisit this, ideally we should always be able to paint on a
+  // layer.
+  if (UseNewLoadingAnimation())
+    RefreshLayer();
 }
 
 void TabIcon::SetBackgroundColor(SkColor bg_color) {
@@ -437,6 +446,8 @@ const gfx::ImageSkia& TabIcon::GetIconToPaint() {
 
 void TabIcon::MaybePaintFaviconPlaceholder(gfx::Canvas* canvas,
                                            const gfx::Rect& bounds) {
+  if (!UseNewLoadingAnimation())
+    return;
   if (!animation_state_.favicon_placeholder_alpha)
     return;
   cc::PaintFlags flags;
@@ -458,14 +469,16 @@ void TabIcon::MaybePaintFavicon(gfx::Canvas* canvas,
                                 const gfx::Rect& bounds) {
   // While loading, the favicon (or placeholder) isn't drawn until it has
   // started fading in.
-  if (!animation_state_.favicon_fade_in_progress)
+  if (UseNewLoadingAnimation() && !animation_state_.favicon_fade_in_progress)
     return;
 
   if (icon.isNull())
     return;
 
   cc::PaintFlags flags;
-  double fade_in_progress = *animation_state_.favicon_fade_in_progress;
+  double fade_in_progress = UseNewLoadingAnimation()
+                                ? *animation_state_.favicon_fade_in_progress
+                                : 1.0;
   flags.setAlpha(fade_in_progress * SK_AlphaOPAQUE);
   // Drop in the new favicon from the top while it's fading in.
   const int offset = round((fade_in_progress - 1.0) * 4.0);
@@ -482,6 +495,9 @@ bool TabIcon::HasNonDefaultFavicon() const {
 }
 
 void TabIcon::MaybeStartFaviconFadeIn() {
+  if (!UseNewLoadingAnimation())
+    return;
+
   if (pending_animation_state_.favicon_fade_in_progress)
     return;
 
@@ -521,6 +537,11 @@ void TabIcon::SetIcon(const GURL& url, const gfx::ImageSkia& icon) {
 
 void TabIcon::SetNetworkState(TabNetworkState network_state,
                               float load_progress) {
+  if (!UseNewLoadingAnimation()) {
+    network_state_ = network_state;
+    return;
+  }
+
   if (network_state_ != network_state) {
     TabNetworkState old_state = network_state_;
     network_state_ = network_state;

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

@@ -20,6 +20,7 @@
 #include "chrome/browser/ui/views/tabs/tab_icon.h"
 #include "chrome/browser/ui/views/tabs/tab_strip.h"
 #include "chrome/browser/ui/views/tabs/tab_style.h"
+#include "chrome/common/chrome_features.h"
 #include "chrome/grit/theme_resources.h"
 #include "chrome/test/views/chrome_views_test_base.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -33,6 +34,12 @@
 
 using views::Widget;
 
+namespace {
+bool UsingNewLoadingAnimation() {
+  return base::FeatureList::IsEnabled(features::kNewTabLoadingAnimation);
+}
+}  // namespace
+
 class FakeTabController : public TabController {
  public:
   FakeTabController() {}
@@ -609,10 +616,12 @@ TEST_F(TabTest, LayeredThrobber) {
   EXPECT_TRUE(icon->layer());
   data.network_state = TabNetworkState::kNone;
   tab.SetData(data);
-  // The post-loading animation should still be playing (loading bar fades out).
-  EXPECT_TRUE(icon->ShowingLoadingAnimation());
-
-  FinishRunningLoadingAnimations(icon);
+  if (UsingNewLoadingAnimation()) {
+    // The post-loading animation should still be playing (loading bar fades
+    // out).
+    EXPECT_TRUE(icon->ShowingLoadingAnimation());
+    FinishRunningLoadingAnimations(icon);
+  }
   EXPECT_FALSE(icon->ShowingLoadingAnimation());
 
   // Simulate a tab that should hide throbber.
@@ -642,9 +651,12 @@ TEST_F(TabTest, LayeredThrobber) {
   EXPECT_TRUE(icon->layer());
   data.network_state = TabNetworkState::kNone;
   tab.SetData(data);
-  // The post-loading animation should still be playing (loading bar fades out).
-  EXPECT_TRUE(icon->ShowingLoadingAnimation());
-  FinishRunningLoadingAnimations(icon);
+  if (UsingNewLoadingAnimation()) {
+    // The post-loading animation should still be playing (loading bar fades
+    // out).
+    EXPECT_TRUE(icon->ShowingLoadingAnimation());
+    FinishRunningLoadingAnimations(icon);
+  }
   EXPECT_FALSE(icon->ShowingLoadingAnimation());
 
   // After loading is done, simulate another resource starting to load.
@@ -713,6 +725,8 @@ TEST_F(TabTest, LoadingProgressIsFixedTo100PercentWhenNotLoading) {
 }
 
 TEST_F(TabTest, LoadingProgressMonotonicallyIncreases) {
+  if (!UsingNewLoadingAnimation())
+    return;
   Widget widget;
   InitWidget(&widget);
 
@@ -745,6 +759,9 @@ TEST_F(TabTest, LoadingProgressMonotonicallyIncreases) {
 }
 
 TEST_F(TabTest, LoadingProgressGoesTo100PercentAfterLoadingIsDone) {
+  if (!UsingNewLoadingAnimation())
+    return;
+
   Widget widget;
   InitWidget(&widget);
 

chrome/common/chrome_features.cc

@@ -384,7 +384,7 @@ const char kNewNetErrorPageUIAlternateContentPreview[] = "content_preview";
 
 #if !defined(OS_ANDROID)
 const base::Feature kNewTabLoadingAnimation{"NewTabLoadingAnimation",
-                                            base::FEATURE_ENABLED_BY_DEFAULT};
+                                            base::FEATURE_DISABLED_BY_DEFAULT};
 #endif  // !OS_ANDROID
 
 #if defined(OS_POSIX)