Index: components/favicon/content/content_favicon_driver.cc |
diff --git a/components/favicon/content/content_favicon_driver.cc b/components/favicon/content/content_favicon_driver.cc |
index e5935ffcc381da3ab051b5bf3ac46f35ad66a9fd..fe8627f12f1bf70a0cb1a828938c85ade384e327 100644 |
--- a/components/favicon/content/content_favicon_driver.cc |
+++ b/components/favicon/content/content_favicon_driver.cc |
@@ -17,11 +17,26 @@ |
#include "content/public/browser/navigation_entry.h" |
#include "content/public/browser/navigation_handle.h" |
#include "content/public/common/favicon_url.h" |
+#include "content/public/common/manifest.h" |
#include "ui/gfx/image/image.h" |
DEFINE_WEB_CONTENTS_USER_DATA_KEY(favicon::ContentFaviconDriver); |
namespace favicon { |
+namespace { |
+ |
+void ExtractManifestIcons( |
+ ContentFaviconDriver::ManifestDownloadCallback callback, |
+ const GURL& manifest_url, |
+ const content::Manifest& manifest) { |
+ std::vector<FaviconURL> candidates; |
+ for (const content::Manifest::Icon& icon : manifest.icons) { |
+ candidates.emplace_back(icon.src, favicon_base::FAVICON, icon.sizes); |
+ } |
+ callback.Run(candidates); |
+} |
+ |
+} // namespace |
pkotwicz
2017/05/12 06:13:28
Nit: "namespace" -> "anonymous namespace"
mastiz
2017/05/12 13:31:32
Code search (//\ namespace$) says otherwise (21K v
pkotwicz
2017/05/12 15:37:41
Fair enough
|
// static |
void ContentFaviconDriver::CreateForWebContents( |
@@ -118,6 +133,11 @@ int ContentFaviconDriver::DownloadImage(const GURL& url, |
callback); |
} |
+void ContentFaviconDriver::DownloadManifest(const GURL& url, |
+ ManifestDownloadCallback callback) { |
+ web_contents()->GetManifest(base::Bind(&ExtractManifestIcons, callback)); |
+} |
+ |
bool ContentFaviconDriver::IsOffTheRecord() { |
DCHECK(web_contents()); |
return web_contents()->GetBrowserContext()->IsOffTheRecord(); |
@@ -156,8 +176,31 @@ void ContentFaviconDriver::DidUpdateFaviconURL( |
return; |
favicon_urls_ = candidates; |
+ |
OnUpdateCandidates(entry->GetURL(), |
- FaviconURLsFromContentFaviconURLs(candidates)); |
+ FaviconURLsFromContentFaviconURLs(candidates), |
+ manifest_url_); |
+} |
+ |
+void ContentFaviconDriver::DidUpdateWebManifestURL( |
+ const base::Optional<GURL>& manifest_url) { |
+ // Ignore the update if there is no last committed navigation entry. This can |
+ // occur when loading an initially blank page. |
+ content::NavigationEntry* entry = |
+ web_contents()->GetController().GetLastCommittedEntry(); |
+ if (!entry) |
+ return; |
+ |
+ manifest_url_ = manifest_url; |
+ |
+ // On regular page loads, DidUpdateManifestURL() is guaranteed to be called |
+ // before DidUpdateFaviconURL(). However, a page can update the favicons via |
+ // javascript. |
+ if (favicon_urls_.has_value()) { |
+ OnUpdateCandidates(entry->GetURL(), |
+ FaviconURLsFromContentFaviconURLs(*favicon_urls_), |
+ manifest_url); |
+ } |
} |
void ContentFaviconDriver::DidStartNavigation( |
@@ -165,6 +208,9 @@ void ContentFaviconDriver::DidStartNavigation( |
if (!navigation_handle->IsInMainFrame()) |
return; |
+ favicon_urls_.reset(); |
+ manifest_url_.reset(); |
pkotwicz
2017/05/12 06:13:28
Can this code be in DidFinishNavigation() instead?
mastiz
2017/05/12 13:31:32
I assume a navigation can be interrupted and hence
pkotwicz
2017/05/12 15:37:41
My understanding is that DidFinishNavigation() sho
mastiz
2017/05/15 14:06:59
The question is whether it's guaranteed to be call
|
+ |
content::ReloadType reload_type = navigation_handle->GetReloadType(); |
if (reload_type == content::ReloadType::NONE || IsOffTheRecord()) |
return; |
@@ -183,8 +229,6 @@ void ContentFaviconDriver::DidFinishNavigation( |
return; |
} |
- favicon_urls_.clear(); |
- |
// Wait till the user navigates to a new URL to start checking the cache |
// again. The cache may be ignored for non-reload navigations (e.g. |
// history.replace() in-page navigation). This is allowed to increase the |