Android: Ensure favicon on preview tabs gets updated properly

This CL fixes a bug that does not update the favicon when
navigation takes places in the preview tab. Preview tab overrides
OverlayContentDelegate to receive the event for navigation and
updates URL accordingly.

For this to work, Ephemeral Tab should observe
FaviconDriverObserver to get notified of a new favicon in case
the navigated page is new, with no corresponding favicon
available in a local db. This also fixes another bug that doesn't
display the right favicon for any new pages on it.

Bug: 968364
Change-Id: I75ab428f9bcc42c7d28bc832a858d2707d1b1c4e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1638305
Commit-Queue: Jinsuk Kim <jinsukkim@chromium.org>
Reviewed-by: Donn Denman <donnd@chromium.org>
Reviewed-by: Matthew Jones <mdjones@chromium.org>
Cr-Commit-Position: refs/heads/master@{#665313}

chrome/android/java/src/org/chromium/chrome/browser/compositor/bottombar/ephemeraltab/EphemeralTabPanel.java

@@ -107,11 +107,18 @@ public class EphemeralTabPanel extends OverlayPanel {
         stopListeningForCloseConditions();
     }
 
+    private class EphemeralTabPanelContentDelegate extends OverlayContentDelegate {
+        @Override
+        public void onMainFrameNavigation(String url, boolean isExternalUrl, boolean isFailure) {
+            if (!isFailure) mUrl = url;
+        }
+    }
+
     @Override
     public OverlayPanelContent createNewOverlayPanelContent() {
         if (mTabModelObserver == null) startListeningForCloseConditions();
-        return new OverlayPanelContent(new OverlayContentDelegate(), new PanelProgressObserver(),
-                mActivity, mIsIncognito, getBarHeight());
+        return new OverlayPanelContent(new EphemeralTabPanelContentDelegate(),
+                new PanelProgressObserver(), mActivity, mIsIncognito, getBarHeight());
     }
 
     private void startListeningForCloseConditions() {

chrome/browser/android/compositor/layer/ephemeral_tab_layer.cc

@@ -9,7 +9,9 @@
 #include "cc/layers/ui_resource_layer.h"
 #include "chrome/browser/favicon/favicon_service_factory.h"
 #include "chrome/browser/profiles/profile.h"
+#include "components/favicon/content/content_favicon_driver.h"
 #include "components/favicon/core/favicon_service.h"
+#include "content/public/browser/web_contents.h"
 #include "third_party/skia/include/core/SkBitmap.h"
 #include "ui/android/resources/resource_manager.h"
 #include "ui/base/l10n/l10n_util_android.h"
@@ -18,6 +20,26 @@
 
 namespace {
 
+void DisplayFavicon(scoped_refptr<cc::UIResourceLayer> layer,
+                    scoped_refptr<cc::UIResourceLayer> icon_layer,
+                    const SkBitmap& favicon,
+                    const float dp_to_px,
+                    const float panel_width,
+                    const float bar_height,
+                    const float padding) {
+  const float icon_width =
+      android::OverlayPanelLayer::kDefaultIconWidthDp * dp_to_px;
+  layer->SetBitmap(favicon);
+  layer->SetBounds(gfx::Size(icon_width, icon_width));
+
+  bool is_rtl = l10n_util::IsLayoutRtl();
+  float icon_x = is_rtl ? panel_width - icon_width - padding : padding;
+  float icon_y = (bar_height - layer->bounds().height()) / 2;
+  icon_layer->SetIsDrawable(false);
+  layer->SetIsDrawable(true);
+  layer->SetPosition(gfx::PointF(icon_x, icon_y));
+}
+
 void OnLocalFaviconAvailable(
     scoped_refptr<cc::UIResourceLayer> layer,
     scoped_refptr<cc::UIResourceLayer> icon_layer,
@@ -33,18 +55,9 @@ void OnLocalFaviconAvailable(
                         &favicon_bitmap);
   if (favicon_bitmap.isNull())
     return;
-  const float icon_width =
-      android::OverlayPanelLayer::kDefaultIconWidthDp * dp_to_px;
   favicon_bitmap.setImmutable();
-  layer->SetBitmap(favicon_bitmap);
-  layer->SetBounds(gfx::Size(icon_width, icon_width));
-
-  bool is_rtl = l10n_util::IsLayoutRtl();
-  float icon_x = is_rtl ? panel_width - icon_width - padding : padding;
-  float icon_y = (bar_height - layer->bounds().height()) / 2;
-  icon_layer->SetIsDrawable(false);
-  layer->SetIsDrawable(true);
-  layer->SetPosition(gfx::PointF(icon_x, icon_y));
+  DisplayFavicon(layer, icon_layer, favicon_bitmap, dp_to_px, panel_width,
+                 bar_height, padding);
 }
 
 }  // namespace
@@ -57,6 +70,7 @@ scoped_refptr<EphemeralTabLayer> EphemeralTabLayer::Create(
 }
 
 void EphemeralTabLayer::SetProperties(
+    content::WebContents* web_contents,
     int title_view_resource_id,
     int caption_view_resource_id,
     jfloat caption_animation_percentage,
@@ -85,6 +99,18 @@ void EphemeralTabLayer::SetProperties(
     float progress_bar_height,
     float progress_bar_opacity,
     int progress_bar_completion) {
+  if (web_contents_ != web_contents) {
+    web_contents_ = web_contents;
+    if (web_contents_) {
+      auto* favicon_driver =
+          favicon::ContentFaviconDriver::FromWebContents(web_contents_);
+      if (favicon_driver)
+        favicon_driver->AddObserver(this);
+      // No need to remove the observer from the previous WebContents since
+      // it is already destroyed by the time it reaches this point.
+    }
+  }
+
   // Round values to avoid pixel gap between layers.
   bar_height = floor(bar_height);
   float bar_top = 0.f;
@@ -215,6 +241,26 @@ void EphemeralTabLayer::SetupTextLayer(float bar_top,
   caption_->SetPosition(gfx::PointF(0.f, caption_top));
 }
 
+void EphemeralTabLayer::OnFaviconUpdated(
+    favicon::FaviconDriver* favicon_driver,
+    NotificationIconType notification_icon_type,
+    const GURL& icon_url,
+    bool icon_url_changed,
+    const gfx::Image& image) {
+  if (notification_icon_type != NON_TOUCH_LARGEST &&
+      notification_icon_type != TOUCH_LARGEST) {
+    return;
+  }
+
+  SkBitmap favicon_bitmap =
+      image.AsImageSkia().GetRepresentation(1.0f).GetBitmap();
+  if (favicon_bitmap.empty())
+    return;
+  favicon_bitmap.setImmutable();
+  DisplayFavicon(favicon_layer_, panel_icon_, favicon_bitmap, dp_to_px_,
+                 panel_width_, bar_height_, bar_margin_side_);
+}
+
 void EphemeralTabLayer::GetLocalFaviconImageForURL(Profile* profile,
                                                    const std::string& url,
                                                    int size) {

chrome/browser/android/compositor/layer/ephemeral_tab_layer.h

@@ -7,6 +7,8 @@
 
 #include "chrome/browser/android/compositor/layer/overlay_panel_layer.h"
 
+#include "components/favicon/core/favicon_driver_observer.h"
+
 class Profile;
 
 namespace base {
@@ -17,16 +19,26 @@ namespace cc {
 class Layer;
 }
 
+namespace content {
+class WebContents;
+}
+
+namespace favicon {
+class FaviconDriver;
+}
+
 namespace ui {
 class ResourceManager;
 }
 
 namespace android {
-class EphemeralTabLayer : public OverlayPanelLayer {
+class EphemeralTabLayer : public OverlayPanelLayer,
+                          public favicon::FaviconDriverObserver {
  public:
   static scoped_refptr<EphemeralTabLayer> Create(
       ui::ResourceManager* resource_manager);
-  void SetProperties(int title_view_resource_id,
+  void SetProperties(content::WebContents* web_contents,
+                     int title_view_resource_id,
                      int caption_view_resource_id,
                      jfloat caption_animation_percentage,
                      jfloat text_layer_min_height,
@@ -67,11 +79,19 @@ class EphemeralTabLayer : public OverlayPanelLayer {
                                   const std::string& url,
                                   int size);
 
+  // favicon::FaviconDriverObserver
+  void OnFaviconUpdated(favicon::FaviconDriver* favicon_driver,
+                        NotificationIconType notification_icon_type,
+                        const GURL& icon_url,
+                        bool icon_url_changed,
+                        const gfx::Image& image) override;
+
  protected:
   explicit EphemeralTabLayer(ui::ResourceManager* resource_manager);
   ~EphemeralTabLayer() override;
 
  private:
+  content::WebContents* web_contents_ = nullptr;
   float dp_to_px_;
   float panel_width_;
   float bar_height_;

chrome/browser/android/compositor/scene_layer/ephemeral_tab_scene_layer.cc

@@ -124,7 +124,7 @@ void EphemeralTabSceneLayer::Update(JNIEnv* env,
   // Move the base page contents up.
   content_container_->SetPosition(gfx::PointF(0.0f, base_page_offset));
   ephemeral_tab_layer_->SetProperties(
-      title_view_resource_id, caption_view_resource_id,
+      web_contents, title_view_resource_id, caption_view_resource_id,
       caption_animation_percentage, text_layer_min_height,
       title_caption_spacing, caption_visible,
       progress_bar_background_resource_id, progress_bar_resource_id, dp_to_px,