Remove useless FaviconHandler::PageChangedSinceFaviconWasRequested()

components/favicon/core/favicon_driver_impl.cc

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "components/favicon/core/favicon_driver_impl.h"
 
 #include "base/command_line.h"
 #include "base/logging.h"
 #include "base/metrics/field_trial.h"
 #include "base/strings/string_util.h"
@@ skipping 80 matching lines @@
   if (large_icon_handler_.get()) {
     large_icon_handler_->OnDidDownloadFavicon(id, image_url, bitmaps,
                                               original_bitmap_sizes);
   }
 }
 
 bool FaviconDriverImpl::IsBookmarked(const GURL& url) {
   return bookmark_model_ && bookmark_model_->IsBookmarked(url);
 }
 
-void FaviconDriverImpl::OnFaviconAvailable(const gfx::Image& image,
+void FaviconDriverImpl::OnFaviconAvailable(const GURL& page_url,
                                            const GURL& icon_url,
+                                           const gfx::Image& image,
                                            bool is_active_favicon) {
+#if !defined(IOS)
+  DCHECK_EQ(page_url, GetActiveURL());
+  // 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): Enable the DCHECK on iOS once favicon fetches occur
+  // synchronously after active URL changes on iOS.
+#endif
+  if (page_url != GetActiveURL())

[sky, 2015/09/16 15:14:24]

Shouldn't this be an a #else?

[pkotwicz, 2015/09/16 15:17:56]

I still check this on non-IOS in the case that FaviconHandler goes rogue.

I have seen this pattern
DCHECK(should_be_true);
if (should_be_true) {
}

in a few places in the codebase.

[stuartmorgan, 2015/09/16 18:35:58]

It's explicitly against Chromium's style guide.

[pkotwicz, 2015/09/16 18:47:57]

Fixed now. You are right it is specifically mentioned in the style guide

+    return;
+
   if (is_active_favicon) {
     bool icon_url_changed = GetActiveFaviconURL() != icon_url;
     // No matter what happens, we need to mark the favicon as being set.
     SetActiveFaviconValidity(true);
     SetActiveFaviconURL(icon_url);
 
     if (image.IsEmpty())
       return;
 
     SetActiveFaviconImage(image);
@@ skipping 20 matching lines @@
 void FaviconDriverImpl::SetFaviconOutOfDateForPage(const GURL& url,
                                                    bool force_reload) {
   if (favicon_service_) {
     favicon_service_->SetFaviconOutOfDateForPage(url);
     if (force_reload)
       favicon_service_->ClearUnableToDownloadFavicons();
   }
 }
 
 void FaviconDriverImpl::OnUpdateFaviconURL(
+    const GURL& page_url,
     const std::vector<FaviconURL>& candidates) {
   DCHECK(!candidates.empty());
-  favicon_handler_->OnUpdateFaviconURL(candidates);
+  favicon_handler_->OnUpdateFaviconURL(page_url, candidates);
   if (touch_icon_handler_.get())
-    touch_icon_handler_->OnUpdateFaviconURL(candidates);
+    touch_icon_handler_->OnUpdateFaviconURL(page_url, candidates);
   if (large_icon_handler_.get())
-    large_icon_handler_->OnUpdateFaviconURL(candidates);
+    large_icon_handler_->OnUpdateFaviconURL(page_url, candidates);
 }
 
 }  // namespace favicon