Convert ContentFaviconDriver to use the new navigation callbacks.

content/browser/frame_host/navigation_handle_impl.cc

Index: content/browser/frame_host/navigation_handle_impl.cc
diff --git a/content/browser/frame_host/navigation_handle_impl.cc b/content/browser/frame_host/navigation_handle_impl.cc
index 30536bd306c76c4da490f9ea142210612c907d0c..44547e2d710872c02b28a3776ec47843c9e9487f 100644
--- a/content/browser/frame_host/navigation_handle_impl.cc
+++ b/content/browser/frame_host/navigation_handle_impl.cc
@@ -16,6 +16,8 @@
 #include "content/browser/frame_host/ancestor_throttle.h"
 #include "content/browser/frame_host/debug_urls.h"
 #include "content/browser/frame_host/frame_tree_node.h"
+#include "content/browser/frame_host/navigation_controller_impl.h"
+#include "content/browser/frame_host/navigation_entry_impl.h"
 #include "content/browser/frame_host/navigator.h"
 #include "content/browser/frame_host/navigator_delegate.h"
 #include "content/browser/loader/resource_dispatcher_host_impl.h"
@@ -100,6 +102,7 @@ NavigationHandleImpl::NavigationHandleImpl(
       is_download_(false),
       is_stream_(false),
       started_from_context_menu_(started_from_context_menu),
+      reload_type_(ReloadType::NONE),
       weak_factory_(this) {
   DCHECK(!navigation_start.is_null());
   redirect_chain_.push_back(url);
@@ -107,6 +110,23 @@ NavigationHandleImpl::NavigationHandleImpl(
   starting_site_instance_ =
       frame_tree_node_->current_frame_host()->GetSiteInstance();
 
+  if (pending_nav_entry_id_) {
+    NavigationControllerImpl* nav_controller =

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

Hmm.  This looks like it will be correct if we find the entry (since it's based
on pending_nav_entry_id_) but missing in some cases.  For example, I'm guessing
we won't have a pending_nav_entry_id_ for renderer-initiated reloads
(location.reload()).  That may be ok for the favicon case, but it's kind of
subtle for a public API.

I think it's also possible in some cases for the pending entry to be gone by the
time of DidStartProvisionalLoad when the handle is constructed.  That's more of
a uncommon race between navigations, I think, and we wouldn't be able to recover
the reload type anyway if it happened.

In general, I was kind of hoping to avoid a dependency on NavigationController
in NavigationHandle, but I suppose it's not a big problem.  On one hand, I was
toying with the idea of getting rid of pending NavigationEntries entirely if
NavigationHandle could be sufficient, but on the other hand, we already have
pending_nav_entry_id_ here to solve another problem.

I guess I'm ok with it, but let's mention some of the downsides (e.g., the
renderer-initiated reload case) in a comment.

[jam, 2017/01/23 19:03:39]

I did find a NavEntry for renderer-initiated reloads, although it doesn't have a
ReloadType set. This is the same as the old behavior, as
WebContentsObserver::DidStartNavigationToPendingEntry also has ReloadType::NONE
in this case.

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

Ah, that's from NavigatorImpl::DidStartMainFrameNavigation.  That's conditional,
though-- it won't be there if there's already a browser-initiated pending entry
(e.g., a slow cross-process navigation getting interrupted by a
location.reload).

Sounds like the outcome may be the same overall, though (i.e., leaving it as
ReloadType::NONE).

+        static_cast<NavigationControllerImpl*>(
+            frame_tree_node_->navigator()->GetController());
+    NavigationEntryImpl* nav_entry =
+        nav_controller->GetEntryWithUniqueID(pending_nav_entry_id_);
+    if (!nav_entry &&
+        nav_controller->GetPendingEntry() &&
+        nav_controller->GetPendingEntry()->GetUniqueID() ==
+            pending_nav_entry_id_) {
+      nav_entry = nav_controller->GetPendingEntry();
+    }
+
+    if (nav_entry)
+      reload_type_ = nav_entry->reload_type();
+  }
+
   if (!IsRendererDebugURL(url_))
     GetDelegate()->DidStartNavigation(this);
 
@@ -396,6 +416,10 @@ const std::string& NavigationHandleImpl::GetSearchableFormEncoding() {
   return searchable_form_encoding_;
 }
 
+ReloadType NavigationHandleImpl::GetReloadType() {
+  return reload_type_;
+}
+
 NavigationData* NavigationHandleImpl::GetNavigationData() {
   return navigation_data_.get();
 }