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

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

Issue 2628133002: When a proxy is detached, immediately delete its associated provisional frame. (Closed)
Patch Set: Remove (hopefully unnecessary) null check 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
OLDNEW
1 // Copyright (c) 2012 The Chromium Authors. All rights reserved. 1 // Copyright (c) 2012 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 <stddef.h> 5 #include <stddef.h>
6 #include <stdint.h> 6 #include <stdint.h>
7 #include <tuple> 7 #include <tuple>
8 8
9 #include "base/bind.h" 9 #include "base/bind.h"
10 #include "base/callback.h" 10 #include "base/callback.h"
(...skipping 824 matching lines...) Expand 10 before | Expand all | Expand 10 after
835 // replicated correctly. 835 // replicated correctly.
836 replication_state.origin = url::Origin(); 836 replication_state.origin = url::Origin();
837 TestRenderFrame* child_frame2 = static_cast<TestRenderFrame*>( 837 TestRenderFrame* child_frame2 = static_cast<TestRenderFrame*>(
838 RenderFrame::FromWebFrame(web_frame->firstChild()->nextSibling())); 838 RenderFrame::FromWebFrame(web_frame->firstChild()->nextSibling()));
839 child_frame2->SwapOut(kProxyRoutingId + 1, true, replication_state); 839 child_frame2->SwapOut(kProxyRoutingId + 1, true, replication_state);
840 EXPECT_TRUE(web_frame->firstChild()->nextSibling()->isWebRemoteFrame()); 840 EXPECT_TRUE(web_frame->firstChild()->nextSibling()->isWebRemoteFrame());
841 EXPECT_TRUE( 841 EXPECT_TRUE(
842 web_frame->firstChild()->nextSibling()->getSecurityOrigin().isUnique()); 842 web_frame->firstChild()->nextSibling()->getSecurityOrigin().isUnique());
843 } 843 }
844 844
845 // Test for https://crbug.com/568676, where a parent detaches a remote child 845 // Test that when a parent detaches a remote child after the provisional
846 // while the browser navigates it to the parent's site in parallel, with the 846 // RenderFrame is created but before it is navigated, the RenderFrame is
847 // detach happening after the provisional RenderFrame is created but before 847 // destroyed along with the proxy. This protects against races in
848 // FrameMsg_Navigate is received. This is a variant of 848 // https://crbug.com/526304 and https://crbug.com/568676.
849 // https://crbug.com/526304. 849 TEST_F(RenderViewImplTest, DetachingProxyAlsoDestroysProvisionalFrame) {
850 TEST_F(RenderViewImplTest, NavigateProxyAndDetachBeforeOnNavigate) {
851 // This test should only run with --site-per-process. 850 // This test should only run with --site-per-process.
852 if (!AreAllSitesIsolatedForTesting()) 851 if (!AreAllSitesIsolatedForTesting())
853 return; 852 return;
854 853
855 LoadHTML("Hello <iframe src='data:text/html,frame 1'></iframe>"); 854 LoadHTML("Hello <iframe src='data:text/html,frame 1'></iframe>");
856 WebFrame* web_frame = frame()->GetWebFrame(); 855 WebFrame* web_frame = frame()->GetWebFrame();
857 TestRenderFrame* child_frame = static_cast<TestRenderFrame*>( 856 TestRenderFrame* child_frame = static_cast<TestRenderFrame*>(
858 RenderFrame::FromWebFrame(web_frame->firstChild())); 857 RenderFrame::FromWebFrame(web_frame->firstChild()));
859 858
860 // Swap the child frame out. 859 // Swap the child frame out.
861 FrameReplicationState replication_state = 860 FrameReplicationState replication_state =
862 ReconstructReplicationStateForTesting(child_frame); 861 ReconstructReplicationStateForTesting(child_frame);
863 child_frame->SwapOut(kProxyRoutingId, true, replication_state); 862 child_frame->SwapOut(kProxyRoutingId, true, replication_state);
864 EXPECT_TRUE(web_frame->firstChild()->isWebRemoteFrame()); 863 EXPECT_TRUE(web_frame->firstChild()->isWebRemoteFrame());
865 864
866 // Do the first step of a remote-to-local transition for the child proxy, 865 // Do the first step of a remote-to-local transition for the child proxy,
867 // which is to create a provisional local frame. 866 // which is to create a provisional local frame.
868 int routing_id = kProxyRoutingId + 1; 867 int routing_id = kProxyRoutingId + 1;
869 mojom::CreateFrameWidgetParams widget_params; 868 mojom::CreateFrameWidgetParams widget_params;
870 widget_params.routing_id = MSG_ROUTING_NONE; 869 widget_params.routing_id = MSG_ROUTING_NONE;
871 widget_params.hidden = false; 870 widget_params.hidden = false;
872 RenderFrameImpl::CreateFrame(routing_id, kProxyRoutingId, MSG_ROUTING_NONE, 871 RenderFrameImpl::CreateFrame(routing_id, kProxyRoutingId, MSG_ROUTING_NONE,
873 frame()->GetRoutingID(), MSG_ROUTING_NONE, 872 frame()->GetRoutingID(), MSG_ROUTING_NONE,
874 replication_state, nullptr, widget_params, 873 replication_state, nullptr, widget_params,
875 FrameOwnerProperties()); 874 FrameOwnerProperties());
876 TestRenderFrame* provisional_frame = 875 {
877 static_cast<TestRenderFrame*>(RenderFrameImpl::FromRoutingID(routing_id)); 876 TestRenderFrame* provisional_frame = static_cast<TestRenderFrame*>(
878 EXPECT_TRUE(provisional_frame); 877 RenderFrameImpl::FromRoutingID(routing_id));
878 EXPECT_TRUE(provisional_frame);
879 }
879 880
880 // Detach the child frame (currently remote) in the main frame. 881 // Detach the child frame (currently remote) in the main frame.
881 ExecuteJavaScriptForTests( 882 ExecuteJavaScriptForTests(
882 "document.body.removeChild(document.querySelector('iframe'));"); 883 "document.body.removeChild(document.querySelector('iframe'));");
883 RenderFrameProxy* child_proxy = 884 RenderFrameProxy* child_proxy =
884 RenderFrameProxy::FromRoutingID(kProxyRoutingId); 885 RenderFrameProxy::FromRoutingID(kProxyRoutingId);
885 EXPECT_FALSE(child_proxy); 886 EXPECT_FALSE(child_proxy);
886 887
887 // Attempt to start a navigation on the RenderFrame that was created to 888 // The provisional frame should have been deleted along with the proxy, and
888 // replace the now-detached RenderFrameProxy. This shouldn't crash and 889 // thus any subsequent messages (such as OnNavigate) already in flight for it
889 // should abort the navigation, since the frame no longer exists. 890 // should be dropped.
890 CommonNavigationParams common_params; 891 {
891 common_params.navigation_type = FrameMsg_Navigate_Type::NORMAL; 892 TestRenderFrame* provisional_frame = static_cast<TestRenderFrame*>(
892 common_params.url = GURL(url::kAboutBlankURL); 893 RenderFrameImpl::FromRoutingID(routing_id));
893 provisional_frame->Navigate(common_params, StartNavigationParams(), 894 EXPECT_FALSE(provisional_frame);
alexmos 2017/01/13 02:26:54 I didn't find a way to test this Navigate() call a
Charlie Reis 2017/01/18 00:18:42 Acknowledged.
894 RequestNavigationParams()); 895 }
895 ProcessPendingMessages();
896
897 // Check that there was no DidCommitProvisionalLoad.
898 const IPC::Message* frame_navigate_msg =
899 render_thread_->sink().GetUniqueMessageMatching(
900 FrameHostMsg_DidCommitProvisionalLoad::ID);
901 EXPECT_FALSE(frame_navigate_msg);
902
903 // Detach the provisional frame to clean it up. Normally, the browser
904 // process would trigger this via FrameMsg_Delete.
905 provisional_frame->GetWebFrame()->detach();
906 } 896 }
907 897
908 // Verify that the renderer process doesn't crash when device scale factor 898 // Verify that the renderer process doesn't crash when device scale factor
909 // changes after a cross-process navigation has commited. 899 // changes after a cross-process navigation has commited.
910 // See https://crbug.com/571603. 900 // See https://crbug.com/571603.
911 TEST_F(RenderViewImplTest, SetZoomLevelAfterCrossProcessNavigation) { 901 TEST_F(RenderViewImplTest, SetZoomLevelAfterCrossProcessNavigation) {
912 // This test should only run with out-of-process iframes enabled. 902 // This test should only run with out-of-process iframes enabled.
913 if (!SiteIsolationPolicy::AreCrossProcessFramesPossible()) 903 if (!SiteIsolationPolicy::AreCrossProcessFramesPossible())
914 return; 904 return;
915 905
(...skipping 1640 matching lines...) Expand 10 before | Expand all | Expand 10 after
2556 ExpectPauseAndResume(3); 2546 ExpectPauseAndResume(3);
2557 blink::WebScriptSource source2( 2547 blink::WebScriptSource source2(
2558 WebString::fromUTF8("function func2() { func1(); }; func2();")); 2548 WebString::fromUTF8("function func2() { func1(); }; func2();"));
2559 frame()->GetWebFrame()->executeScriptInIsolatedWorld(17, &source2, 1, 1); 2549 frame()->GetWebFrame()->executeScriptInIsolatedWorld(17, &source2, 1, 1);
2560 2550
2561 EXPECT_FALSE(IsPaused()); 2551 EXPECT_FALSE(IsPaused());
2562 Detach(); 2552 Detach();
2563 } 2553 }
2564 2554
2565 } // namespace content 2555 } // namespace content
OLDNEW
« content/renderer/render_frame_proxy.h ('K') | « content/renderer/render_frame_proxy.cc ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698