PlzNavigate: make content unit tests work with browser side navigation

content/browser/frame_host/render_frame_host_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/render_frame_host_impl.h"
 
 #include "base/bind.h"
 #include "base/command_line.h"
 #include "base/containers/hash_tables.h"
 #include "base/lazy_instance.h"
 #include "base/metrics/histogram.h"
 #include "base/metrics/user_metrics_action.h"
 #include "base/time/time.h"
 #include "content/browser/accessibility/accessibility_mode_helper.h"
 #include "content/browser/accessibility/browser_accessibility_manager.h"
 #include "content/browser/accessibility/browser_accessibility_state_impl.h"
 #include "content/browser/child_process_security_policy_impl.h"
 #include "content/browser/frame_host/cross_process_frame_connector.h"
 #include "content/browser/frame_host/cross_site_transferring_request.h"
 #include "content/browser/frame_host/frame_accessibility.h"
 #include "content/browser/frame_host/frame_tree.h"
 #include "content/browser/frame_host/frame_tree_node.h"
 #include "content/browser/frame_host/navigator.h"
+#include "content/browser/frame_host/navigator_impl.h"
 #include "content/browser/frame_host/render_frame_host_delegate.h"
 #include "content/browser/frame_host/render_frame_proxy_host.h"
 #include "content/browser/frame_host/render_widget_host_view_child_frame.h"
 #include "content/browser/geolocation/geolocation_service_context.h"
 #include "content/browser/permissions/permission_service_context.h"
 #include "content/browser/permissions/permission_service_impl.h"
 #include "content/browser/renderer_host/input/input_router.h"
 #include "content/browser/renderer_host/input/timeout_monitor.h"
 #include "content/browser/renderer_host/render_process_host_impl.h"
 #include "content/browser/renderer_host/render_view_host_delegate.h"
@@ skipping 833 matching lines @@
 
   frame_tree_node_->render_manager()->OnBeforeUnloadACK(
       unload_ack_is_for_cross_site_transition_, proceed,
       before_unload_end_time);
 
   // If canceled, notify the delegate to cancel its pending navigation entry.
   if (!proceed)
     render_view_host_->GetDelegate()->DidCancelLoading();
 }
 
+bool RenderFrameHostImpl::IsWaitingForBeforeUnloadACK() const {
+  if (!base::CommandLine::ForCurrentProcess()->HasSwitch(
+      switches::kEnableBrowserSideNavigation)) {
+    return is_waiting_for_beforeunload_ack_;
+  }
+  return static_cast<NavigatorImpl*>(frame_tree_node_->navigator())
+      ->IsWaitingForBeforeUnloadACK(frame_tree_node_);

[nasko, 2015/01/17 00:22:01]

Does it make sense to put this on the interface and avoid the cast here?

[clamy, 2015/01/19 17:29:20]

Tried it in the latest patch set, it may be slightly better. I'm not sure though
that I understand the precise difference between Navigator and NavigatorImpl.
Considering they are both in content/browser/frame_host and therefore not
public, why do we have both?

+}
+
 bool RenderFrameHostImpl::IsWaitingForUnloadACK() const {
   return render_view_host_->is_waiting_for_close_ack_ ||
       rfh_state_ == STATE_PENDING_SWAP_OUT;
 }
 
 void RenderFrameHostImpl::OnSwapOutACK() {
   OnSwappedOut();
 }
 
 void RenderFrameHostImpl::OnSwappedOut() {
@@ skipping 509 matching lines @@
 
 // PlzNavigate
 void RenderFrameHostImpl::CommitNavigation(
     ResourceResponse* response,
     scoped_ptr<StreamHandle> body,
     const CommonNavigationParams& common_params,
     const CommitNavigationParams& commit_params) {
   // TODO(clamy): Check if we have to add security checks for the browser plugin
   // guests.
 
+  // There's navigation message params waiting to be sent. Now that we're not

[nasko, 2015/01/17 00:22:01]

nit: 'params' implies 'There're'

[clamy, 2015/01/19 17:29:20]

The whole block was removed since it landed in the speculative RFH patch.

+  // suspended anymore, resume navigation by sending them. If we were swapped

[nasko, 2015/01/17 00:22:01]

We don't use suspending of navigations in PlzNavigate, do we? What are those
suspended params.

[clamy, 2015/01/19 17:29:20]

Same here.

+  // out, we should also stop filtering out the IPC messages now.
+  SetState(RenderFrameHostImpl::STATE_DEFAULT);
+
   Send(new FrameMsg_CommitNavigation(
       routing_id_, response->head, body->GetURL(),
       common_params, commit_params));
   // TODO(clamy): Check if we should start the throbber for non javascript urls
   // here.
 
   // TODO(clamy): Release the stream handle once the renderer has finished
   // reading it.
   stream_handle_ = body.Pass();
 }
@@ skipping 194 matching lines @@
 void RenderFrameHostImpl::DidUseGeolocationPermission() {
   RenderFrameHost* top_frame = frame_tree_node()->frame_tree()->GetMainFrame();
   GetContentClient()->browser()->RegisterPermissionUsage(
       PERMISSION_GEOLOCATION,
       delegate_->GetAsWebContents(),
       GetLastCommittedURL().GetOrigin(),
       top_frame->GetLastCommittedURL().GetOrigin());
 }
 
 }  // namespace content