Fix handling of touch icons when it is disabled

Store the icon type in a bitfield because the same icon URL might be
used for various types on same page. This way webengine won't ignore to
download a default icon what is also set as a touch icon when touch
icons are disabled.

Moreover, do not store the icon types from the previous page because
a subsequent page might use the same icon URL but with different type.
With this change the type of the cached icons are updated after each
page load.

Fixes: QTBUG-70081
Change-Id: I8031a740b07b0c6a8e5759a994f386b13ce87be2
Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>

3 files changed, 23 insertions, 14 deletions

diff --git a/src/core/favicon_manager.cpp b/src/core/favicon_manager.cpp
index de6a0f183..f7ba858c1 100644
--- a/src/core/favicon_manager.cpp
+++ b/src/core/favicon_manager.cpp
@@ -253,7 +253,7 @@ void FaviconManager::update(const QList<FaviconInfo> &candidates)
 
     const QList<FaviconInfo> &faviconInfoList = getFaviconInfoList(true /* candidates only */);
     for (auto it = faviconInfoList.cbegin(), end = faviconInfoList.cend(); it != end; ++it) {
-        if (!touchIconsEnabled && it->type != FaviconInfo::Favicon)
+        if (!touchIconsEnabled && !(it->type & FaviconInfo::Favicon))
             continue;
 
         if (it->isValid())
@@ -272,6 +272,14 @@ void FaviconManager::update(const QList<FaviconInfo> &candidates)
 
 void FaviconManager::updateCandidates(const QList<FaviconInfo> &candidates)
 {
+    // Invalidate types of the already stored candidate icons because it might differ
+    // among pages.
+    for (FaviconInfo candidateFaviconInfo : candidates) {
+        const QUrl &candidateUrl = candidateFaviconInfo.url;
+        if (m_faviconInfoMap.contains(candidateUrl))
+            m_faviconInfoMap[candidateUrl].type = FaviconInfo::InvalidIcon;
+    }
+
     m_candidateCount = candidates.count();
     for (FaviconInfo candidateFaviconInfo : candidates) {
         const QUrl &candidateUrl = candidateFaviconInfo.url;
@@ -279,8 +287,8 @@ void FaviconManager::updateCandidates(const QList<FaviconInfo> &candidates)
         if (!m_faviconInfoMap.contains(candidateUrl))
             m_faviconInfoMap.insert(candidateUrl, candidateFaviconInfo);
         else {
-            // The same icon can be used for more than one page with different types.
-            m_faviconInfoMap[candidateUrl].type = candidateFaviconInfo.type;
+            // The same icon URL can be used for different types.
+            m_faviconInfoMap[candidateUrl].type |= candidateFaviconInfo.type;
         }
 
         m_faviconInfoMap[candidateUrl].candidate = true;
@@ -311,7 +319,7 @@ QUrl FaviconManager::candidateIconUrl(bool touchIconsEnabled) const
 
     unsigned bestArea = 0;
     for (auto it = faviconInfoList.cbegin(), end = faviconInfoList.cend(); it != end; ++it) {
-        if (!touchIconsEnabled && it->type != FaviconInfo::Favicon)
+        if (!touchIconsEnabled && !(it->type & FaviconInfo::Favicon))
             continue;
 
         if (it->isValid() && bestArea < area(it->size)) {
@@ -331,7 +339,7 @@ void FaviconManager::generateCandidateIcon(bool touchIconsEnabled)
     const QList<FaviconInfo> &faviconInfoList = getFaviconInfoList(true /* candidates only */);
 
     for (auto it = faviconInfoList.cbegin(), end = faviconInfoList.cend(); it != end; ++it) {
-        if (!touchIconsEnabled && it->type != FaviconInfo::Favicon)
+        if (!touchIconsEnabled && !(it->type & FaviconInfo::Favicon))
             continue;
 
         if (!it->isValid() || !it->isDownloaded())
@@ -373,7 +381,7 @@ FaviconInfo::FaviconInfo(const FaviconInfo &other)
 {
 }
 
-FaviconInfo::FaviconInfo(const QUrl &url, FaviconInfo::FaviconType type)
+FaviconInfo::FaviconInfo(const QUrl &url, FaviconInfo::FaviconTypeFlags type)
     : url(url)
     , type(type)
     , size(QSize(0, 0))
diff --git a/src/core/favicon_manager.h b/src/core/favicon_manager.h
index f9758d0f0..60d194c4b 100644
--- a/src/core/favicon_manager.h
+++ b/src/core/favicon_manager.h
@@ -84,23 +84,24 @@ class WebContentsAdapterClient;
 // Based on src/3rdparty/chromium/content/public/common/favicon_url.h
 class QWEBENGINECORE_PRIVATE_EXPORT FaviconInfo {
 public:
-    enum FaviconType {
-        InvalidIcon,
-        Favicon,
-        TouchIcon,
-        TouchPrecomposedIcon
+    enum FaviconTypeFlag {
+        InvalidIcon = 0,
+        Favicon = 1 << 0,
+        TouchIcon = 1 << 1,
+        TouchPrecomposedIcon = 1 << 2
     };
+    Q_DECLARE_FLAGS(FaviconTypeFlags, FaviconTypeFlag);
 
     FaviconInfo();
     FaviconInfo(const FaviconInfo &);
-    FaviconInfo(const QUrl &, FaviconInfo::FaviconType);
+    FaviconInfo(const QUrl &, FaviconInfo::FaviconTypeFlags);
     ~FaviconInfo();
 
     bool isValid() const;
     bool isDownloaded() const;
 
     QUrl url;
-    FaviconType type;
+    FaviconTypeFlags type;
     // Stores the largest size in case of multi-size icon
     QSize size;
     bool candidate;
diff --git a/src/core/type_conversion.cpp b/src/core/type_conversion.cpp
index d35426ac3..f69b2a297 100644
--- a/src/core/type_conversion.cpp
+++ b/src/core/type_conversion.cpp
@@ -216,7 +216,7 @@ int flagsFromModifiers(Qt::KeyboardModifiers modifiers)
     return modifierFlags;
 }
 
-FaviconInfo::FaviconType toQt(content::FaviconURL::IconType type)
+FaviconInfo::FaviconTypeFlags toQt(content::FaviconURL::IconType type)
 {
     switch (type) {
     case content::FaviconURL::IconType::kFavicon: