Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(593)

Unified Diff: content/browser/frame_host/render_frame_host_impl.cc

Issue 135723003: Move DidCommitProvisionalLoad code from RenderView to RenderFrame. (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: Some clean up, ready to start reviewing. Created 6 years, 10 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: content/browser/frame_host/render_frame_host_impl.cc
diff --git a/content/browser/frame_host/render_frame_host_impl.cc b/content/browser/frame_host/render_frame_host_impl.cc
index 98122c86f9b7a5903f14980a45c14a317587c07e..682228a20bf55f0999b68415b3a7eabe4b91112a 100644
--- a/content/browser/frame_host/render_frame_host_impl.cc
+++ b/content/browser/frame_host/render_frame_host_impl.cc
@@ -6,6 +6,7 @@
#include "base/containers/hash_tables.h"
#include "base/lazy_instance.h"
+#include "base/metrics/user_metrics_action.h"
#include "content/browser/frame_host/cross_process_frame_connector.h"
#include "content/browser/frame_host/frame_tree.h"
#include "content/browser/frame_host/frame_tree_node.h"
@@ -17,6 +18,7 @@
#include "content/public/browser/render_process_host.h"
#include "content/public/browser/render_widget_host_view.h"
#include "content/public/browser/user_metrics.h"
+#include "content/public/common/url_constants.h"
#include "url/gurl.h"
namespace content {
@@ -133,6 +135,8 @@ bool RenderFrameHostImpl::OnMessageReceived(const IPC::Message &msg) {
OnDidFailProvisionalLoadWithError)
IPC_MESSAGE_HANDLER(FrameHostMsg_DidRedirectProvisionalLoad,
OnDidRedirectProvisionalLoad)
+ IPC_MESSAGE_HANDLER_GENERIC(FrameHostMsg_DidCommitProvisionalLoad,
+ OnNavigate(msg))
IPC_MESSAGE_HANDLER(FrameHostMsg_SwapOut_ACK, OnSwapOutACK)
IPC_MESSAGE_HANDLER(FrameHostMsg_ContextMenu, OnContextMenu)
IPC_END_MESSAGE_MAP_EX()
@@ -187,6 +191,101 @@ void RenderFrameHostImpl::OnDidRedirectProvisionalLoad(
this, page_id, source_url, target_url);
}
+// Called when the renderer navigates. For every frame loaded, we'll get this
+// notification containing parameters identifying the navigation.
+//
+// Subframes are identified by the page transition type. For subframes loaded
+// as part of a wider page load, the page_id will be the same as for the top
+// level frame. If the user explicitly requests a subframe navigation, we will
+// get a new page_id because we need to create a new navigation entry for that
+// action.
+void RenderFrameHostImpl::OnNavigate(const IPC::Message& msg) {
+ // Read the parameters out of the IPC message directly to avoid making another
+ // copy when we filter the URLs.
+ PickleIterator iter(msg);
+ FrameHostMsg_DidCommitProvisionalLoad_Params validated_params;
+ if (!IPC::ParamTraits<FrameHostMsg_DidCommitProvisionalLoad_Params>::
+ Read(&msg, &iter, &validated_params))
+ return;
+
+ // If we're waiting for a cross-site beforeunload ack from this renderer and
+ // we receive a Navigate message from the main frame, then the renderer was
+ // navigating already and sent it before hearing the ViewMsg_Stop message.
+ // We do not want to cancel the pending navigation in this case, since the
+ // old page will soon be stopped. Instead, treat this as a beforeunload ack
+ // to allow the pending navigation to continue.
+ if (render_view_host_->is_waiting_for_beforeunload_ack_ &&
+ render_view_host_->unload_ack_is_for_cross_site_transition_ &&
+ PageTransitionIsMainFrame(validated_params.transition)) {
+ render_view_host_->OnShouldCloseACK(
+ true, render_view_host_->send_should_close_start_time_,
+ base::TimeTicks::Now());
+ return;
+ }
+
+ // If we're waiting for an unload ack from this renderer and we receive a
+ // Navigate message, then the renderer was navigating before it received the
+ // unload request. It will either respond to the unload request soon or our
+ // timer will expire. Either way, we should ignore this message, because we
+ // have already committed to closing this renderer.
+ if (render_view_host_->is_waiting_for_unload_ack_)
+ return;
+
+ // Cache the main frame id, so we can use it for creating the frame tree
+ // root node when needed.
+ if (PageTransitionIsMainFrame(validated_params.transition)) {
+ if (render_view_host_->main_frame_id_ == -1) {
+ render_view_host_->main_frame_id_ = validated_params.frame_id;
+ } else {
+ // TODO(nasko): We plan to remove the usage of frame_id in navigation
+ // and move to routing ids. This is in place to ensure that a
+ // renderer is not misbehaving and sending us incorrect data.
+ DCHECK_EQ(render_view_host_->main_frame_id_, validated_params.frame_id);
+ }
+ }
+ RenderProcessHost* process = GetProcess();
+
+ // Attempts to commit certain off-limits URL should be caught more strictly
+ // than our FilterURL checks below. If a renderer violates this policy, it
+ // should be killed.
+ if (!render_view_host_->CanCommitURL(validated_params.url)) {
Charlie Reis 2014/02/05 23:30:37 Let's move CanCommitURL to RFH rather than calling
nasko 2014/02/06 01:55:13 Done.
+ VLOG(1) << "Blocked URL " << validated_params.url.spec();
+ validated_params.url = GURL(kAboutBlankURL);
+ RecordAction(base::UserMetricsAction("CanCommitURL_BlockedAndKilled"));
+ // Kills the process.
+ process->ReceivedBadMessage();
+ }
+
+ // Now that something has committed, we don't need to track whether the
+ // initial page has been accessed.
+ render_view_host_->has_accessed_initial_document_ = false;
+
+ // Without this check, an evil renderer can trick the browser into creating
+ // a navigation entry for a banned URL. If the user clicks the back button
+ // followed by the forward button (or clicks reload, or round-trips through
+ // session restore, etc), we'll think that the browser commanded the
+ // renderer to load the URL and grant the renderer the privileges to request
+ // the URL. To prevent this attack, we block the renderer from inserting
+ // banned URLs into the navigation controller in the first place.
+ process->FilterURL(false, &validated_params.url);
+ process->FilterURL(true, &validated_params.referrer.url);
+ for (std::vector<GURL>::iterator it(validated_params.redirects.begin());
+ it != validated_params.redirects.end(); ++it) {
+ process->FilterURL(false, &(*it));
+ }
+ process->FilterURL(true, &validated_params.searchable_form_url);
+
+ // Without this check, the renderer can trick the browser into using
+ // filenames it can't access in a future session restore.
+ if (!render_view_host_->CanAccessFilesOfPageState(
+ validated_params.page_state)) {
+ GetProcess()->ReceivedBadMessage();
+ return;
+ }
+
+ frame_tree_node()->navigator()->DidNavigate(this, validated_params);
+}
+
void RenderFrameHostImpl::SwapOut() {
if (render_view_host_->IsRenderViewLive()) {
Send(new FrameMsg_SwapOut(routing_id_));

Powered by Google App Engine
This is Rietveld 408576698