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

Unified Diff: content/browser/frame_host/render_frame_host_manager_unittest.cc

Issue 196133032: Add test for filtering of IPC messages when RenderFrameHost is swapped out. (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: Created 6 years, 9 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
« no previous file with comments | « content/browser/frame_host/render_frame_host_impl.cc ('k') | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: content/browser/frame_host/render_frame_host_manager_unittest.cc
diff --git a/content/browser/frame_host/render_frame_host_manager_unittest.cc b/content/browser/frame_host/render_frame_host_manager_unittest.cc
index 2e7ad3fca74c2fda31c0c9982198e0c0386094ac..c34ee76ab390c4f163e25099b4e3c7999da31f2b 100644
--- a/content/browser/frame_host/render_frame_host_manager_unittest.cc
+++ b/content/browser/frame_host/render_frame_host_manager_unittest.cc
@@ -2,6 +2,7 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
+#include "base/files/file_path.h"
#include "base/strings/utf_string_conversions.h"
#include "content/browser/frame_host/cross_site_transferring_request.h"
#include "content/browser/frame_host/navigation_controller_impl.h"
@@ -248,6 +249,31 @@ TEST_F(RenderFrameHostManagerTest, NewTabPageProcesses) {
contents2->GetRenderViewHost()->GetSiteInstance()->GetProcess());
}
+// This observer is used to check whether IPC messages are being filtered for
+// swapped out RenderFrameHost objects. It observes the plugin crash events,
+// which the test which follows it simulates being sent. The test is succsesful
Charlie Reis 2014/03/19 20:40:57 nit: which the FilterMessagesWhileSwappedOut test
nasko 2014/03/21 20:32:56 Done.
+// if the event is not observed. See http://crbug.com/351815
+class PluginCrashObserver : public WebContentsObserver {
+ public:
+ PluginCrashObserver(WebContents* web_contents)
+ : WebContentsObserver(web_contents),
+ event_triggered_(false) {}
+
+ virtual void PluginCrashed(const base::FilePath& plugin_path,
+ base::ProcessId plugin_pid) OVERRIDE {
+ event_triggered_ = true;
+ }
+
+ bool event_triggered() {
+ return event_triggered_;
+ }
+
+ private:
+ bool event_triggered_;
+
+ DISALLOW_COPY_AND_ASSIGN(PluginCrashObserver);
+};
+
// Ensure that the browser ignores most IPC messages that arrive from a
// RenderViewHost that has been swapped out. We do not want to take
// action on requests from a non-active renderer. The main exception is
@@ -304,6 +330,19 @@ TEST_F(RenderFrameHostManagerTest, FilterMessagesWhileSwappedOut) {
ViewHostMsg_UpdateTitle(rvh()->GetRoutingID(), 0, ntp_title, direction)));
EXPECT_EQ(dest_title, contents()->GetTitle());
+ // The same logic should apply to RenderFrameHosts as well and routing through
+ // swapped out RFH shouldn't be allowed. Use a PluginCrashObserver to check
+ // if the IPC message is allowed through or not.
+ PluginCrashObserver crash_observer(contents());
+ // TODO(nasko): The following line shouldn't be needed when we are properly
+ // swapping out RFH objects or move to proxy objects.
+ ntp_rvh->main_render_frame_host()->set_swapped_out(true);
Charlie Reis 2014/03/19 20:40:57 Is this line actually needed? You're checking for
nasko 2014/03/21 20:32:56 Quite a bit has changed, so this is no longer appl
+ EXPECT_TRUE(ntp_rvh->main_render_frame_host()->is_swapped_out());
+ EXPECT_TRUE(ntp_rvh->main_render_frame_host()->OnMessageReceived(
+ FrameHostMsg_PluginCrashed(
+ main_rfh()->GetRoutingID(), base::FilePath(), 0)));
+ EXPECT_FALSE(crash_observer.event_triggered());
+
// We cannot filter out synchronous IPC messages, because the renderer would
// be left waiting for a reply. We pick RunBeforeUnloadConfirm as an example
// that can run easily within a unit test, and that needs to receive a reply
« no previous file with comments | « content/browser/frame_host/render_frame_host_impl.cc ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698