From 625aaecd20d6dba3fc53191f0b2593e714c3580a Mon Sep 17 00:00:00 2001
From: Mikel Astiz <[email protected]>
Date: Mon, 15 Jun 2026 14:27:02 -0700
Subject: [PATCH] [favicon] Always redownload Web App manifest favicons in incognito

In consistency with how other favicon types are handled, manifest
favicons should always be redownloaded in incognito. This avoids leaking
whether a site was previously visited in regular mode to the incognito
session via cache hit/miss side-channels.

Fixed: 520196753
Change-Id: Ic9d8941023a6a5c0bfb841be0d34e2a80bfa260a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7921032
Commit-Queue: Mikel Astiz <[email protected]>
Reviewed-by: Peter Kotwicz <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1647081}
---

diff --git a/components/favicon/core/favicon_handler.cc b/components/favicon/core/favicon_handler.cc
index ab36630..07c36d72 100644
--- a/components/favicon/core/favicon_handler.cc
+++ b/components/favicon/core/favicon_handler.cc
@@ -392,9 +392,15 @@
   DCHECK(got_favicon_from_history_);
 
   bool has_valid_result = HasValidResult(favicon_bitmap_results);
+  // For off-the-record profiles pretend that favicons from FaviconService are
+  // expired so websites don't know if a site was previously visited in regular
+  // mode. Note however that any cached favicon may still be displayed in the
+  // UI, as there is no privacy downside in showing favicons fetched while the
+  // user was browsing in non-incognito mode.
   bool has_expired_or_incomplete_result =
-      !has_valid_result || HasExpiredOrIncompleteResult(preferred_icon_size(),
-                                                        favicon_bitmap_results);
+      !has_valid_result || delegate_->IsOffTheRecord() ||
+      HasExpiredOrIncompleteResult(preferred_icon_size(),
+                                   favicon_bitmap_results);
 
   if (has_valid_result &&
       (notification_icon_url_ != manifest_url_ ||
@@ -600,7 +606,9 @@
   bool has_valid_result = HasValidResult(favicon_bitmap_results);
   // For off-the-record profiles pretend that favicons from FaviconService are
   // expired so websites don't know if a site was previously visited in regular
-  // mode.
+  // mode. Note however that any cached favicon may still be displayed in the
+  // UI, as there is no privacy downside in showing favicons fetched while the
+  // user was browsing in non-incognito mode.
   initial_history_result_expired_or_incomplete_ =
       !has_valid_result || delegate_->IsOffTheRecord() ||
       HasExpiredOrIncompleteResult(preferred_icon_size(),
diff --git a/components/favicon/core/favicon_handler_unittest.cc b/components/favicon/core/favicon_handler_unittest.cc
index 9043872f..aecae32 100644
--- a/components/favicon/core/favicon_handler_unittest.cc
+++ b/components/favicon/core/favicon_handler_unittest.cc
@@ -1947,6 +1947,33 @@
   EXPECT_THAT(delegate_.downloads(), IsEmpty());
 }
 
+TEST_F(FaviconHandlerManifestsEnabledTest,
+       GetFaviconFromManifestInHistoryInIncognito) {
+  ON_CALL(delegate_, IsOffTheRecord()).WillByDefault(Return(true));
+  favicon_service_.fake()->Store(
+      kPageURL, kManifestURL,
+      CreateRawBitmapResult(kManifestURL, kWebManifestIcon));
+
+  // OnFaviconUpdated should be called with the cached icon initially.
+  EXPECT_CALL(delegate_,
+              OnFaviconUpdated(_, FaviconDriverObserver::TOUCH_LARGEST,
+                               kManifestURL, _, _));
+
+  // Since the fake manifest downloader returns empty results by default, the
+  // handler will fall back to the candidate icon and download it (see below for
+  // more details about why a download is expected).
+  EXPECT_CALL(delegate_,
+              OnFaviconUpdated(_, FaviconDriverObserver::TOUCH_LARGEST,
+                               kIconURL12x12, _, _));
+
+  RunHandlerWithSimpleTouchIconCandidates({kIconURL12x12}, kManifestURL);
+
+  // The handler should have requested the manifest download, although the icon
+  // was cached, because the handler is in incognito mode and cached icons must
+  // be treated as expired to prevent side-channel attacks.
+  EXPECT_THAT(delegate_.downloads(), ElementsAre(kManifestURL, kIconURL12x12));
+}
+
 // Test that a favicon corresponding to a web manifest is reported when:
 // - There is data in the favicon database for the manifest URL.
 // AND

diff --git a/components/favicon/core/favicon_handler_unittest.cc b/components/favicon/core/favicon_handler_unittest.cc
index 9043872f..aecae32 100644
--- a/components/favicon/core/favicon_handler_unittest.cc
+++ b/components/favicon/core/favicon_handler_unittest.cc
@@ -1947,6 +1947,33 @@
   EXPECT_THAT(delegate_.downloads(), IsEmpty());
 }
 
+TEST_F(FaviconHandlerManifestsEnabledTest,
+       GetFaviconFromManifestInHistoryInIncognito) {
+  ON_CALL(delegate_, IsOffTheRecord()).WillByDefault(Return(true));
+  favicon_service_.fake()->Store(
+      kPageURL, kManifestURL,
+      CreateRawBitmapResult(kManifestURL, kWebManifestIcon));
+
+  // OnFaviconUpdated should be called with the cached icon initially.
+  EXPECT_CALL(delegate_,
+              OnFaviconUpdated(_, FaviconDriverObserver::TOUCH_LARGEST,
+                               kManifestURL, _, _));
+
+  // Since the fake manifest downloader returns empty results by default, the
+  // handler will fall back to the candidate icon and download it (see below for
+  // more details about why a download is expected).
+  EXPECT_CALL(delegate_,
+              OnFaviconUpdated(_, FaviconDriverObserver::TOUCH_LARGEST,
+                               kIconURL12x12, _, _));
+
+  RunHandlerWithSimpleTouchIconCandidates({kIconURL12x12}, kManifestURL);
+
+  // The handler should have requested the manifest download, although the icon
+  // was cached, because the handler is in incognito mode and cached icons must
+  // be treated as expired to prevent side-channel attacks.
+  EXPECT_THAT(delegate_.downloads(), ElementsAre(kManifestURL, kIconURL12x12));
+}
+
 // Test that a favicon corresponding to a web manifest is reported when:
 // - There is data in the favicon database for the manifest URL.
 // AND