Refactor the implementation of the webNavigation extension API.

chrome/browser/extensions/api/web_navigation/web_navigation_api.cc

 // Copyright (c) 2012 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.
 
 // Implements the Chrome Extensions WebNavigation API.
 
 #include "chrome/browser/extensions/api/web_navigation/web_navigation_api.h"
 
 #include "base/lazy_instance.h"
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/extensions/api/web_navigation/web_navigation_api_constants.h"
 #include "chrome/browser/extensions/api/web_navigation/web_navigation_api_helpers.h"
 #include "chrome/browser/extensions/extension_tab_util.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/tab_contents/retargeting_details.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_list.h"
 #include "chrome/common/extensions/api/web_navigation.h"
 #include "content/public/browser/navigation_details.h"
+#include "content/public/browser/navigation_handle.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/notification_types.h"
 #include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/render_process_host.h"
 #include "content/public/browser/resource_request_details.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/common/url_constants.h"
 #include "extensions/browser/event_router.h"
 #include "extensions/browser/extension_api_frame_id_map.h"
 #include "extensions/browser/view_type_utils.h"
@@ skipping 229 matching lines @@
     content::RenderFrameHost* old_host,
     content::RenderFrameHost* new_host) {
   if (old_host) {
     RenderFrameDeleted(old_host);
     navigation_state_.FrameHostDeleted(old_host);
   }
 
   navigation_state_.FrameHostCreated(new_host);
 }
 
-void WebNavigationTabObserver::DidStartProvisionalLoadForFrame(
-    content::RenderFrameHost* render_frame_host,
-    const GURL& validated_url,
-    bool is_error_page,
-    bool is_iframe_srcdoc) {
-  DVLOG(2) << "DidStartProvisionalLoad("
-           << "render_frame_host=" << render_frame_host
-           << ", frame_num=" << render_frame_host->GetRoutingID()
-           << ", url=" << validated_url << ")";
-  navigation_state_.StartTrackingNavigation(render_frame_host, validated_url,
-                                            is_error_page, is_iframe_srcdoc);
-
-  if (!navigation_state_.CanSendEvents(render_frame_host))
+void WebNavigationTabObserver::DidStartNavigation(
+    content::NavigationHandle* navigation_handle) {
+  if (navigation_handle->IsSynchronousNavigation())
     return;
 
-  helpers::DispatchOnBeforeNavigate(
-      web_contents(),
-      render_frame_host,
-      navigation_state_.GetUrl(render_frame_host));
+  if (!FrameNavigationState::IsValidUrl(navigation_handle->GetURL()))
+    return;
+
+  helpers::DispatchOnBeforeNavigate(navigation_handle);
 }
 
-void WebNavigationTabObserver::DidCommitProvisionalLoadForFrame(
-    content::RenderFrameHost* render_frame_host,
-    const GURL& url,
-    ui::PageTransition transition_type) {
-  DVLOG(2) << "DidCommitProvisionalLoad("
-           << "render_frame_host=" << render_frame_host
-           << ", frame_num=" << render_frame_host->GetRoutingID()
-           << ", url=" << url << ")";
-  bool is_reference_fragment_navigation =
-      IsReferenceFragmentNavigation(render_frame_host, url);
-  bool is_history_state_modification =
-      navigation_state_.GetNavigationCommitted(render_frame_host);
-
-  // Update the URL as it might have changed.
-  navigation_state_.UpdateFrame(render_frame_host, url);
-  navigation_state_.SetNavigationCommitted(render_frame_host);
-
-  if (!navigation_state_.CanSendEvents(render_frame_host))
-    return;
-
-  events::HistogramValue histogram_value = events::UNKNOWN;
-  std::string event_name;
-  if (is_reference_fragment_navigation) {
-    histogram_value = events::WEB_NAVIGATION_ON_REFERENCE_FRAGMENT_UPDATED;
-    event_name = web_navigation::OnReferenceFragmentUpdated::kEventName;
-  } else if (is_history_state_modification) {
-    histogram_value = events::WEB_NAVIGATION_ON_HISTORY_STATE_UPDATED;
-    event_name = web_navigation::OnHistoryStateUpdated::kEventName;
+void WebNavigationTabObserver::DidFinishNavigation(

[Devlin, 2016/02/05 18:30:28]

I'm sure it's just that I'm not as familiar with all these notifications, but
the flow of:
DidFinishNavigation -> StartTrackingDocumentLoad (which sets is_loading to true
on the navigation frame state - indicating "there is a navigation going on")
seems... odd.

I'm not really sure there's anything much to be done, but maybe we could add a
super short comment here, and/or rephrase the comment for is_loading?

[nasko, 2016/02/05 23:41:22]

I rephrased the comment on is_loading, which I missed initially.

+    content::NavigationHandle* navigation_handle) {
+  if (navigation_handle->HasCommitted()) {
+    if (navigation_handle->IsErrorPage()) {
+      HandleError(navigation_handle);

[Devlin, 2016/02/05 18:30:28]

HandleError vs DispatchOnErrorOccurred is a little weird, especially when
HandleError() just starts tracking the document load and then calls
DispatchOnErrorOccurred().

Maybe this would be cleaner as

bool did_commit = handle->HasCommitted();
bool is_error = !did_commit || handle->IsErrorPage();
if (is_error) {
  if (did_commit)
    StartTrackingDocumentLoad();
  DispatchOnErrorOccurred();
} else {
  HandleCommit();
}

or:
if (!handle->HasCommitted() || handle->IsErrorPage())
  HandleError();  // Add check for HasCommitted to HandleError()
else
  HandleCommit();

[nasko, 2016/02/05 23:41:22]

Done.

+    } else {
+      HandleCommit(navigation_handle);
+    }
   } else {
-    if (navigation_state_.GetIsServerRedirected(render_frame_host)) {
-      transition_type = ui::PageTransitionFromInt(
-          transition_type | ui::PAGE_TRANSITION_SERVER_REDIRECT);
-    }
-    histogram_value = events::WEB_NAVIGATION_ON_COMMITTED;
-    event_name = web_navigation::OnCommitted::kEventName;
+    helpers::DispatchOnErrorOccurred(navigation_handle);
   }
-  helpers::DispatchOnCommitted(
-      histogram_value, event_name, web_contents(), render_frame_host,
-      navigation_state_.GetUrl(render_frame_host), transition_type);
-}
-
-void WebNavigationTabObserver::DidFailProvisionalLoad(
-    content::RenderFrameHost* render_frame_host,
-    const GURL& validated_url,
-    int error_code,
-    const base::string16& error_description,
-    bool was_ignored_by_handler) {
-  DVLOG(2) << "DidFailProvisionalLoad("
-           << "render_frame_host=" << render_frame_host
-           << ", frame_num=" << render_frame_host->GetRoutingID()
-           << ", url=" << validated_url << ")";
-  if (navigation_state_.CanSendEvents(render_frame_host)) {
-    helpers::DispatchOnErrorOccurred(
-        web_contents(),
-        render_frame_host,
-        navigation_state_.GetUrl(render_frame_host),
-        error_code);
-  }
-  navigation_state_.SetErrorOccurredInFrame(render_frame_host);
 }
 
 void WebNavigationTabObserver::DocumentLoadedInFrame(
     content::RenderFrameHost* render_frame_host) {
-  DVLOG(2) << "DocumentLoadedInFrame("
-           << "render_frame_host=" << render_frame_host
-           << ", frame_num=" << render_frame_host->GetRoutingID() << ")";
   if (!navigation_state_.CanSendEvents(render_frame_host))
     return;
+
   navigation_state_.SetParsingFinished(render_frame_host);
   helpers::DispatchOnDOMContentLoaded(
       web_contents(),
       render_frame_host,
       navigation_state_.GetUrl(render_frame_host));
 
   if (!navigation_state_.GetNavigationCompleted(render_frame_host))
     return;
 
   // The load might already have finished by the time we finished parsing. For
   // compatibility reasons, we artifically delay the load completed signal until
   // after parsing was completed.
   helpers::DispatchOnCompleted(web_contents(),
                                render_frame_host,
                                navigation_state_.GetUrl(render_frame_host));
 }
 
 void WebNavigationTabObserver::DidFinishLoad(
     content::RenderFrameHost* render_frame_host,
     const GURL& validated_url) {
-  DVLOG(2) << "DidFinishLoad("
-           << "render_frame_host=" << render_frame_host
-           << ", frame_num=" << render_frame_host->GetRoutingID()
-           << ", url=" << validated_url << ")";
   // When showing replacement content, we might get load signals for frames
   // that weren't reguarly loaded.
   if (!navigation_state_.IsValidFrame(render_frame_host))
     return;
+
   navigation_state_.SetNavigationCompleted(render_frame_host);
   if (!navigation_state_.CanSendEvents(render_frame_host))
     return;
 
   // A new navigation might have started before the old one completed.
   // Ignore the old navigation completion in that case.
   // srcdoc iframes will report a url of about:blank, still let it through.
   if (navigation_state_.GetUrl(render_frame_host) != validated_url &&
       (navigation_state_.GetUrl(render_frame_host) !=
            GURL(content::kAboutSrcDocURL) ||
@@ skipping 10 matching lines @@
                                render_frame_host,
                                navigation_state_.GetUrl(render_frame_host));
 }
 
 void WebNavigationTabObserver::DidFailLoad(
     content::RenderFrameHost* render_frame_host,
     const GURL& validated_url,
     int error_code,
     const base::string16& error_description,
     bool was_ignored_by_handler) {
-  DVLOG(2) << "DidFailLoad("
-           << "render_frame_host=" << render_frame_host
-           << ", frame_num=" << render_frame_host->GetRoutingID()
-           << ", url=" << validated_url << ")";
   // When showing replacement content, we might get load signals for frames
   // that weren't reguarly loaded.
   if (!navigation_state_.IsValidFrame(render_frame_host))
     return;
+
   if (navigation_state_.CanSendEvents(render_frame_host)) {
     helpers::DispatchOnErrorOccurred(
         web_contents(),
         render_frame_host,
         navigation_state_.GetUrl(render_frame_host),
         error_code);
   }
   navigation_state_.SetErrorOccurredInFrame(render_frame_host);
 }
 
-void WebNavigationTabObserver::DidGetRedirectForResourceRequest(
-    content::RenderFrameHost* render_frame_host,
-    const content::ResourceRedirectDetails& details) {
-  if (details.resource_type != content::RESOURCE_TYPE_MAIN_FRAME &&
-      details.resource_type != content::RESOURCE_TYPE_SUB_FRAME) {
-    return;
-  }
-  navigation_state_.SetIsServerRedirected(render_frame_host);
-}
-
 void WebNavigationTabObserver::DidOpenRequestedURL(
     content::WebContents* new_contents,
     content::RenderFrameHost* source_render_frame_host,
     const GURL& url,
     const content::Referrer& referrer,
     WindowOpenDisposition disposition,
     ui::PageTransition transition) {
   if (!navigation_state_.CanSendEvents(source_render_frame_host))
     return;
 
@@ skipping 12 matching lines @@
                                              source_render_frame_host,
                                              new_contents,
                                              url);
 }
 
 void WebNavigationTabObserver::WebContentsDestroyed() {
   g_tab_observer.Get().erase(web_contents());
   registrar_.RemoveAll();
 }
 
+void WebNavigationTabObserver::HandleCommit(
+    content::NavigationHandle* navigation_handle) {
+  bool is_reference_fragment_navigation = IsReferenceFragmentNavigation(
+      navigation_handle->GetRenderFrameHost(), navigation_handle->GetURL());
+
+  navigation_state_.StartTrackingDocumentLoad(
+      navigation_handle->GetRenderFrameHost(), navigation_handle->GetURL(),
+      navigation_handle->IsSamePage(),
+      false,  // is_error_page
+      navigation_handle->IsSrcdoc());
+
+  events::HistogramValue histogram_value = events::UNKNOWN;
+  std::string event_name;
+  if (is_reference_fragment_navigation) {
+    histogram_value = events::WEB_NAVIGATION_ON_REFERENCE_FRAGMENT_UPDATED;
+    event_name = web_navigation::OnReferenceFragmentUpdated::kEventName;
+  } else if (navigation_handle->IsSamePage()) {
+    histogram_value = events::WEB_NAVIGATION_ON_HISTORY_STATE_UPDATED;
+    event_name = web_navigation::OnHistoryStateUpdated::kEventName;
+  } else {
+    histogram_value = events::WEB_NAVIGATION_ON_COMMITTED;
+    event_name = web_navigation::OnCommitted::kEventName;
+  }
+  helpers::DispatchOnCommitted(histogram_value, event_name, navigation_handle);
+}
+
+void WebNavigationTabObserver::HandleError(
+    content::NavigationHandle* navigation_handle) {
+  navigation_state_.StartTrackingDocumentLoad(
+      navigation_handle->GetRenderFrameHost(), navigation_handle->GetURL(),
+      navigation_handle->IsSamePage(),
+      true,  // is_error_page
+      navigation_handle->IsSrcdoc());
+
+  helpers::DispatchOnErrorOccurred(navigation_handle);
+}
+
 // See also NavigationController::IsURLInPageNavigation.
 bool WebNavigationTabObserver::IsReferenceFragmentNavigation(
     content::RenderFrameHost* render_frame_host,
     const GURL& url) {
   GURL existing_url = navigation_state_.GetUrl(render_frame_host);
   if (existing_url == url)
     return false;
 
   url::Replacements<char> replacements;
   replacements.ClearRef();
@@ skipping 130 matching lines @@
   return g_factory.Pointer();
 }
 
 void WebNavigationAPI::OnListenerAdded(const EventListenerInfo& details) {
   web_navigation_event_router_.reset(new WebNavigationEventRouter(
       Profile::FromBrowserContext(browser_context_)));
   EventRouter::Get(browser_context_)->UnregisterObserver(this);
 }
 
 }  // namespace extensions