From f40ce5b0e297205977af598a5ebc6c5c920c4a4a Mon Sep 17 00:00:00 2001 From: alexmos Date: Wed, 25 Feb 2015 12:19:56 -0800 Subject: [PATCH] Make load events in iframe elements work with OOPIF (Chromium side). Today, when the parent frame defines a load event handler in the iframe element of an out-of-process child frame, the event does not fire in the parent process. This CL adds the plumbing to forward the event from the child frame's process to the parent frame's process. Blink side: https://codereview.chromium.org/937203003 BUG=453690 Review URL: https://codereview.chromium.org/954793002 Cr-Commit-Position: refs/heads/master@{#318101} --- .../frame_host/render_frame_host_impl.cc | 16 +++++++++ .../frame_host/render_frame_host_impl.h | 1 + .../browser/site_per_process_browsertest.cc | 34 +++++++++++++++++++ content/common/frame_messages.h | 7 ++++ content/renderer/render_frame_impl.cc | 4 +++ content/renderer/render_frame_impl.h | 1 + content/renderer/render_frame_proxy.cc | 5 +++ content/renderer/render_frame_proxy.h | 1 + content/test/data/frame_with_load_event.html | 10 ++++++ 9 files changed, 79 insertions(+) create mode 100644 content/test/data/frame_with_load_event.html diff --git a/content/browser/frame_host/render_frame_host_impl.cc b/content/browser/frame_host/render_frame_host_impl.cc index 6b248ecd6ea08..38f4192c62ab9 100644 --- a/content/browser/frame_host/render_frame_host_impl.cc +++ b/content/browser/frame_host/render_frame_host_impl.cc @@ -373,6 +373,7 @@ bool RenderFrameHostImpl::OnMessageReceived(const IPC::Message &msg) { IPC_MESSAGE_HANDLER(FrameHostMsg_UpdateEncoding, OnUpdateEncoding) IPC_MESSAGE_HANDLER(FrameHostMsg_BeginNavigation, OnBeginNavigation) + IPC_MESSAGE_HANDLER(FrameHostMsg_DispatchLoad, OnDispatchLoad) IPC_MESSAGE_HANDLER(FrameHostMsg_TextSurroundingSelectionResponse, OnTextSurroundingSelectionResponse) IPC_MESSAGE_HANDLER(AccessibilityHostMsg_Events, OnAccessibilityEvents) @@ -1223,6 +1224,21 @@ void RenderFrameHostImpl::OnBeginNavigation( frame_tree_node(), common_params, begin_params, body); } +void RenderFrameHostImpl::OnDispatchLoad() { + CHECK(base::CommandLine::ForCurrentProcess()->HasSwitch( + switches::kSitePerProcess)); + // Only frames with an out-of-process parent frame should be sending this + // message. + RenderFrameProxyHost* proxy = + frame_tree_node()->render_manager()->GetProxyToParent(); + if (!proxy) { + GetProcess()->ReceivedBadMessage(); + return; + } + + proxy->Send(new FrameMsg_DispatchLoad(proxy->GetRoutingID())); +} + void RenderFrameHostImpl::OnAccessibilityEvents( const std::vector& params, int reset_token) { diff --git a/content/browser/frame_host/render_frame_host_impl.h b/content/browser/frame_host/render_frame_host_impl.h index 71afa041fd9bb..ca442d27367f9 100644 --- a/content/browser/frame_host/render_frame_host_impl.h +++ b/content/browser/frame_host/render_frame_host_impl.h @@ -501,6 +501,7 @@ class CONTENT_EXPORT RenderFrameHostImpl void OnBeginNavigation(const CommonNavigationParams& common_params, const BeginNavigationParams& begin_params, scoped_refptr body); + void OnDispatchLoad(); void OnAccessibilityEvents( const std::vector& params, int reset_token); diff --git a/content/browser/site_per_process_browsertest.cc b/content/browser/site_per_process_browsertest.cc index d7384299a521a..f2a76b9313877 100644 --- a/content/browser/site_per_process_browsertest.cc +++ b/content/browser/site_per_process_browsertest.cc @@ -1353,4 +1353,38 @@ IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest, EXPECT_TRUE(node3->current_frame_host()->IsRenderFrameLive()); } +// Verify that load events for iframe elements work when the child frame is +// out-of-process. In such cases, the load event is forwarded from the child +// frame to the parent frame via the browser process. +IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest, LoadEventForwarding) { + // Load a page with a cross-site frame. The parent page has an onload + // handler in the iframe element that appends "LOADED" to the document title. + { + GURL main_url( + embedded_test_server()->GetURL("/frame_with_load_event.html")); + base::string16 expected_title(base::UTF8ToUTF16("LOADED")); + TitleWatcher title_watcher(shell()->web_contents(), expected_title); + EXPECT_TRUE(NavigateToURL(shell(), main_url)); + EXPECT_EQ(title_watcher.WaitAndGetTitle(), expected_title); + } + + // It is safe to obtain the root frame tree node here, as it doesn't change. + FrameTreeNode* root = static_cast(shell()->web_contents()) + ->GetFrameTree() + ->root(); + + // Load another cross-site page into the iframe and check that the load event + // is fired. + { + GURL foo_url(embedded_test_server()->GetURL("foo.com", "/title1.html")); + base::string16 expected_title(base::UTF8ToUTF16("LOADEDLOADED")); + TitleWatcher title_watcher(shell()->web_contents(), expected_title); + TestNavigationObserver observer(shell()->web_contents()); + NavigateFrameToURL(root->child_at(0), foo_url); + EXPECT_TRUE(observer.last_navigation_succeeded()); + EXPECT_EQ(foo_url, observer.last_navigation_url()); + EXPECT_EQ(title_watcher.WaitAndGetTitle(), expected_title); + } +} + } // namespace content diff --git a/content/common/frame_messages.h b/content/common/frame_messages.h index 5a7fc0d1b0c1d..a9ca4b86796c2 100644 --- a/content/common/frame_messages.h +++ b/content/common/frame_messages.h @@ -521,6 +521,9 @@ IPC_MESSAGE_ROUTED1(FrameMsg_AddStyleSheetByURL, std::string) IPC_MESSAGE_ROUTED1(FrameMsg_SetAccessibilityMode, AccessibilityMode) +// Dispatch a load event in the iframe element containing this frame. +IPC_MESSAGE_ROUTED0(FrameMsg_DispatchLoad) + #if defined(OS_ANDROID) // External popup menus. @@ -861,6 +864,10 @@ IPC_MESSAGE_ROUTED1(FrameHostMsg_BeforeUnloadHandlersPresent, IPC_MESSAGE_ROUTED1(FrameHostMsg_UnloadHandlersPresent, bool /* present */) +// Dispatch a load event for this frame in the iframe element of an +// out-of-process parent frame. +IPC_MESSAGE_ROUTED0(FrameHostMsg_DispatchLoad) + #if defined(OS_MACOSX) || defined(OS_ANDROID) // Message to show/hide a popup menu using native controls. diff --git a/content/renderer/render_frame_impl.cc b/content/renderer/render_frame_impl.cc index 515eb1c853763..b3bca5aff5593 100644 --- a/content/renderer/render_frame_impl.cc +++ b/content/renderer/render_frame_impl.cc @@ -2768,6 +2768,10 @@ void RenderFrameImpl::didChangeThemeColor() { routing_id_, frame_->document().themeColor())); } +void RenderFrameImpl::dispatchLoad() { + Send(new FrameHostMsg_DispatchLoad(routing_id_)); +} + void RenderFrameImpl::requestNotificationPermission( const blink::WebSecurityOrigin& origin, blink::WebNotificationPermissionCallback* callback) { diff --git a/content/renderer/render_frame_impl.h b/content/renderer/render_frame_impl.h index 938c9e41e1c94..9512149246196 100644 --- a/content/renderer/render_frame_impl.h +++ b/content/renderer/render_frame_impl.h @@ -423,6 +423,7 @@ class CONTENT_EXPORT RenderFrameImpl virtual void addNavigationTransitionData( const blink::WebTransitionElementData& data); virtual void didChangeThemeColor(); + virtual void dispatchLoad(); virtual void requestNotificationPermission( const blink::WebSecurityOrigin& origin, blink::WebNotificationPermissionCallback* callback); diff --git a/content/renderer/render_frame_proxy.cc b/content/renderer/render_frame_proxy.cc index e98add45510e8..733a0bbf2ea87 100644 --- a/content/renderer/render_frame_proxy.cc +++ b/content/renderer/render_frame_proxy.cc @@ -185,6 +185,7 @@ bool RenderFrameProxy::OnMessageReceived(const IPC::Message& msg) { IPC_MESSAGE_HANDLER(FrameMsg_DisownOpener, OnDisownOpener) IPC_MESSAGE_HANDLER(FrameMsg_DidStartLoading, OnDidStartLoading) IPC_MESSAGE_HANDLER(FrameMsg_DidStopLoading, OnDidStopLoading) + IPC_MESSAGE_HANDLER(FrameMsg_DispatchLoad, OnDispatchLoad) IPC_MESSAGE_UNHANDLED(handled = false) IPC_END_MESSAGE_MAP() @@ -262,6 +263,10 @@ void RenderFrameProxy::OnDidStopLoading() { web_frame_->didStopLoading(); } +void RenderFrameProxy::OnDispatchLoad() { + web_frame_->DispatchLoadEventForFrameOwner(); +} + void RenderFrameProxy::frameDetached() { if (web_frame_->parent()) web_frame_->parent()->removeChild(web_frame_); diff --git a/content/renderer/render_frame_proxy.h b/content/renderer/render_frame_proxy.h index 4dcf4844d3fe2..8367c42560214 100644 --- a/content/renderer/render_frame_proxy.h +++ b/content/renderer/render_frame_proxy.h @@ -140,6 +140,7 @@ class CONTENT_EXPORT RenderFrameProxy void OnCompositorFrameSwapped(const IPC::Message& message); void OnDisownOpener(); void OnDidStopLoading(); + void OnDispatchLoad(); // The routing ID by which this RenderFrameProxy is known. const int routing_id_; diff --git a/content/test/data/frame_with_load_event.html b/content/test/data/frame_with_load_event.html new file mode 100644 index 0000000000000..22cd085856f5e --- /dev/null +++ b/content/test/data/frame_with_load_event.html @@ -0,0 +1,10 @@ + + + + + + This page has a cross-site iframe with a load event. + + + +