Index: content/renderer/render_frame_impl.cc |
diff --git a/content/renderer/render_frame_impl.cc b/content/renderer/render_frame_impl.cc |
index c7d721dbee6d9e60d95a36bfbf80344d5a6d3fd6..0d3d03a75383cf3dc3698866a9b422ebc39a7dca 100644 |
--- a/content/renderer/render_frame_impl.cc |
+++ b/content/renderer/render_frame_impl.cc |
@@ -74,6 +74,7 @@ |
#include "content/renderer/notification_provider.h" |
#include "content/renderer/npapi/plugin_channel_host.h" |
#include "content/renderer/push_messaging_dispatcher.h" |
+#include "content/renderer/render_frame_proxy.h" |
#include "content/renderer/render_process.h" |
#include "content/renderer/render_thread_impl.h" |
#include "content/renderer/render_view_impl.h" |
@@ -368,10 +369,31 @@ RenderFrameImpl* RenderFrameImpl::FromRoutingID(int32 routing_id) { |
} |
// static |
+void RenderFrameImpl::CreateFrame(int routing_id, int parent_routing_id) { |
+ CHECK_NE(MSG_ROUTING_NONE, parent_routing_id); |
Charlie Reis
2014/07/24 22:36:30
Wait, the comments on this function and the IPC me
kenrb
2014/07/25 23:42:06
I don't know what is intended here, so I left a TO
|
+ |
+ RenderFrameProxy* proxy = RenderFrameProxy::FromRoutingID(parent_routing_id); |
+ |
+ // If the browser is sending a valid parent routing id, it better be created |
Charlie Reis
2014/07/24 22:36:31
it better be -> it should already be
kenrb
2014/07/25 23:42:06
Done.
|
+ // and registered. |
+ CHECK(proxy); |
+ blink::WebRemoteFrame* parent_web_frame = proxy->web_frame(); |
+ |
+ // Create the RenderFrame and WebLocalFrame, linking the two. |
+ RenderFrameImpl* render_frame = |
+ RenderFrameImpl::Create(proxy->render_view(), routing_id); |
+ blink::WebLocalFrame* web_frame = |
+ parent_web_frame->createLocalChild("", render_frame); |
+ render_frame->SetWebFrame(web_frame); |
+ render_frame->Initialize(); |
+} |
+ |
+// static |
RenderFrame* RenderFrame::FromWebFrame(blink::WebFrame* web_frame) { |
return RenderFrameImpl::FromWebFrame(web_frame); |
} |
+// static |
RenderFrameImpl* RenderFrameImpl::FromWebFrame(blink::WebFrame* web_frame) { |
FrameMap::iterator iter = g_frame_map.Get().find(web_frame); |
if (iter != g_frame_map.Get().end()) |
@@ -668,11 +690,13 @@ bool RenderFrameImpl::Send(IPC::Message* message) { |
delete message; |
return false; |
} |
- if (is_swapped_out_ || render_view_->is_swapped_out()) { |
+ if (frame_->parent() == NULL && |
+ (is_swapped_out_ || render_view_->is_swapped_out())) { |
if (!SwappedOutMessages::CanSendWhileSwappedOut(message)) { |
delete message; |
return false; |
} |
+ |
// In most cases, send IPCs through the proxy when swapped out. In some |
// calls the associated RenderViewImpl routing id is used to send |
// messages, so don't use the proxy. |
@@ -684,7 +708,8 @@ bool RenderFrameImpl::Send(IPC::Message* message) { |
} |
bool RenderFrameImpl::OnMessageReceived(const IPC::Message& msg) { |
- GetContentClient()->SetActiveURL(frame_->document().url()); |
+ if (!frame_->document().isNull()) |
dcheng
2014/07/24 18:23:28
I don't understand why we need this check. Under w
kenrb
2014/07/24 21:06:17
Right now we have RenderFrameProxy wrapping a Rend
|
+ GetContentClient()->SetActiveURL(frame_->document().url()); |
ObserverListBase<RenderFrameObserver>::Iterator it(observers_); |
RenderFrameObserver* observer; |
@@ -755,7 +780,8 @@ void RenderFrameImpl::OnNavigate(const FrameMsg_Navigate_Params& params) { |
return; |
// Swap this renderer back in if necessary. |
- if (render_view_->is_swapped_out_) { |
+ if (render_view_->is_swapped_out_ && |
+ GetWebFrame() == render_view_->webview()->mainFrame()) { |
// We marked the view as hidden when swapping the view out, so be sure to |
// reset the visibility state before navigating to the new URL. |
render_view_->webview()->setVisibilityState( |
@@ -948,6 +974,8 @@ void RenderFrameImpl::OnBeforeUnload() { |
void RenderFrameImpl::OnSwapOut(int proxy_routing_id) { |
RenderFrameProxy* proxy = NULL; |
+ bool is_site_per_process = |
+ CommandLine::ForCurrentProcess()->HasSwitch(switches::kSitePerProcess); |
// Only run unload if we're not swapped out yet, but send the ack either way. |
if (!is_swapped_out_ || !render_view_->is_swapped_out_) { |
@@ -994,7 +1022,8 @@ void RenderFrameImpl::OnSwapOut(int proxy_routing_id) { |
// run a second time, thanks to a check in FrameLoader::stopLoading. |
// TODO(creis): Need to add a better way to do this that avoids running the |
// beforeunload handler. For now, we just run it a second time silently. |
- render_view_->NavigateToSwappedOutURL(frame_); |
+ if (!is_site_per_process || frame_->parent() == NULL) |
+ render_view_->NavigateToSwappedOutURL(frame_); |
// Let WebKit know that this view is hidden so it can drop resources and |
// stop compositing. |
@@ -1014,8 +1043,17 @@ void RenderFrameImpl::OnSwapOut(int proxy_routing_id) { |
// Now that all of the cleanup is complete and the browser side is notified, |
// start using the RenderFrameProxy, if one is created. |
- if (proxy) |
- set_render_frame_proxy(proxy); |
+ if (proxy) { |
+ if (frame_->parent()) { |
+ frame_->swap(proxy->web_frame()); |
+ if (is_site_per_process) { |
Charlie Reis
2014/07/24 22:36:31
When would this be false? We don't swap out subfr
nasko
2014/07/25 07:13:21
It shouldn't be false. We will only have proxies f
kenrb
2014/07/25 23:42:06
I will leave these for Nasko's follow-up CL.
|
+ // TODO(nasko): delete the frame here, since we've replaced it with a |
Charlie Reis
2014/07/24 22:36:30
Can we do this now? (Is it a leak if we don't?)
nasko
2014/07/25 07:13:21
It likely is a leak. I left this piece for a follo
|
+ // proxy. |
+ } |
+ } else { |
+ set_render_frame_proxy(proxy); |
+ } |
+ } |
} |
void RenderFrameImpl::OnContextMenuClosed( |
@@ -1685,6 +1723,7 @@ void RenderFrameImpl::loadURLExternally( |
blink::WebNavigationPolicy policy, |
const blink::WebString& suggested_name) { |
DCHECK(!frame_ || frame_ == frame); |
+ |
Referrer referrer(RenderViewImpl::GetReferrerFromRequest(frame, request)); |
if (policy == blink::WebNavigationPolicyDownload) { |
render_view_->Send(new ViewHostMsg_DownloadUrl(render_view_->GetRoutingID(), |
@@ -1707,8 +1746,9 @@ blink::WebNavigationPolicy RenderFrameImpl::decidePolicyForNavigation( |
blink::WebNavigationPolicy default_policy, |
bool is_redirect) { |
DCHECK(!frame_ || frame_ == frame); |
- return DecidePolicyForNavigation( |
+ WebNavigationPolicy value = DecidePolicyForNavigation( |
Charlie Reis
2014/07/24 22:36:31
Why is this change needed? (Leftover from an earl
nasko
2014/07/25 07:13:21
Most likely leftover from debugging statements usi
kenrb
2014/07/25 23:42:06
Done.
|
this, frame, extra_data, request, type, default_policy, is_redirect); |
+ return value; |
} |
blink::WebHistoryItem RenderFrameImpl::historyItemForNewChildFrame( |
@@ -1813,8 +1853,8 @@ void RenderFrameImpl::didStartProvisionalLoad(blink::WebLocalFrame* frame) { |
DidStartProvisionalLoad(frame)); |
FOR_EACH_OBSERVER(RenderFrameObserver, observers_, DidStartProvisionalLoad()); |
- Send(new FrameHostMsg_DidStartProvisionalLoadForFrame(routing_id_, |
- ds->request().url())); |
+ Send(new FrameHostMsg_DidStartProvisionalLoadForFrame( |
+ routing_id_, ds->request().url())); |
Charlie Reis
2014/07/24 22:36:31
This indent looks wrong. I think it was fine befo
kenrb
2014/07/25 23:42:06
Done.
|
} |
void RenderFrameImpl::didReceiveServerRedirectForProvisionalLoad( |
@@ -2383,13 +2423,19 @@ void RenderFrameImpl::willSendRequest( |
if (request.targetType() == blink::WebURLRequest::TargetIsMainFrame) { |
request.setFirstPartyForCookies(request.url()); |
} else { |
- request.setFirstPartyForCookies( |
- frame->top()->document().firstPartyForCookies()); |
+ // TODO(nasko): When the top-level frame is remote, there is no document. |
+ // This is broken and should be fixed to propagate the URL. |
Charlie Reis
2014/07/24 22:36:31
URL -> first party
kenrb
2014/07/25 23:42:06
Done.
|
+ WebFrame* top = frame->top(); |
+ if (top->isWebLocalFrame()) { |
+ request.setFirstPartyForCookies( |
+ frame->top()->document().firstPartyForCookies()); |
+ } |
} |
} |
WebFrame* top_frame = frame->top(); |
- if (!top_frame) |
+ // TODO(nasko): Hack around asking about top-frame data source. |
Charlie Reis
2014/07/24 22:36:31
This could lead to some nasty bugs. We should ela
kenrb
2014/07/25 23:42:06
Done.
|
+ if (!top_frame || top_frame->isWebRemoteFrame()) |
top_frame = frame; |
WebDataSource* provisional_data_source = top_frame->provisionalDataSource(); |
WebDataSource* top_data_source = top_frame->dataSource(); |
@@ -2486,8 +2532,16 @@ void RenderFrameImpl::willSendRequest( |
provider_id = provider->provider_id(); |
} |
- int parent_routing_id = frame->parent() ? |
- FromWebFrame(frame->parent())->GetRoutingID() : -1; |
+ WebFrame* parent = frame->parent(); |
+ int parent_routing_id = MSG_ROUTING_NONE; |
+ if (!parent) { |
+ parent_routing_id = -1; |
+ } else if (parent->isWebLocalFrame()) { |
+ parent_routing_id = FromWebFrame(parent)->GetRoutingID(); |
+ } else { |
+ parent_routing_id = RenderFrameProxy::FromWebFrame(parent)->routing_id(); |
+ } |
+ |
RequestExtraData* extra_data = new RequestExtraData(); |
extra_data->set_visibility_state(render_view_->visibilityState()); |
extra_data->set_custom_user_agent(custom_user_agent); |
@@ -3198,28 +3252,35 @@ WebNavigationPolicy RenderFrameImpl::DecidePolicyForNavigation( |
#endif |
Referrer referrer(RenderViewImpl::GetReferrerFromRequest(frame, request)); |
+ const CommandLine& command_line = *CommandLine::ForCurrentProcess(); |
- if (is_swapped_out_ || render_view_->is_swapped_out()) { |
- if (request.url() != GURL(kSwappedOutURL)) { |
- // Targeted links may try to navigate a swapped out frame. Allow the |
- // browser process to navigate the tab instead. Note that it is also |
- // possible for non-targeted navigations (from this view) to arrive |
- // here just after we are swapped out. It's ok to send them to the |
- // browser, as long as they're for the top level frame. |
- // TODO(creis): Ensure this supports targeted form submissions when |
- // fixing http://crbug.com/101395. |
- if (frame->parent() == NULL) { |
- OpenURL(frame, request.url(), referrer, default_policy); |
- return blink::WebNavigationPolicyIgnore; // Suppress the load here. |
+ bool is_subframe = !!frame->parent(); |
+ |
+ if (command_line.HasSwitch(switches::kSitePerProcess) && is_subframe) { |
+ // There's no reason to ignore navigations on subframes (why again?) |
Charlie Reis
2014/07/24 22:36:31
"(why again?)"... ?
Perhaps this is because subfr
nasko
2014/07/25 07:13:21
Yes, subframes don't use swapout logic, so the exi
kenrb
2014/07/25 23:42:06
Done.
|
+ } else { |
+ if (is_swapped_out_ || render_view_->is_swapped_out()) { |
+ if (request.url() != GURL(kSwappedOutURL)) { |
+ // Targeted links may try to navigate a swapped out frame. Allow the |
+ // browser process to navigate the tab instead. Note that it is also |
+ // possible for non-targeted navigations (from this view) to arrive |
+ // here just after we are swapped out. It's ok to send them to the |
+ // browser, as long as they're for the top level frame. |
+ // TODO(creis): Ensure this supports targeted form submissions when |
+ // fixing http://crbug.com/101395. |
+ if (frame->parent() == NULL) { |
+ OpenURL(frame, request.url(), referrer, default_policy); |
+ return blink::WebNavigationPolicyIgnore; // Suppress the load here. |
+ } |
+ |
+ // We should otherwise ignore in-process iframe navigations, if they |
+ // arrive just after we are swapped out. |
+ return blink::WebNavigationPolicyIgnore; |
} |
- // We should otherwise ignore in-process iframe navigations, if they |
- // arrive just after we are swapped out. |
- return blink::WebNavigationPolicyIgnore; |
+ // Allow kSwappedOutURL to complete. |
+ return default_policy; |
} |
- |
- // Allow kSwappedOutURL to complete. |
- return default_policy; |
} |
// Webkit is asking whether to navigate to a new URL. |
@@ -3237,7 +3298,6 @@ WebNavigationPolicy RenderFrameImpl::DecidePolicyForNavigation( |
// all top-level navigations to the browser to let it swap processes when |
// crossing site boundaries. This is currently expected to break some script |
// calls and navigations, such as form submissions. |
- const CommandLine& command_line = *CommandLine::ForCurrentProcess(); |
bool force_swap_due_to_flag = |
command_line.HasSwitch(switches::kEnableStrictSiteIsolation) || |
command_line.HasSwitch(switches::kSitePerProcess); |