Commit 355b8185 authored by Peter Boström's avatar Peter Boström Committed by Commit Bot

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/1364212Reviewed-by: default avatarSidney San Martín <sdy@chromium.org>
Commit-Queue: Peter Boström <pbos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#614199}
parent 68aeecd7
...@@ -151,6 +151,9 @@ bool TabIcon::ShowingLoadingAnimation() const { ...@@ -151,6 +151,9 @@ bool TabIcon::ShowingLoadingAnimation() const {
if (NetworkStateIsAnimated(network_state_)) if (NetworkStateIsAnimated(network_state_))
return true; return true;
if (!UseNewLoadingAnimation())
return false;
if (LoadingAnimationNeedsRepaint()) if (LoadingAnimationNeedsRepaint())
return true; return true;
...@@ -182,14 +185,20 @@ void TabIcon::SetCanPaintToLayer(bool can_paint_to_layer) { ...@@ -182,14 +185,20 @@ void TabIcon::SetCanPaintToLayer(bool can_paint_to_layer) {
} }
void TabIcon::StepLoadingAnimation(const base::TimeDelta& elapsed_time) { 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(); UpdatePendingAnimationState();
if (LoadingAnimationNeedsRepaint()) if (LoadingAnimationNeedsRepaint())
SchedulePaint(); 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) { void TabIcon::SetBackgroundColor(SkColor bg_color) {
...@@ -437,6 +446,8 @@ const gfx::ImageSkia& TabIcon::GetIconToPaint() { ...@@ -437,6 +446,8 @@ const gfx::ImageSkia& TabIcon::GetIconToPaint() {
void TabIcon::MaybePaintFaviconPlaceholder(gfx::Canvas* canvas, void TabIcon::MaybePaintFaviconPlaceholder(gfx::Canvas* canvas,
const gfx::Rect& bounds) { const gfx::Rect& bounds) {
if (!UseNewLoadingAnimation())
return;
if (!animation_state_.favicon_placeholder_alpha) if (!animation_state_.favicon_placeholder_alpha)
return; return;
cc::PaintFlags flags; cc::PaintFlags flags;
...@@ -458,14 +469,16 @@ void TabIcon::MaybePaintFavicon(gfx::Canvas* canvas, ...@@ -458,14 +469,16 @@ void TabIcon::MaybePaintFavicon(gfx::Canvas* canvas,
const gfx::Rect& bounds) { const gfx::Rect& bounds) {
// While loading, the favicon (or placeholder) isn't drawn until it has // While loading, the favicon (or placeholder) isn't drawn until it has
// started fading in. // started fading in.
if (!animation_state_.favicon_fade_in_progress) if (UseNewLoadingAnimation() && !animation_state_.favicon_fade_in_progress)
return; return;
if (icon.isNull()) if (icon.isNull())
return; return;
cc::PaintFlags flags; 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); flags.setAlpha(fade_in_progress * SK_AlphaOPAQUE);
// Drop in the new favicon from the top while it's fading in. // Drop in the new favicon from the top while it's fading in.
const int offset = round((fade_in_progress - 1.0) * 4.0); const int offset = round((fade_in_progress - 1.0) * 4.0);
...@@ -482,6 +495,9 @@ bool TabIcon::HasNonDefaultFavicon() const { ...@@ -482,6 +495,9 @@ bool TabIcon::HasNonDefaultFavicon() const {
} }
void TabIcon::MaybeStartFaviconFadeIn() { void TabIcon::MaybeStartFaviconFadeIn() {
if (!UseNewLoadingAnimation())
return;
if (pending_animation_state_.favicon_fade_in_progress) if (pending_animation_state_.favicon_fade_in_progress)
return; return;
...@@ -521,6 +537,11 @@ void TabIcon::SetIcon(const GURL& url, const gfx::ImageSkia& icon) { ...@@ -521,6 +537,11 @@ void TabIcon::SetIcon(const GURL& url, const gfx::ImageSkia& icon) {
void TabIcon::SetNetworkState(TabNetworkState network_state, void TabIcon::SetNetworkState(TabNetworkState network_state,
float load_progress) { float load_progress) {
if (!UseNewLoadingAnimation()) {
network_state_ = network_state;
return;
}
if (network_state_ != network_state) { if (network_state_ != network_state) {
TabNetworkState old_state = network_state_; TabNetworkState old_state = network_state_;
network_state_ = network_state; network_state_ = network_state;
......
...@@ -20,6 +20,7 @@ ...@@ -20,6 +20,7 @@
#include "chrome/browser/ui/views/tabs/tab_icon.h" #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_strip.h"
#include "chrome/browser/ui/views/tabs/tab_style.h" #include "chrome/browser/ui/views/tabs/tab_style.h"
#include "chrome/common/chrome_features.h"
#include "chrome/grit/theme_resources.h" #include "chrome/grit/theme_resources.h"
#include "chrome/test/views/chrome_views_test_base.h" #include "chrome/test/views/chrome_views_test_base.h"
#include "testing/gtest/include/gtest/gtest.h" #include "testing/gtest/include/gtest/gtest.h"
...@@ -33,6 +34,12 @@ ...@@ -33,6 +34,12 @@
using views::Widget; using views::Widget;
namespace {
bool UsingNewLoadingAnimation() {
return base::FeatureList::IsEnabled(features::kNewTabLoadingAnimation);
}
} // namespace
class FakeTabController : public TabController { class FakeTabController : public TabController {
public: public:
FakeTabController() {} FakeTabController() {}
...@@ -609,10 +616,12 @@ TEST_F(TabTest, LayeredThrobber) { ...@@ -609,10 +616,12 @@ TEST_F(TabTest, LayeredThrobber) {
EXPECT_TRUE(icon->layer()); EXPECT_TRUE(icon->layer());
data.network_state = TabNetworkState::kNone; data.network_state = TabNetworkState::kNone;
tab.SetData(data); tab.SetData(data);
// The post-loading animation should still be playing (loading bar fades out). if (UsingNewLoadingAnimation()) {
EXPECT_TRUE(icon->ShowingLoadingAnimation()); // The post-loading animation should still be playing (loading bar fades
// out).
FinishRunningLoadingAnimations(icon); EXPECT_TRUE(icon->ShowingLoadingAnimation());
FinishRunningLoadingAnimations(icon);
}
EXPECT_FALSE(icon->ShowingLoadingAnimation()); EXPECT_FALSE(icon->ShowingLoadingAnimation());
// Simulate a tab that should hide throbber. // Simulate a tab that should hide throbber.
...@@ -642,9 +651,12 @@ TEST_F(TabTest, LayeredThrobber) { ...@@ -642,9 +651,12 @@ TEST_F(TabTest, LayeredThrobber) {
EXPECT_TRUE(icon->layer()); EXPECT_TRUE(icon->layer());
data.network_state = TabNetworkState::kNone; data.network_state = TabNetworkState::kNone;
tab.SetData(data); tab.SetData(data);
// The post-loading animation should still be playing (loading bar fades out). if (UsingNewLoadingAnimation()) {
EXPECT_TRUE(icon->ShowingLoadingAnimation()); // The post-loading animation should still be playing (loading bar fades
FinishRunningLoadingAnimations(icon); // out).
EXPECT_TRUE(icon->ShowingLoadingAnimation());
FinishRunningLoadingAnimations(icon);
}
EXPECT_FALSE(icon->ShowingLoadingAnimation()); EXPECT_FALSE(icon->ShowingLoadingAnimation());
// After loading is done, simulate another resource starting to load. // After loading is done, simulate another resource starting to load.
...@@ -713,6 +725,8 @@ TEST_F(TabTest, LoadingProgressIsFixedTo100PercentWhenNotLoading) { ...@@ -713,6 +725,8 @@ TEST_F(TabTest, LoadingProgressIsFixedTo100PercentWhenNotLoading) {
} }
TEST_F(TabTest, LoadingProgressMonotonicallyIncreases) { TEST_F(TabTest, LoadingProgressMonotonicallyIncreases) {
if (!UsingNewLoadingAnimation())
return;
Widget widget; Widget widget;
InitWidget(&widget); InitWidget(&widget);
...@@ -745,6 +759,9 @@ TEST_F(TabTest, LoadingProgressMonotonicallyIncreases) { ...@@ -745,6 +759,9 @@ TEST_F(TabTest, LoadingProgressMonotonicallyIncreases) {
} }
TEST_F(TabTest, LoadingProgressGoesTo100PercentAfterLoadingIsDone) { TEST_F(TabTest, LoadingProgressGoesTo100PercentAfterLoadingIsDone) {
if (!UsingNewLoadingAnimation())
return;
Widget widget; Widget widget;
InitWidget(&widget); InitWidget(&widget);
......
...@@ -387,7 +387,7 @@ const char kNewNetErrorPageUIAlternateContentPreview[] = "content_preview"; ...@@ -387,7 +387,7 @@ const char kNewNetErrorPageUIAlternateContentPreview[] = "content_preview";
#if !defined(OS_ANDROID) #if !defined(OS_ANDROID)
const base::Feature kNewTabLoadingAnimation{"NewTabLoadingAnimation", const base::Feature kNewTabLoadingAnimation{"NewTabLoadingAnimation",
base::FEATURE_ENABLED_BY_DEFAULT}; base::FEATURE_DISABLED_BY_DEFAULT};
#endif // !OS_ANDROID #endif // !OS_ANDROID
#if defined(OS_POSIX) #if defined(OS_POSIX)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment