Convert ContentFaviconDriver to use the new navigation callbacks.

content/browser/frame_host/navigation_handle_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 "content/browser/frame_host/navigation_handle_impl.h"
 
 #include <iterator>
 
 #include "base/debug/dump_without_crashing.h"
 #include "base/logging.h"
 #include "content/browser/appcache/appcache_navigation_handle.h"
 #include "content/browser/appcache/appcache_service_impl.h"
 #include "content/browser/browsing_data/clear_site_data_throttle.h"
 #include "content/browser/child_process_security_policy_impl.h"
 #include "content/browser/devtools/render_frame_devtools_agent_host.h"
 #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"
 #include "content/browser/service_worker/service_worker_context_wrapper.h"
 #include "content/browser/service_worker/service_worker_navigation_handle.h"
 #include "content/common/frame_messages.h"
 #include "content/common/resource_request_body_impl.h"
 #include "content/common/site_isolation_policy.h"
 #include "content/public/browser/content_browser_client.h"
 #include "content/public/browser/navigation_ui_data.h"
@@ skipping 64 matching lines @@
       frame_tree_node_(frame_tree_node),
       next_index_(0),
       navigation_start_(navigation_start),
       pending_nav_entry_id_(pending_nav_entry_id),
       request_context_type_(REQUEST_CONTEXT_TYPE_UNSPECIFIED),
       mixed_content_context_type_(blink::WebMixedContentContextType::Blockable),
       should_replace_current_entry_(false),
       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);
 
   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);
 
   if (IsInMainFrame()) {
     TRACE_EVENT_ASYNC_BEGIN_WITH_TIMESTAMP1(
         "navigation", "Navigation StartToCommit", this,
         navigation_start, "Initial URL", url_.spec());
   }
 }
 
@@ skipping 269 matching lines @@
 }
 
 const GURL& NavigationHandleImpl::GetSearchableFormURL() {
   return searchable_form_url_;
 }
 
 const std::string& NavigationHandleImpl::GetSearchableFormEncoding() {
   return searchable_form_encoding_;
 }
 
+ReloadType NavigationHandleImpl::GetReloadType() {
+  return reload_type_;
+}
+
 NavigationData* NavigationHandleImpl::GetNavigationData() {
   return navigation_data_.get();
 }
 
 const GlobalRequestID& NavigationHandleImpl::GetGlobalRequestID() {
   DCHECK(state_ == WILL_PROCESS_RESPONSE || state_ == DEFERRING_RESPONSE ||
          state_ == READY_TO_COMMIT);
   return request_id_;
 }
 
@@ skipping 397 matching lines @@
       content::AncestorThrottle::MaybeCreateThrottleFor(this);
   if (ancestor_throttle)
     throttles_.push_back(std::move(ancestor_throttle));
 
   throttles_.insert(throttles_.begin(),
                     std::make_move_iterator(throttles_to_register.begin()),
                     std::make_move_iterator(throttles_to_register.end()));
 }
 
 }  // namespace content