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

Unified Diff: content/browser/renderer_host/render_view_host_impl.cc

Issue 135723003: Move DidCommitProvisionalLoad code from RenderView to RenderFrame. (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: Fixed unit tests and removed WebContents::DidNavigate 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/renderer_host/render_view_host_impl.cc
diff --git a/content/browser/renderer_host/render_view_host_impl.cc b/content/browser/renderer_host/render_view_host_impl.cc
index 752d34e591a24248a4e4c52e321c7df742d9024a..73cd509caab7d196a82674ba498640b459f3504e 100644
--- a/content/browser/renderer_host/render_view_host_impl.cc
+++ b/content/browser/renderer_host/render_view_host_impl.cc
@@ -43,6 +43,7 @@
#include "content/common/content_switches_internal.h"
#include "content/common/desktop_notification_messages.h"
#include "content/common/drag_messages.h"
+#include "content/common/frame_messages.h"
#include "content/common/input_messages.h"
#include "content/common/inter_process_time_ticks_converter.h"
#include "content/common/speech_recognition_messages.h"
@@ -1220,7 +1221,6 @@ bool RenderViewHostImpl::OnMessageReceived(const IPC::Message& msg) {
IPC_MESSAGE_HANDLER_DELAY_REPLY(ViewHostMsg_RunModal, OnRunModal)
IPC_MESSAGE_HANDLER(ViewHostMsg_RenderViewReady, OnRenderViewReady)
IPC_MESSAGE_HANDLER(ViewHostMsg_RenderProcessGone, OnRenderProcessGone)
- IPC_MESSAGE_HANDLER_GENERIC(ViewHostMsg_FrameNavigate, OnNavigate(msg))
IPC_MESSAGE_HANDLER(ViewHostMsg_UpdateState, OnUpdateState)
IPC_MESSAGE_HANDLER(ViewHostMsg_UpdateTitle, OnUpdateTitle)
IPC_MESSAGE_HANDLER(ViewHostMsg_UpdateEncoding, OnUpdateEncoding)
@@ -1434,97 +1434,15 @@ void RenderViewHostImpl::OnDidStartProvisionalLoadForFrame(
NOTREACHED();
}
-// 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.
Charlie Reis 2014/02/06 21:49:07 The rest of this comment can be removed as well.
nasko 2014/02/06 21:58:43 Done.
void RenderViewHostImpl::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);
- ViewHostMsg_FrameNavigate_Params validated_params;
- if (!IPC::ParamTraits<ViewHostMsg_FrameNavigate_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 (is_waiting_for_beforeunload_ack_ &&
- unload_ack_is_for_cross_site_transition_ &&
- PageTransitionIsMainFrame(validated_params.transition)) {
- OnShouldCloseACK(true, 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 (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 (main_frame_id_ == -1) {
- 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(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 (!CanCommitURL(validated_params.url)) {
- 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.
- 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 (!CanAccessFilesOfPageState(validated_params.page_state)) {
- GetProcess()->ReceivedBadMessage();
- return;
- }
-
- delegate_->DidNavigate(this, validated_params);
+ // TODO(nasko): Forward calls to the top level RenderFrameHost until all
+ // callers of this method on RenderViewHost are removed.
+ delegate_->GetFrameTree()->GetMainFrame()->OnMessageReceived(msg);
}
void RenderViewHostImpl::OnUpdateState(int32 page_id, const PageState& state) {
@@ -1963,15 +1881,6 @@ void RenderViewHostImpl::ToggleSpeechInput() {
Send(new InputTagSpeechMsg_ToggleSpeechInput(GetRoutingID()));
}
-bool RenderViewHostImpl::CanCommitURL(const GURL& url) {
- // TODO(creis): We should also check for WebUI pages here. Also, when the
- // out-of-process iframes implementation is ready, we should check for
- // cross-site URLs that are not allowed to commit in this process.
-
- // Give the client a chance to disallow URLs from committing.
- return GetContentClient()->browser()->CanCommitURL(GetProcess(), url);
-}
-
void RenderViewHostImpl::ExitFullscreen() {
RejectMouseLockOrUnlockIfNecessary();
// Notify delegate_ and renderer of fullscreen state change.

Powered by Google App Engine
This is Rietveld 408576698