Remove useless FaviconHandler::PageChangedSinceFaviconWasRequested()

components/favicon/ios/web_favicon_driver.cc

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..8fd9fa481ab1b565b6118de5548104307d9465a9 100644
--- a/components/favicon/ios/web_favicon_driver.cc
+++ b/components/favicon/ios/web_favicon_driver.cc
@@ -32,6 +32,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 +73,7 @@ GURL WebFaviconDriver::GetActiveURL() {
 }
 
 bool WebFaviconDriver::GetActiveFaviconValidity() {
-  return GetFaviconStatus().valid;
+  return !ActiveURLChangedSinceFetchFavicon() && GetFaviconStatus().valid;
 }
 
 void WebFaviconDriver::SetActiveFaviconValidity(bool validity) {
@@ -76,7 +81,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 +92,22 @@ 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.

[stuartmorgan, 2015/09/03 23:11:29]

Could you add a TODO (you can put me as the reference) to eliminate this once
iOS only triggers favicon fetches synchronously, matching other platforms?

[pkotwicz, 2015/09/08 19:24:43]

Done.

+  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().
+  return GetActiveURL() != fetch_favicon_url_;

[stuartmorgan, 2015/09/03 23:11:29]

Why drop the fragment-trimming logic?

[pkotwicz, 2015/09/08 19:24:43]

I put it back in for iOS

+}
+
 web::FaviconStatus& WebFaviconDriver::GetFaviconStatus() {
-  DCHECK(web_state()->GetNavigationManager()->GetVisibleItem());
+  DCHECK(!ActiveURLChangedSinceFetchFavicon());
   return web_state()->GetNavigationManager()->GetVisibleItem()->GetFavicon();
 }
 
@@ -105,6 +124,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));
 }