PlzNavigate: implement CommitNavigation on the browser side

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"
@@ skipping 1205 matching lines @@
     render_view_host_->delegate_->RendererUnresponsive(
         render_view_host_,
         render_view_host_->is_waiting_for_beforeunload_ack(),
         render_view_host_->IsWaitingForUnloadACK());
 }
 
 void RenderFrameHostImpl::NotificationClosed(int notification_id) {
   cancel_notification_callbacks_.erase(notification_id);
 }
 
+// PlzNavigate
+void RenderFrameHostImpl::CommitNavigation(
+    const FrameMsg_CommitNavigation_Params& params) {
+  // TODO(clamy): Check if we have to add security checks for the browser plugin
+  // guests.
+
+  DCHECK(render_view_host_->rvh_state() == RenderViewHostImpl::STATE_DEFAULT);
+  // Get back to a clean state, in case we start a new navigation without
+  // completing a RVH swap or unload handler.
+  render_view_host_->SetState(RenderViewHostImpl::STATE_DEFAULT);

[Charlie Reis, 2014/09/15 21:19:39]

I don't understand this state transition.  The destination RFH/RVH is either
already in the default state (e.g., if it's new), or we're swapping it back in. 
In that case, it seems like the state should match whether it's visible or not. 
We do the visual swap after the renderer tells us the navigation has committed,
right?  Or are we moving that earlier to this point in time?

[clamy, 2014/09/18 20:50:39]

Right now we are swapping it in RFHM::CommitNavigation. However we probably want
to do it after the renderer tells us that the navigation has committed (I was
planning to do that in another CL since this one is already a bit big).

[Charlie Reis, 2014/09/19 23:12:31]

Making that change in another CL is fine, but I do think it needs to be moved. 
Can we remove this line until then, or put a TODO to move it if it's needed in
the meantime?

+  Send(new FrameMsg_CommitNavigation(routing_id_, params));
+  // TODO(clamy): Check if we have to be concerned about throbbing javascript

[Charlie Reis, 2014/09/15 21:19:39]

What does this mean?

+  // urls.
+}
+
 void RenderFrameHostImpl::PlatformNotificationPermissionRequestDone(
     int request_id, blink::WebNotificationPermission permission) {
   Send(new PlatformNotificationMsg_PermissionRequestComplete(
       routing_id_, request_id, permission));
 }
 
 void RenderFrameHostImpl::SetAccessibilityMode(AccessibilityMode mode) {
   Send(new FrameMsg_SetAccessibilityMode(routing_id_, mode));
 }
 
@@ skipping 100 matching lines @@
   // Clear any state if a pending navigation is canceled or preempted.
   if (suspended_nav_params_)
     suspended_nav_params_.reset();
 
   TRACE_EVENT_ASYNC_END0("navigation",
                          "RenderFrameHostImpl navigation suspended", this);
   navigations_suspended_ = false;
 }
 
 }  // namespace content