Revert "Reland "Disable the new tab-loading animation""

This reverts commit ad12d441.

Reason for revert:
Findit found this to be the most likely culprit for the single_process_mash_browser_tests failures on various Linux bots:
https://findit-for-me.appspot.com/waterfall/failure?url=https://build.chromium.org/p/chromium.memory/builders/Linux%20Chromium%20OS%20ASan%20LSan%20Tests%20%281%29/builds/30518


Original change's description:
> 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/1372236
> Reviewed-by: Peter Boström <pbos@chromium.org>
> Commit-Queue: Peter Boström <pbos@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#615581}

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

Change-Id: Ib9599bb8fd44327ae756d3c9523049367409612c
Bug: chromium:912543, chromium:913135, chromium:913784, chromium:912328, chromium:905745, chromium:905918, chromium:910265
Reviewed-on: https://chromium-review.googlesource.com/c/1373832
Commit-Queue: Friedrich Horschig [CET] <fhorschig@chromium.org>
Reviewed-by: Friedrich Horschig [CET] <fhorschig@chromium.org>
Cr-Commit-Position: refs/heads/master@{#615878}

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

@@ -152,9 +152,6 @@ bool TabIcon::ShowingLoadingAnimation() const {
   if (NetworkStateIsAnimated(network_state_))
     return true;
 
-  if (!UseNewLoadingAnimation())
-    return false;
-
   if (LoadingAnimationNeedsRepaint())
     return true;
 
@@ -186,20 +183,14 @@ void TabIcon::SetCanPaintToLayer(bool can_paint_to_layer) {
 }
 
 void TabIcon::StepLoadingAnimation(const base::TimeDelta& 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;
+  waiting_state_.elapsed_time = elapsed_time;
 
   UpdatePendingAnimationState();
 
   if (LoadingAnimationNeedsRepaint())
     SchedulePaint();
 
-  // TODO(pbos): Revisit this, ideally we should always be able to paint on a
-  // layer.
-  if (UseNewLoadingAnimation())
-    RefreshLayer();
+  RefreshLayer();
 }
 
 void TabIcon::SetBackgroundColor(SkColor bg_color) {
@@ -448,8 +439,6 @@ 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;
@@ -471,18 +460,15 @@ 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 (UseNewLoadingAnimation() && !animation_state_.favicon_fade_in_progress)
+  if (!animation_state_.favicon_fade_in_progress)
     return;
 
   if (icon.isNull())
     return;
 
   cc::PaintFlags flags;
-  double fade_in_progress =
-      UseNewLoadingAnimation() ? gfx::Tween::CalculateValue(
-                                     gfx::Tween::FAST_OUT_SLOW_IN,
-                                     *animation_state_.favicon_fade_in_progress)
-                               : 1.0;
+  double fade_in_progress = gfx::Tween::CalculateValue(
+      gfx::Tween::FAST_OUT_SLOW_IN, *animation_state_.favicon_fade_in_progress);
   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);
@@ -499,9 +485,6 @@ bool TabIcon::HasNonDefaultFavicon() const {
 }
 
 void TabIcon::MaybeStartFaviconFadeIn() {
-  if (!UseNewLoadingAnimation())
-    return;
-
   if (pending_animation_state_.favicon_fade_in_progress)
     return;
 
@@ -541,11 +524,6 @@ 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,7 +20,6 @@
 #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"
@@ -34,12 +33,6 @@
 
 using views::Widget;
 
-namespace {
-bool UsingNewLoadingAnimation() {
-  return base::FeatureList::IsEnabled(features::kNewTabLoadingAnimation);
-}
-}  // namespace
-
 class FakeTabController : public TabController {
  public:
   FakeTabController() {}
@@ -616,12 +609,10 @@ TEST_F(TabTest, LayeredThrobber) {
   EXPECT_TRUE(icon->layer());
   data.network_state = TabNetworkState::kNone;
   tab.SetData(data);
-  if (UsingNewLoadingAnimation()) {
-    // The post-loading animation should still be playing (loading bar fades
-    // out).
-    EXPECT_TRUE(icon->ShowingLoadingAnimation());
-    FinishRunningLoadingAnimations(icon);
-  }
+  // 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.
@@ -651,12 +642,9 @@ TEST_F(TabTest, LayeredThrobber) {
   EXPECT_TRUE(icon->layer());
   data.network_state = TabNetworkState::kNone;
   tab.SetData(data);
-  if (UsingNewLoadingAnimation()) {
-    // The post-loading animation should still be playing (loading bar fades
-    // out).
-    EXPECT_TRUE(icon->ShowingLoadingAnimation());
-    FinishRunningLoadingAnimations(icon);
-  }
+  // 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.
@@ -725,8 +713,6 @@ TEST_F(TabTest, LoadingProgressIsFixedTo100PercentWhenNotLoading) {
 }
 
 TEST_F(TabTest, LoadingProgressMonotonicallyIncreases) {
-  if (!UsingNewLoadingAnimation())
-    return;
   Widget widget;
   InitWidget(&widget);
 
@@ -759,9 +745,6 @@ 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_DISABLED_BY_DEFAULT};
+                                            base::FEATURE_ENABLED_BY_DEFAULT};
 #endif  // !OS_ANDROID
 
 #if defined(OS_POSIX)