Convert ContentFaviconDriver to use the new navigation callbacks.

components/favicon/content/content_favicon_driver.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/content/content_favicon_driver.h"
 
 #include "base/bind.h"
 #include "components/favicon/content/favicon_url_util.h"
 #include "components/favicon/core/favicon_service.h"
 #include "components/favicon/core/favicon_url.h"
 #include "components/history/core/browser/history_service.h"
 #include "content/public/browser/browser_context.h"
 #include "content/public/browser/favicon_status.h"
 #include "content/public/browser/navigation_controller.h"
 #include "content/public/browser/navigation_details.h"
 #include "content/public/browser/navigation_entry.h"
+#include "content/public/browser/navigation_handle.h"
 #include "content/public/common/favicon_url.h"
 #include "ui/gfx/image/image.h"
 
 DEFINE_WEB_CONTENTS_USER_DATA_KEY(favicon::ContentFaviconDriver);
 
 namespace favicon {
 
 // static
 void ContentFaviconDriver::CreateForWebContents(
     content::WebContents* web_contents,
@@ skipping 128 matching lines @@
   content::NavigationEntry* entry =
       web_contents()->GetController().GetLastCommittedEntry();
   if (!entry)
     return;
 
   favicon_urls_ = candidates;
   OnUpdateFaviconURL(entry->GetURL(),
                      FaviconURLsFromContentFaviconURLs(candidates));
 }
 
-void ContentFaviconDriver::DidStartNavigationToPendingEntry(
-    const GURL& url,
-    content::ReloadType reload_type) {
+void ContentFaviconDriver::DidStartNavigation(

[Charlie Reis, 2017/01/23 18:30:08]

Note: This will get called more often than before, such as for
renderer-initiated navigations (e.g., link clicks, script navigations, etc).

sdefresne@ or pkotwicz@, can you confirm that's ok for the
SetFaviconOutOfDateForPage logic?  I would expect so, but if not, we can do an
early return if !navigation_handle->IsRendererInitiated().

[jam, 2017/01/23 19:05:02]

This shouldn't make a difference because the function early returns for
non-reloads, so the cases you mention wouldn't have any effect right?

[Charlie Reis, 2017/01/23 19:53:30]

Yes, looks like you're right.

+    content::NavigationHandle* navigation_handle) {
+  if (!navigation_handle->IsInMainFrame())
+    return;
+
+  content::ReloadType reload_type = navigation_handle->GetReloadType();
   if (reload_type == content::ReloadType::NONE || IsOffTheRecord())
     return;
 
-  bypass_cache_page_url_ = url;
+  bypass_cache_page_url_ = navigation_handle->GetURL();
   SetFaviconOutOfDateForPage(
-      url, reload_type == content::ReloadType::BYPASSING_CACHE);
+      navigation_handle->GetURL(),
+      reload_type == content::ReloadType::BYPASSING_CACHE);
 }
 
-void ContentFaviconDriver::DidNavigateMainFrame(
-    const content::LoadCommittedDetails& details,
-    const content::FrameNavigateParams& params) {
+void ContentFaviconDriver::DidFinishNavigation(
+    content::NavigationHandle* navigation_handle) {
+  if (!navigation_handle->IsInMainFrame() ||
+      !navigation_handle->HasCommitted() ||
+      navigation_handle->IsErrorPage()) {
+    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
   // likelihood that "reloading a page ignoring the cache" redownloads the
   // favicon. In particular, a page may do an in-page navigation before
   // FaviconHandler has the time to determine that the favicon needs to be
   // redownloaded.
-  GURL url = details.entry->GetURL();
+  GURL url = navigation_handle->GetURL();
   if (url != bypass_cache_page_url_)
     bypass_cache_page_url_ = GURL();
 
   // Get the favicon, either from history or request it from the net.
   FetchFavicon(url);
 }
 
 }  // namespace favicon