Do not fade in favicon before page commit

Prevents showing cached favicons for the previous page during a timing
bug while it's connecting to the new page.

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

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

@@ -110,7 +110,8 @@ void TabIcon::SetData(const TabRendererData& data) {
   const bool was_showing_load = ShowingLoadingAnimation();
 
   inhibit_loading_animation_ = data.should_hide_throbber;
-  SetIcon(data.url, data.favicon);
+  SetIcon(data.url, data.favicon,
+          data.network_state == TabNetworkState::kWaiting);
   SetNetworkState(data.network_state, data.load_progress);
   SetIsCrashed(data.IsCrashed());
 
@@ -430,19 +431,28 @@ bool TabIcon::MaybePaintFavicon(gfx::Canvas* canvas,
   return true;
 }
 
-void TabIcon::SetIcon(const GURL& url, const gfx::ImageSkia& icon) {
+bool TabIcon::HasNonDefaultFavicon() const {
+  ui::ResourceBundle& rb = ui::ResourceBundle::GetSharedInstance();
+  return !favicon_.isNull() && !favicon_.BackedBySameObjectAs(
+                                   *rb.GetImageSkiaNamed(IDR_DEFAULT_FAVICON));
+}
+
+void TabIcon::SetIcon(const GURL& url,
+                      const gfx::ImageSkia& icon,
+                      bool tab_is_waiting) {
   // Detect when updating to the same icon. This avoids re-theming and
   // re-painting.
   if (favicon_.BackedBySameObjectAs(icon))
     return;
-  favicon_ = icon;
 
-  ui::ResourceBundle& rb = ui::ResourceBundle::GetSharedInstance();
-  const bool is_default_favicon =
-      icon.BackedBySameObjectAs(*rb.GetImageSkiaNamed(IDR_DEFAULT_FAVICON));
+  favicon_ = icon;
+  const bool is_default_favicon = !HasNonDefaultFavicon();
 
-  if (!is_default_favicon && !pending_animation_state_.favicon_fade_in_progress)
+  // Start fading in non-default favicons if we're not in the waiting state.
+  if (!is_default_favicon && !tab_is_waiting &&
+      !pending_animation_state_.favicon_fade_in_progress) {
     pending_animation_state_.favicon_fade_in_progress = 0.0;
+  }
 
   if (is_default_favicon || ShouldThemifyFaviconForUrl(url)) {
     themed_favicon_ = ThemeImage(icon);
@@ -468,7 +478,8 @@ void TabIcon::SetNetworkState(TabNetworkState network_state,
       pending_animation_state_.favicon_fade_in_progress.reset();
     }
 
-    if (!is_animated) {
+    if (!is_animated || (network_state_ == TabNetworkState::kLoading &&
+                         HasNonDefaultFavicon())) {
       // Start fading in the favicon if we're no longer animating.
       if (!pending_animation_state_.favicon_fade_in_progress)
         pending_animation_state_.favicon_fade_in_progress = 0.0;

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

@@ -114,9 +114,14 @@ class TabIcon : public views::View {
   bool MaybePaintFavicon(gfx::Canvas* canvas,
                          const gfx::ImageSkia& icon,
                          const gfx::Rect& bounds);
+  bool HasNonDefaultFavicon() const;
 
   // Sets the icon. Depending on the URL the icon may be automatically themed.
-  void SetIcon(const GURL& url, const gfx::ImageSkia& favicon);
+  // The |tab_is_waiting| flag is used to check if we're allowed to start fading
+  // in the new icon.
+  void SetIcon(const GURL& url,
+               const gfx::ImageSkia& favicon,
+               bool tab_is_waiting);
 
   // For certain types of tabs the loading animation is not desired so the
   // caller can set inhibit_loading_animation to true. When false, the loading