Tab Search: Add Tab.favicon_url for OTR profile

We switched to use chrome://favicon2 from base64 stream for favicon.
But for OTR profile chrome://favicon2 is not available. This CL fixes
favicon not showing up correctly in guest mode.

Related CL: https://chromium-review.googlesource.com/c/chromium/src/+/2406932

Bug: 1099917
Change-Id: I5fdd1dda997d08024d6ce3aa6d6a569d38ac026e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2424510
Commit-Queue: Yuheng Huang <yuhengh@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Thomas Lukaszewicz <tluk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#810287}

chrome/browser/ui/webui/tab_search/tab_search.mojom

@@ -19,15 +19,38 @@ struct WindowTabs {
 
 // Information about an existing open tab.
 struct Tab {
+  // Whether the tab is active.
   bool active;
+
+  // The index of the tab in the current tab strip.
   int32 index;
+
+  // The unique identifier of the tab.
   int32 tab_id;
+
+  // The group identifier of the tab.
   string? group_id;
+
+  // Whether the tab is pinned.
   bool pinned;
+
+  // The title of the tab.
   string title;
+
+  // The URL of the tab.
   string url;
+
+  // The URL of the favicon in data scheme.
+  // Only used in OTR profile where chrome://favicon2 is not available.
+  string? favicon_url;
+
+  // Whether the favicon is the default one.
   bool is_default_favicon;
+
+  // Whether the tab strip should show the icon.
   bool show_icon;
+
+  // Time ticks when the tab is last active.
   mojo_base.mojom.TimeTicks last_active_time_ticks;
 };
 

chrome/browser/ui/webui/tab_search/tab_search_page_handler.cc

@@ -16,6 +16,7 @@
 #include "base/trace_event/trace_event.h"
 #include "chrome/browser/extensions/extension_tab_util.h"
 #include "chrome/browser/favicon/favicon_utils.h"
+#include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_finder.h"
 #include "chrome/browser/ui/browser_list.h"
@@ -32,10 +33,12 @@ constexpr base::TimeDelta kTabsChangeDelay =
 TabSearchPageHandler::TabSearchPageHandler(
     mojo::PendingReceiver<tab_search::mojom::PageHandler> receiver,
     mojo::PendingRemote<tab_search::mojom::Page> page,
+    content::WebUI* web_ui,
     Delegate* delegate)
     : receiver_(this, std::move(receiver)),
       page_(std::move(page)),
       browser_(chrome::FindLastActive()),
+      web_ui_(web_ui),
       delegate_(delegate),
       debounce_timer_(std::make_unique<base::RetainingOneShotTimer>(
           FROM_HERE,
@@ -182,6 +185,12 @@ tab_search::mojom::TabPtr TabSearchPageHandler::GetTabData(
   if (tab_renderer_data.favicon.isNull()) {
     tab_data->is_default_favicon = true;
   } else {
+    // Only send favicon_url for OTR profile where chrome://favicon2 is not
+    // available.
+    if (browser_->profile()->IsOffTheRecord()) {
+      tab_data->favicon_url = webui::EncodePNGAndMakeDataURI(
+          tab_renderer_data.favicon, web_ui_->GetDeviceScaleFactor());
+    }
     tab_data->is_default_favicon =
         tab_renderer_data.favicon.BackedBySameObjectAs(
             favicon::GetDefaultFavicon().AsImageSkia());

chrome/browser/ui/webui/tab_search/tab_search_page_handler.h

@@ -42,6 +42,7 @@ class TabSearchPageHandler : public tab_search::mojom::PageHandler,
   TabSearchPageHandler(
       mojo::PendingReceiver<tab_search::mojom::PageHandler> receiver,
       mojo::PendingRemote<tab_search::mojom::Page> page,
+      content::WebUI* web_ui,
       Delegate* delegate);
   TabSearchPageHandler(const TabSearchPageHandler&) = delete;
   TabSearchPageHandler& operator=(const TabSearchPageHandler&) = delete;
@@ -99,6 +100,7 @@ class TabSearchPageHandler : public tab_search::mojom::PageHandler,
   mojo::Receiver<tab_search::mojom::PageHandler> receiver_;
   mojo::Remote<tab_search::mojom::Page> page_;
   Browser* const browser_;
+  content::WebUI* const web_ui_;
   Delegate* const delegate_;
   BrowserTabStripTracker browser_tab_strip_tracker_{this, this};
   std::unique_ptr<base::RetainingOneShotTimer> debounce_timer_;

chrome/browser/ui/webui/tab_search/tab_search_page_handler_unittest.cc

@@ -60,6 +60,7 @@ void ExpectNewTab(const tab_search::mojom::Tab* tab,
   EXPECT_FALSE(tab->pinned);
   EXPECT_EQ(title, tab->title);
   EXPECT_EQ(url, tab->url);
+  EXPECT_FALSE(tab->favicon_url.has_value());
   EXPECT_TRUE(tab->is_default_favicon);
   EXPECT_TRUE(tab->show_icon);
   EXPECT_GT(tab->last_active_time_ticks, base::TimeTicks());
@@ -79,10 +80,12 @@ void ExpectProfileTabs(tab_search::mojom::ProfileTabs* profile_tabs) {
 class TestTabSearchPageHandler : public TabSearchPageHandler {
  public:
   TestTabSearchPageHandler(mojo::PendingRemote<tab_search::mojom::Page> page,
+                           content::WebUI* web_ui,
                            TabSearchPageHandler::Delegate* delegate)
       : TabSearchPageHandler(
             mojo::PendingReceiver<tab_search::mojom::PageHandler>(),
             std::move(page),
+            web_ui,
             delegate) {
     mock_debounce_timer_ = new base::MockRetainingOneShotTimer();
     SetTimerForTesting(base::WrapUnique(mock_debounce_timer_));
@@ -118,7 +121,7 @@ class TabSearchPageHandlerTest : public BrowserWithTestWindowTest {
     BrowserList::SetLastActive(browser1());
     handler_delegate_ = std::make_unique<MockTabSearchPageHandlerDelegate>();
     handler_ = std::make_unique<TestTabSearchPageHandler>(
-        page_.BindAndGetRemote(), handler_delegate_.get());
+        page_.BindAndGetRemote(), web_ui(), handler_delegate_.get());
   }
 
   void TearDown() override {
@@ -357,4 +360,23 @@ TEST_F(TabSearchPageHandlerTest, CloseUITest) {
   handler()->CloseUI();
 }
 
+// OTR profile should have a non-empty base64 favicon_url for each tab
+TEST_F(TabSearchPageHandlerTest, OTRProfileFaviconTest) {
+  AddTabWithTitle(browser3(), GURL(kTabUrl1), kTabName1);
+  BrowserList::SetLastActive(browser3());
+  ASSERT_TRUE(browser3()->profile()->IsOffTheRecord());
+  tab_search::mojom::PageHandler::GetProfileTabsCallback callback =
+      base::BindLambdaForTesting(
+          [&](tab_search::mojom::ProfileTabsPtr profile_tabs) {
+            ASSERT_EQ(1u, profile_tabs->windows.size());
+            auto* window1 = profile_tabs->windows[0].get();
+            ASSERT_EQ(1u, window1->tabs.size());
+            ASSERT_TRUE(window1->tabs[0]->favicon_url.has_value());
+          });
+  testing::StrictMock<MockPage> page;
+  auto handler = std::make_unique<TestTabSearchPageHandler>(
+      page.BindAndGetRemote(), web_ui(), handler_delegate());
+  handler->GetProfileTabs(std::move(callback));
+}
+
 }  // namespace

chrome/browser/ui/webui/tab_search/tab_search_ui.cc

@@ -104,6 +104,6 @@ void TabSearchUI::CreatePageHandler(
     mojo::PendingRemote<tab_search::mojom::Page> page,
     mojo::PendingReceiver<tab_search::mojom::PageHandler> receiver) {
   DCHECK(page);
-  page_handler_ = std::make_unique<TabSearchPageHandler>(std::move(receiver),
-                                                         std::move(page), this);
+  page_handler_ = std::make_unique<TabSearchPageHandler>(
+      std::move(receiver), std::move(page), web_ui(), this);
 }