From f658dcb0893e7869cc0c029bfe0be82838af4bb8 Mon Sep 17 00:00:00 2001 From: Peter Varga Date: Sat, 27 Feb 2016 22:59:51 +0100 Subject: Clean up FaviconManager and fix icon url in NavigationEntry Change-Id: I56a109c9071ef581c6a51b5b7b8ce5a2464c6a76 Task-number: QTBUG-51179 Reviewed-by: Allan Sandfeld Jensen --- src/core/favicon_manager.cpp | 183 ++++++++++++---------------------- src/core/favicon_manager.h | 14 +-- src/core/favicon_manager_p.h | 6 +- src/core/web_contents_delegate_qt.cpp | 20 +--- 4 files changed, 75 insertions(+), 148 deletions(-) (limited to 'src') diff --git a/src/core/favicon_manager.cpp b/src/core/favicon_manager.cpp index 6cbb8bc1c..8469f054e 100644 --- a/src/core/favicon_manager.cpp +++ b/src/core/favicon_manager.cpp @@ -44,6 +44,8 @@ #include "web_contents_adapter_client.h" #include "base/bind.h" +#include "content/public/browser/favicon_status.h" +#include "content/public/browser/navigation_entry.h" #include "content/public/browser/web_contents.h" #include "third_party/skia/include/core/SkBitmap.h" #include "third_party/skia/include/core/SkPixelRef.h" @@ -76,12 +78,12 @@ FaviconManagerPrivate::~FaviconManagerPrivate() { } -int FaviconManagerPrivate::downloadIcon(const QUrl &url, bool candidate) +int FaviconManagerPrivate::downloadIcon(const QUrl &url) { static int fakeId = 0; int id; - bool cached = candidate && m_icons.contains(url); + bool cached = m_icons.contains(url); if (isResourceUrl(url) || cached) { id = --fakeId; m_pendingRequests.insert(id, url); @@ -94,13 +96,8 @@ int FaviconManagerPrivate::downloadIcon(const QUrl &url, bool candidate) base::Bind(&FaviconManagerPrivate::iconDownloadFinished, m_weakFactory.GetWeakPtr())); } - if (candidate) { - Q_ASSERT(!m_inProgressCandidateRequests.contains(id)); - m_inProgressCandidateRequests.insert(id, url); - } else { - Q_ASSERT(!m_inProgressCustomRequests.contains(id)); - m_inProgressCustomRequests.insert(id, url); - } + Q_ASSERT(!m_inProgressRequests.contains(id)); + m_inProgressRequests.insert(id, url); return id; } @@ -118,12 +115,10 @@ void FaviconManagerPrivate::iconDownloadFinished(int id, storeIcon(id, toQIcon(bitmaps)); } -/* Pending requests are used as a workaround for avoiding signal iconChanged when - * accessing each cached icons or icons stored in qrc. They don't have to be downloaded - * thus the m_inProgressCustomRequests maybe emptied right before the next icon is added to - * in-progress-requests queue. The m_pendingRequests stores these requests until all - * candidates are added to the queue then pending requests should be cleaned up by this - * function. +/* Pending requests are used to mark icons that are already downloaded (cached icons or icons + * stored in qrc). These requests are also stored in the m_inProgressRequests but the corresponding + * icons are stored in m_icons explicitly by this function. It is necessary to avoid + * m_inProgressRequests being emptied right before the next icon is added by a downloadIcon() call. */ void FaviconManagerPrivate::downloadPendingRequests() { @@ -143,10 +138,9 @@ void FaviconManagerPrivate::downloadPendingRequests() void FaviconManagerPrivate::storeIcon(int id, const QIcon &icon) { Q_Q(FaviconManager); + Q_ASSERT(m_inProgressRequests.contains(id)); - bool candidate = m_inProgressCandidateRequests.contains(id); - - QUrl requestUrl = candidate ? m_inProgressCandidateRequests[id] : m_inProgressCustomRequests[id]; + QUrl requestUrl = m_inProgressRequests[id]; FaviconInfo &faviconInfo = q->m_faviconInfoMap[requestUrl]; unsigned iconCount = 0; @@ -168,29 +162,46 @@ void FaviconManagerPrivate::storeIcon(int id, const QIcon &icon) } } } - - q->m_hasDownloadedIcon = true; - } else if (id < 0) { - // Icon is cached - q->m_hasDownloadedIcon = true; - } else { + } else if (id >= 0) { // Reset size if icon cannot be downloaded faviconInfo.size = QSize(0, 0); } - if (candidate) { - m_inProgressCandidateRequests.remove(id); - if (m_inProgressCandidateRequests.isEmpty()) - m_viewClient->iconChanged(q->getProposedFaviconInfo().url); - } else { - m_inProgressCustomRequests.remove(id); + m_inProgressRequests.remove(id); + if (m_inProgressRequests.isEmpty()) + propagateIcon(); +} + +void FaviconManagerPrivate::propagateIcon() const +{ + Q_Q(const FaviconManager); + + QUrl iconUrl; + const QList &faviconInfoList = q->getFaviconInfoList(true /* candidates only */); + + unsigned bestArea = 0; + for (auto it = faviconInfoList.cbegin(), end = faviconInfoList.cend(); it != end; ++it) { + if (it->type != FaviconInfo::Favicon) + continue; + + if (it->isValid() && bestArea < area(it->size)) { + iconUrl = it->url; + bestArea = area(it->size); + } } - Q_EMIT q->iconDownloaded(requestUrl); + content::NavigationEntry *entry = m_webContents->GetController().GetVisibleEntry(); + if (entry) { + content::FaviconStatus &favicon = entry->GetFavicon(); + favicon.url = toGurl(iconUrl); + favicon.valid = true; + } + + m_viewClient->iconChanged(iconUrl); } FaviconManager::FaviconManager(FaviconManagerPrivate *d) - : m_hasDownloadedIcon(false) + : m_hasCandidate(false) { Q_ASSERT(d); d_ptr.reset(d); @@ -228,121 +239,59 @@ QList FaviconManager::getFaviconInfoList(bool candidatesOnly) const return faviconInfoList; } - -void FaviconManager::downloadIcon(const QUrl &url, FaviconInfo::FaviconType iconType) -{ - Q_D(FaviconManager); - - // If the favicon cannot be found in the list that means that it is not a candidate - // for any visited page (including the current one). In this case the type of the icon - // is unknown: it has to be specified explicitly. - if (!m_faviconInfoMap.contains(url)) { - FaviconInfo newFaviconInfo(url, iconType); - m_faviconInfoMap.insert(url, newFaviconInfo); - } - - d->downloadIcon(url, false); - d->downloadPendingRequests(); -} - -void FaviconManager::removeIcon(const QUrl &url) -{ - Q_D(FaviconManager); - int removed = d->m_icons.remove(url); - - if (removed) { - Q_ASSERT(removed == 1); - Q_ASSERT(m_faviconInfoMap.contains(url)); - m_faviconInfoMap[url].size = QSize(0, 0); - } -} - -bool FaviconManager::hasAvailableCandidateIcon() const -{ - Q_D(const FaviconManager); - return m_hasDownloadedIcon || !d->m_inProgressCandidateRequests.isEmpty(); -} - void FaviconManager::update(const QList &candidates) { Q_D(FaviconManager); updateCandidates(candidates); - for (auto it = m_faviconInfoMap.cbegin(), end = m_faviconInfoMap.cend(); it != end; ++it) { - const FaviconInfo &faviconInfo = it.value(); - if (!faviconInfo.candidate || faviconInfo.type != FaviconInfo::Favicon) + const QList &faviconInfoList = getFaviconInfoList(true /* candidates only */); + for (auto it = faviconInfoList.cbegin(), end = faviconInfoList.cend(); it != end; ++it) { + if (it->type != FaviconInfo::Favicon) continue; - if (faviconInfo.isValid()) - d->downloadIcon(faviconInfo.url, true); + if (it->isValid()) + d->downloadIcon(it->url); } d->downloadPendingRequests(); + + // Reset icon if nothing was downloaded + if (d->m_inProgressRequests.isEmpty()) { + content::NavigationEntry *entry = d->m_webContents->GetController().GetVisibleEntry(); + if (entry && !entry->GetFavicon().valid) + d->m_viewClient->iconChanged(QUrl()); + } } void FaviconManager::updateCandidates(const QList &candidates) { + m_hasCandidate = candidates.count(); for (FaviconInfo candidateFaviconInfo : candidates) { - QUrl candidateUrl = candidateFaviconInfo.url; - if (m_faviconInfoMap.contains(candidateUrl)) { - m_faviconInfoMap[candidateUrl].candidate = true; - // Update type in case of the icon was downloaded manually + const QUrl &candidateUrl = candidateFaviconInfo.url; + + 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; - continue; } - candidateFaviconInfo.candidate = true; - m_faviconInfoMap.insert(candidateUrl, candidateFaviconInfo); + m_faviconInfoMap[candidateUrl].candidate = true; } } void FaviconManager::resetCandidates() { - m_hasDownloadedIcon = false; + m_hasCandidate = false; for (auto it = m_faviconInfoMap.begin(), end = m_faviconInfoMap.end(); it != end; ++it) it->candidate = false; } - -FaviconInfo FaviconManager::getProposedFaviconInfo() const +bool FaviconManager::hasCandidate() const { - FaviconInfo proposedFaviconInfo = getFirstFaviconInfo(); - - // If nothing has been downloaded yet return the first favicon - // if there is available for dev-tools - if (!m_hasDownloadedIcon) - return proposedFaviconInfo; - - unsigned bestArea = area(proposedFaviconInfo.size); - for (auto it = m_faviconInfoMap.cbegin(), end = m_faviconInfoMap.cend(); it != end; ++it) { - const FaviconInfo &faviconInfo = it.value(); - if (!faviconInfo.candidate || faviconInfo.type != FaviconInfo::Favicon) - continue; - - if (faviconInfo.isValid() && bestArea < area(faviconInfo.size)) { - proposedFaviconInfo = faviconInfo; - bestArea = area(proposedFaviconInfo.size); - } - } - - return proposedFaviconInfo; + return m_hasCandidate; } -FaviconInfo FaviconManager::getFirstFaviconInfo() const -{ - for (auto it = m_faviconInfoMap.cbegin(), end = m_faviconInfoMap.cend(); it != end; ++it) { - const FaviconInfo &faviconInfo = it.value(); - if (!faviconInfo.candidate || faviconInfo.type != FaviconInfo::Favicon) - continue; - - if (faviconInfo.isValid()) - return faviconInfo; - } - - return FaviconInfo(); -} - - FaviconInfo::FaviconInfo() : url(QUrl()) diff --git a/src/core/favicon_manager.h b/src/core/favicon_manager.h index ff5c76a9b..dc702a0da 100644 --- a/src/core/favicon_manager.h +++ b/src/core/favicon_manager.h @@ -73,7 +73,7 @@ public: QUrl url; FaviconType type; - // Stores the size of the highest quality in case of multi-size icon + // Stores the largest size in case of multi-size icon QSize size; bool candidate; bool multiSize; @@ -88,25 +88,17 @@ public: QIcon getIcon(const QUrl &) const; FaviconInfo getFaviconInfo(const QUrl &) const; QList getFaviconInfoList(bool) const; - void downloadIcon(const QUrl &url, FaviconInfo::FaviconType iconType = FaviconInfo::Favicon); - void removeIcon(const QUrl &); - -Q_SIGNALS: - void iconDownloaded(const QUrl &url); private: FaviconManager(FaviconManagerPrivate *); - bool hasAvailableCandidateIcon() const; void update(const QList &); void updateCandidates(const QList &); void resetCandidates(); - - FaviconInfo getProposedFaviconInfo() const; - FaviconInfo getFirstFaviconInfo() const; + bool hasCandidate() const; QMap m_faviconInfoMap; - bool m_hasDownloadedIcon; + bool m_hasCandidate; Q_DISABLE_COPY(FaviconManager) Q_DECLARE_PRIVATE(FaviconManager) diff --git a/src/core/favicon_manager_p.h b/src/core/favicon_manager_p.h index 8358245a2..80a012474 100644 --- a/src/core/favicon_manager_p.h +++ b/src/core/favicon_manager_p.h @@ -82,19 +82,19 @@ public: FaviconManagerPrivate(content::WebContents *, WebContentsAdapterClient *); ~FaviconManagerPrivate(); - int downloadIcon(const QUrl &, bool); + int downloadIcon(const QUrl &); void iconDownloadFinished(int, int, const GURL &, const std::vector &, const std::vector &); void storeIcon(int, const QIcon &); void downloadPendingRequests(); + void propagateIcon() const; content::WebContents *m_webContents; WebContentsAdapterClient *m_viewClient; base::WeakPtrFactory m_weakFactory; QMap m_icons; - QMap m_inProgressCandidateRequests; - QMap m_inProgressCustomRequests; + QMap m_inProgressRequests; QMap m_pendingRequests; Q_DECLARE_PUBLIC(FaviconManager) diff --git a/src/core/web_contents_delegate_qt.cpp b/src/core/web_contents_delegate_qt.cpp index a369f9550..51b9d5bd0 100644 --- a/src/core/web_contents_delegate_qt.cpp +++ b/src/core/web_contents_delegate_qt.cpp @@ -60,7 +60,6 @@ #include "components/web_cache/browser/web_cache_manager.h" #include "content/browser/renderer_host/render_widget_host_impl.h" -#include "content/public/browser/favicon_status.h" #include "content/public/browser/invalidate_type.h" #include "content/public/browser/navigation_entry.h" #include "content/public/browser/render_view_host.h" @@ -241,16 +240,11 @@ void WebContentsDelegateQt::DidFinishLoad(content::RenderFrameHost* render_frame if (render_frame_host->GetParent()) return; + if (!m_faviconManager->hasCandidate()) + m_viewClient->iconChanged(QUrl()); + m_viewClient->loadProgressChanged(100); m_viewClient->loadFinished(true, toQt(validated_url)); - - content::NavigationEntry *entry = web_contents()->GetController().GetVisibleEntry(); - if (!entry) - return; - - // No available icon for the current entry - if (!entry->GetFavicon().valid && !m_faviconManager->hasAvailableCandidateIcon()) - m_viewClient->iconChanged(QUrl()); } void WebContentsDelegateQt::DidUpdateFaviconURL(const std::vector &candidates) @@ -263,14 +257,6 @@ void WebContentsDelegateQt::DidUpdateFaviconURL(const std::vectorupdate(faviconCandidates); - - content::NavigationEntry *entry = web_contents()->GetController().GetVisibleEntry(); - if (entry) { - FaviconInfo proposedFaviconInfo = m_faviconManager->getProposedFaviconInfo(); - content::FaviconStatus &favicon = entry->GetFavicon(); - favicon.url = toGurl(proposedFaviconInfo.url); - favicon.valid = proposedFaviconInfo.isValid(); - } } content::ColorChooser *WebContentsDelegateQt::OpenColorChooser(content::WebContents *source, SkColor color, const std::vector &suggestion) -- cgit v1.2.3