Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(237)

Unified Diff: components/favicon/content/content_favicon_driver.cc

Issue 2799273002: Add support to process favicons from Web Manifests (Closed)
Patch Set: Add hacks for demoing (DONOTSUBMIT) Created 3 years, 8 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
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 d6bbcac5d8f4465732789f634c9778e2e8779099..1d7ce4c80d3507ca7f62fa9371f797797bdffaba 100644
--- a/components/favicon/content/content_favicon_driver.cc
+++ b/components/favicon/content/content_favicon_driver.cc
@@ -16,11 +16,39 @@
#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 {
+
+// TODO / DONOTSUBMIT: Only for prototyping purposes.
+void ExtractManifestURL(
+ const base::Callback<void(const base::Optional<GURL>&)>& callback,
+ const GURL& manifest_url,
+ const content::Manifest& manifest) {
+ if (manifest_url.is_empty())
+ callback.Run(base::nullopt);
+ else
+ callback.Run(manifest_url);
+}
+
+void ExtractManifestIcons(
+ ContentFaviconDriver::ManifestDownloadCallback callback,
+ const GURL& manifest_url,
+ const content::Manifest& manifest) {
+ // TODO/DONOTSUBMIT(mastiz): This should distinguish 404s from other status
+ // codes.
+ std::vector<FaviconURL> candidates;
+ for (const content::Manifest::Icon& icon : manifest.icons) {
+ candidates.emplace_back(icon.src, favicon_base::FAVICON, icon.sizes);
+ }
+ callback.Run(/*status_code=*/200, candidates);
+}
+
+} // namespace
// static
void ContentFaviconDriver::CreateForWebContents(
@@ -112,10 +140,17 @@ int ContentFaviconDriver::DownloadImage(const GURL& url,
bool bypass_cache = (bypass_cache_page_url_ == GetActiveURL());
bypass_cache_page_url_ = GURL();
+ LOG(INFO) << "MIKEL Downloading image " << url;
+
return web_contents()->DownloadImage(url, true, max_image_size, bypass_cache,
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();
@@ -144,6 +179,7 @@ void ContentFaviconDriver::OnFaviconUpdated(
void ContentFaviconDriver::DidUpdateFaviconURL(
const std::vector<content::FaviconURL>& candidates) {
+ LOG(INFO) << "MIKEL DidUpdateFaviconURL() " << candidates.size();
DCHECK(!candidates.empty());
// Ignore the update if there is no last committed navigation entry. This can
@@ -154,8 +190,33 @@ void ContentFaviconDriver::DidUpdateFaviconURL(
return;
favicon_urls_ = candidates;
- OnUpdateFaviconURL(entry->GetURL(),
- FaviconURLsFromContentFaviconURLs(candidates));
+
+ // On regular page loads, DidUpdateManifestURL() is guaranteed to be called
+ // after DidUpdateFaviconURL(). However, a page can update the favicons via
+ // javascript.
+ if (received_manifest_url_) {
+ OnUpdateCandidates(entry->GetURL(),
+ FaviconURLsFromContentFaviconURLs(favicon_urls_),
+ manifest_url_);
+ }
+}
+
+void ContentFaviconDriver::DidUpdateManifestURL(
+ const base::Optional<GURL>& manifest_url) {
+ LOG(INFO) << "MIKEL DidUpdateManifestURL() " << manifest_url.value_or(GURL());
+ // 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;
+
+ received_manifest_url_ = true;
+ manifest_url_ = manifest_url;
+
+ OnUpdateCandidates(entry->GetURL(),
+ FaviconURLsFromContentFaviconURLs(favicon_urls_),
+ manifest_url);
}
void ContentFaviconDriver::DidStartNavigation(
@@ -163,6 +224,10 @@ void ContentFaviconDriver::DidStartNavigation(
if (!navigation_handle->IsInMainFrame())
return;
+ favicon_urls_.clear();
+ received_manifest_url_ = false;
+ manifest_url_.reset();
+
content::ReloadType reload_type = navigation_handle->GetReloadType();
if (reload_type == content::ReloadType::NONE || IsOffTheRecord())
return;
@@ -181,7 +246,13 @@ void ContentFaviconDriver::DidFinishNavigation(
return;
}
- favicon_urls_.clear();
+ /////////////////////////////
+ // DONOTSUBMIT / TODO: This hack is here to make the prototype work.
+ base::Callback<void(const base::Optional<GURL>&)> cb =
+ base::Bind(&ContentFaviconDriver::DidUpdateManifestURL,
+ base::Unretained(this));
+ web_contents()->GetManifest(base::Bind(&ExtractManifestURL, cb));
+ /////////////////////////////
// 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.

Powered by Google App Engine
This is Rietveld 408576698