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

Side by Side Diff: content/renderer/render_frame_impl.cc

Issue 2628133002: When a proxy is detached, immediately delete its associated provisional frame. (Closed)
Patch Set: Charlie's nit Created 3 years, 11 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 unified diff | Download patch
« no previous file with comments | « content/browser/site_per_process_browsertest.cc ('k') | content/renderer/render_frame_proxy.h » ('j') | no next file with comments »
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
OLDNEW
1 // Copyright 2013 The Chromium Authors. All rights reserved. 1 // Copyright 2013 The Chromium Authors. All rights reserved.
2 // Use of this source code is governed by a BSD-style license that can be 2 // Use of this source code is governed by a BSD-style license that can be
3 // found in the LICENSE file. 3 // found in the LICENSE file.
4 4
5 #include "content/renderer/render_frame_impl.h" 5 #include "content/renderer/render_frame_impl.h"
6 6
7 #include <map> 7 #include <map>
8 #include <string> 8 #include <string>
9 #include <utility> 9 #include <utility>
10 #include <vector> 10 #include <vector>
(...skipping 985 matching lines...) Expand 10 before | Expand all | Expand 10 after
996 // The remote frame could've been detached while the remote-to-local 996 // The remote frame could've been detached while the remote-to-local
997 // navigation was being initiated in the browser process. Drop the 997 // navigation was being initiated in the browser process. Drop the
998 // navigation and don't create the frame in that case. See 998 // navigation and don't create the frame in that case. See
999 // https://crbug.com/526304. 999 // https://crbug.com/526304.
1000 if (!proxy) 1000 if (!proxy)
1001 return; 1001 return;
1002 1002
1003 render_frame = RenderFrameImpl::Create(proxy->render_view(), routing_id); 1003 render_frame = RenderFrameImpl::Create(proxy->render_view(), routing_id);
1004 render_frame->InitializeBlameContext(nullptr); 1004 render_frame->InitializeBlameContext(nullptr);
1005 render_frame->proxy_routing_id_ = proxy_routing_id; 1005 render_frame->proxy_routing_id_ = proxy_routing_id;
1006 proxy->set_provisional_frame_routing_id(routing_id);
1006 web_frame = blink::WebLocalFrame::createProvisional( 1007 web_frame = blink::WebLocalFrame::createProvisional(
1007 render_frame, proxy->web_frame(), replicated_state.sandbox_flags); 1008 render_frame, proxy->web_frame(), replicated_state.sandbox_flags);
1008 } 1009 }
1009 render_frame->BindToWebFrame(web_frame); 1010 render_frame->BindToWebFrame(web_frame);
1010 CHECK(parent_routing_id != MSG_ROUTING_NONE || !web_frame->parent()); 1011 CHECK(parent_routing_id != MSG_ROUTING_NONE || !web_frame->parent());
1011 1012
1012 if (widget_params.routing_id != MSG_ROUTING_NONE) { 1013 if (widget_params.routing_id != MSG_ROUTING_NONE) {
1013 CHECK(!web_frame->parent() || 1014 CHECK(!web_frame->parent() ||
1014 SiteIsolationPolicy::AreCrossProcessFramesPossible()); 1015 SiteIsolationPolicy::AreCrossProcessFramesPossible());
1015 render_frame->render_widget_ = RenderWidget::CreateForFrame( 1016 render_frame->render_widget_ = RenderWidget::CreateForFrame(
(...skipping 586 matching lines...) Expand 10 before | Expand all | Expand 10 after
1602 void RenderFrameImpl::OnAssociatedInterfaceRequest( 1603 void RenderFrameImpl::OnAssociatedInterfaceRequest(
1603 const std::string& interface_name, 1604 const std::string& interface_name,
1604 mojo::ScopedInterfaceEndpointHandle handle) { 1605 mojo::ScopedInterfaceEndpointHandle handle) {
1605 associated_interfaces_.BindRequest(interface_name, std::move(handle)); 1606 associated_interfaces_.BindRequest(interface_name, std::move(handle));
1606 } 1607 }
1607 1608
1608 void RenderFrameImpl::OnNavigate( 1609 void RenderFrameImpl::OnNavigate(
1609 const CommonNavigationParams& common_params, 1610 const CommonNavigationParams& common_params,
1610 const StartNavigationParams& start_params, 1611 const StartNavigationParams& start_params,
1611 const RequestNavigationParams& request_params) { 1612 const RequestNavigationParams& request_params) {
1612 // If this RenderFrame is going to replace a RenderFrameProxy, it is possible
1613 // that the proxy was detached before this navigation request was received.
1614 // In that case, abort the navigation. See https://crbug.com/526304 and
1615 // https://crbug.com/568676.
1616 // TODO(nasko, alexmos): Eventually, the browser process will send an IPC to
1617 // clean this frame up after https://crbug.com/548275 is fixed.
1618 if (proxy_routing_id_ != MSG_ROUTING_NONE) {
1619 RenderFrameProxy* proxy =
1620 RenderFrameProxy::FromRoutingID(proxy_routing_id_);
1621 if (!proxy)
1622 return;
1623 }
1624
1625 RenderThreadImpl* render_thread_impl = RenderThreadImpl::current(); 1613 RenderThreadImpl* render_thread_impl = RenderThreadImpl::current();
1626 // Can be NULL in tests. 1614 // Can be NULL in tests.
1627 if (render_thread_impl) 1615 if (render_thread_impl)
1628 render_thread_impl->GetRendererScheduler()->OnNavigationStarted(); 1616 render_thread_impl->GetRendererScheduler()->OnNavigationStarted();
1629 DCHECK(!IsBrowserSideNavigationEnabled()); 1617 DCHECK(!IsBrowserSideNavigationEnabled());
1630 TRACE_EVENT2("navigation,rail", "RenderFrameImpl::OnNavigate", "id", 1618 TRACE_EVENT2("navigation,rail", "RenderFrameImpl::OnNavigate", "id",
1631 routing_id_, "url", common_params.url.possibly_invalid_spec()); 1619 routing_id_, "url", common_params.url.possibly_invalid_spec());
1632 NavigateInternal(common_params, start_params, request_params, 1620 NavigateInternal(common_params, start_params, request_params,
1633 std::unique_ptr<StreamOverrideParameters>()); 1621 std::unique_ptr<StreamOverrideParameters>());
1634 } 1622 }
(...skipping 1498 matching lines...) Expand 10 before | Expand all | Expand 10 after
3133 type == DetachType::Remove) { 3121 type == DetachType::Remove) {
3134 frame->parent()->removeChild(frame); 3122 frame->parent()->removeChild(frame);
3135 } 3123 }
3136 3124
3137 // |frame| is invalid after here. Be sure to clear frame_ as well, since this 3125 // |frame| is invalid after here. Be sure to clear frame_ as well, since this
3138 // object may not be deleted immediately and other methods may try to access 3126 // object may not be deleted immediately and other methods may try to access
3139 // it. 3127 // it.
3140 frame->close(); 3128 frame->close();
3141 frame_ = nullptr; 3129 frame_ = nullptr;
3142 3130
3131 // If this was a provisional frame with an associated proxy, tell the proxy
3132 // that it's no longer associated with this frame.
3133 if (proxy_routing_id_ != MSG_ROUTING_NONE) {
3134 RenderFrameProxy* proxy =
3135 RenderFrameProxy::FromRoutingID(proxy_routing_id_);
3136
3137 // |proxy| should always exist. Detaching the proxy would've also detached
3138 // this provisional frame. The proxy should also not be associated with
3139 // another provisional frame at this point.
3140 CHECK(proxy);
3141 CHECK_EQ(routing_id_, proxy->provisional_frame_routing_id());
3142
3143 proxy->set_provisional_frame_routing_id(MSG_ROUTING_NONE);
3144 }
3145
3143 delete this; 3146 delete this;
3144 // Object is invalid after this point. 3147 // Object is invalid after this point.
3145 } 3148 }
3146 3149
3147 void RenderFrameImpl::frameFocused() { 3150 void RenderFrameImpl::frameFocused() {
3148 Send(new FrameHostMsg_FrameFocused(routing_id_)); 3151 Send(new FrameHostMsg_FrameFocused(routing_id_));
3149 } 3152 }
3150 3153
3151 void RenderFrameImpl::willCommitProvisionalLoad(blink::WebLocalFrame* frame) { 3154 void RenderFrameImpl::willCommitProvisionalLoad(blink::WebLocalFrame* frame) {
3152 DCHECK_EQ(frame_, frame); 3155 DCHECK_EQ(frame_, frame);
(...skipping 1899 matching lines...) Expand 10 before | Expand all | Expand 10 after
5052 Send(new FrameHostMsg_DidCommitProvisionalLoad(routing_id_, params)); 5055 Send(new FrameHostMsg_DidCommitProvisionalLoad(routing_id_, params));
5053 5056
5054 // If we end up reusing this WebRequest (for example, due to a #ref click), 5057 // If we end up reusing this WebRequest (for example, due to a #ref click),
5055 // we don't want the transition type to persist. Just clear it. 5058 // we don't want the transition type to persist. Just clear it.
5056 navigation_state->set_transition_type(ui::PAGE_TRANSITION_LINK); 5059 navigation_state->set_transition_type(ui::PAGE_TRANSITION_LINK);
5057 } 5060 }
5058 5061
5059 bool RenderFrameImpl::SwapIn() { 5062 bool RenderFrameImpl::SwapIn() {
5060 CHECK_NE(proxy_routing_id_, MSG_ROUTING_NONE); 5063 CHECK_NE(proxy_routing_id_, MSG_ROUTING_NONE);
5061 CHECK(!in_frame_tree_); 5064 CHECK(!in_frame_tree_);
5065
5066 // The proxy should always exist. If it was detached while the provisional
5067 // LocalFrame was being navigated, the provisional frame would've been
5068 // cleaned up by RenderFrameProxy::frameDetached. See
5069 // https://crbug.com/526304 and https://crbug.com/568676 for context.
5062 RenderFrameProxy* proxy = RenderFrameProxy::FromRoutingID(proxy_routing_id_); 5070 RenderFrameProxy* proxy = RenderFrameProxy::FromRoutingID(proxy_routing_id_);
5063 5071 CHECK(proxy);
5064 // The proxy might have been detached while the provisional LocalFrame was
5065 // being navigated. In that case, don't swap the frame back in the tree
5066 // and return early (to avoid sending confusing IPCs to the browser
5067 // process). See https://crbug.com/526304 and https://crbug.com/568676.
5068 if (!proxy)
5069 return false;
5070 5072
5071 int proxy_routing_id = proxy_routing_id_; 5073 int proxy_routing_id = proxy_routing_id_;
5072 if (!proxy->web_frame()->swap(frame_)) 5074 if (!proxy->web_frame()->swap(frame_))
5073 return false; 5075 return false;
5074 5076
5075 proxy_routing_id_ = MSG_ROUTING_NONE; 5077 proxy_routing_id_ = MSG_ROUTING_NONE;
5076 in_frame_tree_ = true; 5078 in_frame_tree_ = true;
5077 5079
5078 // If this is the main frame going from a remote frame to a local frame, 5080 // If this is the main frame going from a remote frame to a local frame,
5079 // it needs to set RenderViewImpl's pointer for the main frame to itself 5081 // it needs to set RenderViewImpl's pointer for the main frame to itself
(...skipping 1794 matching lines...) Expand 10 before | Expand all | Expand 10 after
6874 // event target. Potentially a Pepper plugin will receive the event. 6876 // event target. Potentially a Pepper plugin will receive the event.
6875 // In order to tell whether a plugin gets the last mouse event and which it 6877 // In order to tell whether a plugin gets the last mouse event and which it
6876 // is, we set |pepper_last_mouse_event_target_| to null here. If a plugin gets 6878 // is, we set |pepper_last_mouse_event_target_| to null here. If a plugin gets
6877 // the event, it will notify us via DidReceiveMouseEvent() and set itself as 6879 // the event, it will notify us via DidReceiveMouseEvent() and set itself as
6878 // |pepper_last_mouse_event_target_|. 6880 // |pepper_last_mouse_event_target_|.
6879 pepper_last_mouse_event_target_ = nullptr; 6881 pepper_last_mouse_event_target_ = nullptr;
6880 #endif 6882 #endif
6881 } 6883 }
6882 6884
6883 } // namespace content 6885 } // namespace content
OLDNEW
« no previous file with comments | « content/browser/site_per_process_browsertest.cc ('k') | content/renderer/render_frame_proxy.h » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698