PlzNavigate: Fix dynamic iframe back forward layout test failure

content/browser/frame_host/navigator_impl.cc

 // Copyright 2013 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/navigator_impl.h"
 
 #include <utility>
 
 #include "base/logging.h"
 #include "base/metrics/histogram_macros.h"
@@ skipping 27 matching lines @@
 #include "content/public/browser/navigation_details.h"
 #include "content/public/browser/page_navigator.h"
 #include "content/public/browser/render_view_host.h"
 #include "content/public/browser/stream_handle.h"
 #include "content/public/browser/user_metrics.h"
 #include "content/public/common/bindings_policy.h"
 #include "content/public/common/browser_side_navigation_policy.h"
 #include "content/public/common/content_client.h"
 #include "content/public/common/content_constants.h"
 #include "content/public/common/resource_response.h"
+#include "net/base/load_flags.h"
 #include "net/base/net_errors.h"
 #include "url/gurl.h"
 #include "url/url_util.h"
 
 namespace content {
 
 namespace {
 
 FrameMsg_Navigate_Type::Value GetNavigationType(
     const GURL& old_url,
@@ skipping 250 matching lines @@
 }
 
 bool NavigatorImpl::NavigateToEntry(
     FrameTreeNode* frame_tree_node,
     const FrameNavigationEntry& frame_entry,
     const NavigationEntryImpl& entry,
     ReloadType reload_type,
     bool is_same_document_history_load,
     bool is_history_navigation_in_new_child,
     bool is_pending_entry,
+    bool is_user_gesture,
     const scoped_refptr<ResourceRequestBodyImpl>& post_body) {
   TRACE_EVENT0("browser,navigation", "NavigatorImpl::NavigateToEntry");
 
   GURL dest_url = frame_entry.url();
   Referrer dest_referrer = frame_entry.referrer();
   if (reload_type == ReloadType::ORIGINAL_REQUEST_URL &&
       entry.GetOriginalRequestURL().is_valid() && !entry.GetHasPostData()) {
     // We may have been redirected when navigating to the current URL.
     // Use the URL the user originally intended to visit, if it's valid and if a
     // POST wasn't involved; the latter case avoids issues with sending data to
@@ skipping 46 matching lines @@
                      ->last_navigation_previews_state();
   } else if (reload_type == ReloadType::DISABLE_LOFI_MODE) {
     // Disable LoFi when asked for it explicitly.
     previews_state = PREVIEWS_NO_TRANSFORM;
   }
 
   // PlzNavigate: the RenderFrameHosts are no longer asked to navigate.
   if (IsBrowserSideNavigationEnabled()) {
     navigation_data_.reset(new NavigationMetricsData(navigation_start, dest_url,
                                                      entry.restore_type()));
-    RequestNavigation(frame_tree_node, dest_url, dest_referrer, frame_entry,
-                      entry, reload_type, previews_state,
-                      is_same_document_history_load,
-                      is_history_navigation_in_new_child, navigation_start);
+    RequestNavigation(
+        frame_tree_node, dest_url, dest_referrer, frame_entry, entry,
+        reload_type, previews_state, is_same_document_history_load,
+        is_history_navigation_in_new_child, is_user_gesture, navigation_start);
     if (frame_tree_node->IsMainFrame() &&
         frame_tree_node->navigation_request()) {
       // TODO(carlosk): extend these traces to support subframes and
       // non-PlzNavigate navigations.
       // For the trace below we're using the navigation handle as the async
       // trace id, |navigation_start| as the timestamp and reporting the
       // FrameTreeNode id as a parameter. For navigations where no network
       // request is made (data URLs, JavaScript URLs, etc) there is no handle
       // and so no tracing is done.
       TRACE_EVENT_ASYNC_BEGIN_WITH_TIMESTAMP1(
@@ skipping 97 matching lines @@
     delegate_->DidStartNavigationToPendingEntry(dest_url, reload_type);
 
   return true;
 }
 
 bool NavigatorImpl::NavigateToPendingEntry(
     FrameTreeNode* frame_tree_node,
     const FrameNavigationEntry& frame_entry,
     ReloadType reload_type,
     bool is_same_document_history_load) {
-  return NavigateToEntry(frame_tree_node, frame_entry,
-                         *controller_->GetPendingEntry(), reload_type,
-                         is_same_document_history_load, false, true, nullptr);
+  return NavigateToEntry(
+      frame_tree_node, frame_entry, *controller_->GetPendingEntry(),
+      reload_type, is_same_document_history_load, false, true, false, nullptr);
 }
 
 bool NavigatorImpl::NavigateNewChildFrame(
     RenderFrameHostImpl* render_frame_host,
+    bool is_user_gesture,
     const GURL& default_url) {
   NavigationEntryImpl* entry =
       controller_->GetEntryWithUniqueID(render_frame_host->nav_entry_id());
   if (!entry)
     return false;
 
   FrameNavigationEntry* frame_entry =
       entry->GetFrameEntry(render_frame_host->frame_tree_node());
   if (!frame_entry)
     return false;
@@ skipping 12 matching lines @@
         render_frame_host->frame_tree_node()->unique_name();
     const char kFramePathPrefix[] = "<!--framePath ";
     if (base::StartsWith(unique_name, kFramePathPrefix,
                          base::CompareCase::SENSITIVE)) {
       UMA_HISTOGRAM_COUNTS("SessionRestore.RestoreSubframeFramePathLength",
                            unique_name.size());
     }
   }
 
   return NavigateToEntry(render_frame_host->frame_tree_node(), *frame_entry,
-                         *entry, ReloadType::NONE, false, true, false, nullptr);
+                         *entry, ReloadType::NONE, false, true, false,
+                         is_user_gesture, nullptr);
 }
 
 void NavigatorImpl::DidNavigate(
     RenderFrameHostImpl* render_frame_host,
     const FrameHostMsg_DidCommitProvisionalLoad_Params& params,
     std::unique_ptr<NavigationHandleImpl> navigation_handle) {
   FrameTree* frame_tree = render_frame_host->frame_tree_node()->frame_tree();
   bool oopifs_possible = SiteIsolationPolicy::AreCrossProcessFramesPossible();
 
   bool has_embedded_credentials =
@@ skipping 396 matching lines @@
   // |entry| during NavigateToEntry.  This will go away when we shortcut this
   // further in https://crbug.com/536906.
   scoped_refptr<FrameNavigationEntry> frame_entry(entry->GetFrameEntry(node));
   if (!frame_entry) {
     frame_entry = new FrameNavigationEntry(
         node->unique_name(), -1, -1, nullptr,
         static_cast<SiteInstanceImpl*>(source_site_instance), dest_url,
         referrer_to_use, method, -1);
   }
   NavigateToEntry(node, *frame_entry, *entry.get(), ReloadType::NONE, false,
-                  false, false, post_body);
+                  false, false, false, post_body);
 }
 
 // PlzNavigate
 void NavigatorImpl::OnBeforeUnloadACK(FrameTreeNode* frame_tree_node,
                                       bool proceed) {
   CHECK(IsBrowserSideNavigationEnabled());
   DCHECK(frame_tree_node);
 
   NavigationRequest* navigation_request = frame_tree_node->navigation_request();
 
   // The NavigationRequest may have been canceled while the renderer was
   // executing the BeforeUnload event.
   if (!navigation_request)
     return;
 
-  DCHECK_EQ(NavigationRequest::WAITING_FOR_RENDERER_RESPONSE,
-            navigation_request->state());
+  // A new navigation may have occurred.

[clamy, 2017/02/15 16:27:55]

This seems wrong. If we started a new browser-initiated NavigationRequest, it
should have committed immediately or be waiting for the BeforeUnload ACK.
Similarly, a renderer-initiated navigation could not have started while we were
waiting for the BeforeUnload ACK. In what case are you hitting this issue?

[ananta, 2017/02/15 23:45:02]

The scenario is the following:-
1. A browser navigation was initiated via a renderer history back forward 
   mechanism. It is waiting for the unload ack from the renderer.
2. A new navigation from the renderer comes in. We allow this as the 
   pending navigation above is a renderer navigation.
3. Eventually the ack comes in. The navigation which is now underway is 
   the one initiated in step 2 and hence the state check fails.

+  if (navigation_request->state() !=
+      NavigationRequest::WAITING_FOR_RENDERER_RESPONSE) {
+    return;
+  }
 
   // If the navigation is allowed to proceed, send the request to the IO thread.
   if (proceed)
     navigation_request->BeginNavigation();
   else
     CancelNavigation(frame_tree_node);
 }
 
 // PlzNavigate
 void NavigatorImpl::OnBeginNavigation(
@@ skipping 164 matching lines @@
 // PlzNavigate
 void NavigatorImpl::RequestNavigation(FrameTreeNode* frame_tree_node,
                                       const GURL& dest_url,
                                       const Referrer& dest_referrer,
                                       const FrameNavigationEntry& frame_entry,
                                       const NavigationEntryImpl& entry,
                                       ReloadType reload_type,
                                       PreviewsState previews_state,
                                       bool is_same_document_history_load,
                                       bool is_history_navigation_in_new_child,
+                                      bool is_user_gesture,
                                       base::TimeTicks navigation_start) {
   CHECK(IsBrowserSideNavigationEnabled());
   DCHECK(frame_tree_node);
 
   // This value must be set here because creating a NavigationRequest might
   // change the renderer live/non-live status and change this result.
   bool should_dispatch_beforeunload =
       !is_same_document_history_load &&
       frame_tree_node->current_frame_host()->ShouldDispatchBeforeUnload();
   FrameMsg_Navigate_Type::Value navigation_type = GetNavigationType(
       frame_tree_node->current_url(),  // old_url
       dest_url,                        // new_url
       reload_type,                     // reload_type
       entry,                           // entry
       frame_entry,                     // frame_entry
       is_same_document_history_load);  // is_same_document_history_load
-  std::unique_ptr<NavigationRequest> scoped_request =
-      NavigationRequest::CreateBrowserInitiated(
-          frame_tree_node, dest_url, dest_referrer, frame_entry, entry,
-          navigation_type, previews_state, is_same_document_history_load,
-          is_history_navigation_in_new_child, navigation_start, controller_);
+
+  std::unique_ptr<NavigationRequest> scoped_request;
+  // We treat JS history navigations in child frame as renderer initiated
+  // navigations.
+  if (is_history_navigation_in_new_child && !is_user_gesture) {
+    scoped_request =
+        NavigationRequest::CreateRendererInitiatedHistoryNavigation(
+            frame_tree_node, dest_url, dest_referrer, frame_entry, entry,
+            navigation_type, previews_state, is_history_navigation_in_new_child,
+            is_user_gesture, navigation_start, controller_);
+  }
+
+  if (!scoped_request.get()) {
+    scoped_request = NavigationRequest::CreateBrowserInitiated(
+        frame_tree_node, dest_url, dest_referrer, frame_entry, entry,
+        navigation_type, previews_state, is_same_document_history_load,
+        is_history_navigation_in_new_child, navigation_start, controller_);
+  }
 
   // Navigation to a javascript URL is not a "real" navigation so there is no
   // need to create a NavigationHandle. The navigation commits immediately and
   // the NavigationRequest is not assigned to the FrameTreeNode as navigating to
   // a Javascript URL should not interrupt a previous navigation.
   // Note: The scoped_request will be destroyed at the end of this function.
   if (dest_url.SchemeIs(url::kJavaScriptScheme)) {
     RenderFrameHostImpl* render_frame_host =
         frame_tree_node->render_manager()->GetFrameHostForNavigation(
             *scoped_request.get());
@@ skipping 127 matching lines @@
     if (navigation_handle)
       navigation_handle->update_entry_id_for_transfer(entry->GetUniqueID());
 
     controller_->SetPendingEntry(std::move(entry));
     if (delegate_)
       delegate_->NotifyChangedNavigationState(content::INVALIDATE_TYPE_URL);
   }
 }
 
 }  // namespace content