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

Unified Diff: components/favicon/ios/web_favicon_driver.cc

Issue 1272413002: Remove useless FaviconHandler::PageChangedSinceFaviconWasRequested() (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Created 5 years, 3 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/ios/web_favicon_driver.cc
diff --git a/components/favicon/ios/web_favicon_driver.cc b/components/favicon/ios/web_favicon_driver.cc
index b691ecfb91de4403dc691c93552751d0218c7e9e..d7e2a2fbf9ca84fc068fe7069f76b4ac5f3a52e2 100644
--- a/components/favicon/ios/web_favicon_driver.cc
+++ b/components/favicon/ios/web_favicon_driver.cc
@@ -17,6 +17,15 @@
DEFINE_WEB_STATE_USER_DATA_KEY(favicon::WebFaviconDriver);
namespace favicon {
+namespace {
+
+std::string UrlWithoutFragment(const GURL& gurl) {
sky 2015/09/11 15:27:15 Return a GURL? Especially given the name you went
pkotwicz 2015/09/11 17:39:58 Done.
+ GURL::Replacements replacements;
+ replacements.ClearRef();
+ return gurl.ReplaceComponents(replacements).spec();
+}
+
+} // namespace
// static
void WebFaviconDriver::CreateForWebState(
@@ -32,6 +41,11 @@ void WebFaviconDriver::CreateForWebState(
history_service, bookmark_model));
}
+void WebFaviconDriver::FetchFavicon(const GURL& url) {
+ fetch_favicon_url_ = url;
+ FaviconDriverImpl::FetchFavicon(url);
+}
+
gfx::Image WebFaviconDriver::GetFavicon() const {
web::NavigationItem* item =
web_state()->GetNavigationManager()->GetLastCommittedItem();
@@ -68,7 +82,7 @@ GURL WebFaviconDriver::GetActiveURL() {
}
bool WebFaviconDriver::GetActiveFaviconValidity() {
- return GetFaviconStatus().valid;
+ return !ActiveURLChangedSinceFetchFavicon() && GetFaviconStatus().valid;
}
void WebFaviconDriver::SetActiveFaviconValidity(bool validity) {
@@ -76,7 +90,7 @@ void WebFaviconDriver::SetActiveFaviconValidity(bool validity) {
}
GURL WebFaviconDriver::GetActiveFaviconURL() {
- return GetFaviconStatus().url;
+ return ActiveURLChangedSinceFetchFavicon() ? GURL() : GetFaviconStatus().url;
}
void WebFaviconDriver::SetActiveFaviconURL(const GURL& url) {
@@ -87,8 +101,25 @@ void WebFaviconDriver::SetActiveFaviconImage(const gfx::Image& image) {
GetFaviconStatus().image = image;
}
+bool WebFaviconDriver::ShouldSendFaviconAvailableNotifications() {
+ // FaviconHandler sends notifications with the assumption that the active URL
+ // has not changed. Suppress FaviconHandler notifications if the active URL
+ // has changed.
+ return !ActiveURLChangedSinceFetchFavicon();
+}
+
+bool WebFaviconDriver::ActiveURLChangedSinceFetchFavicon() {
+ // On iOS the active URL can change in between calls to FetchFavicon(). For
+ // instance, FetchFavicon() is not synchronously called when the active URL
+ // changes as a result of CRWSessionController::goToEntry().
+ // TODO(stuartmorgan): Remove this once iOS always triggers favicon fetches
+ // synchronously after active URL changes.
+ return UrlWithoutFragment(GetActiveURL()) !=
+ UrlWithoutFragment(fetch_favicon_url_);
+}
+
web::FaviconStatus& WebFaviconDriver::GetFaviconStatus() {
- DCHECK(web_state()->GetNavigationManager()->GetVisibleItem());
+ DCHECK(!ActiveURLChangedSinceFetchFavicon());
return web_state()->GetNavigationManager()->GetVisibleItem()->GetFavicon();
}
@@ -105,6 +136,9 @@ WebFaviconDriver::~WebFaviconDriver() {
void WebFaviconDriver::FaviconUrlUpdated(
const std::vector<web::FaviconURL>& candidates) {
+ if (fetch_favicon_url_.is_empty() || ActiveURLChangedSinceFetchFavicon())
+ return;
+
DCHECK(!candidates.empty());
OnUpdateFaviconURL(FaviconURLsFromWebFaviconURLs(candidates));
}
« components/favicon/core/favicon_driver.h ('K') | « components/favicon/ios/web_favicon_driver.h ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698