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

Unified Diff: content/browser/renderer_host/render_process_host_browsertest.cc

Issue 9751001: Don't allow a renderer to exit if we are using it in other views. (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: Add a test. Created 8 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 | « no previous file | content/browser/renderer_host/render_process_host_impl.h » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: content/browser/renderer_host/render_process_host_browsertest.cc
diff --git a/content/browser/renderer_host/render_process_host_browsertest.cc b/content/browser/renderer_host/render_process_host_browsertest.cc
index 995addc8a90c696aae3d2b4c5c1831d29922c3de..df9b604ea1d6e4d6976eca39cd91d59789f6f21c 100644
--- a/content/browser/renderer_host/render_process_host_browsertest.cc
+++ b/content/browser/renderer_host/render_process_host_browsertest.cc
@@ -6,14 +6,23 @@
#include "base/bind_helpers.h"
#include "base/command_line.h"
#include "base/process.h"
+#include "chrome/browser/chrome_content_browser_client.h"
jam 2012/03/21 23:30:12 you're going to get a checkdeps failure for this..
Charlie Reis 2012/03/21 23:44:14 Yeah, I realized that... This seems like a limita
Charlie Reis 2012/03/21 23:51:17 Actually, we've already got a render_process_host_
#include "chrome/browser/ui/browser.h"
#include "chrome/test/base/in_process_browser_test.h"
#include "chrome/test/base/ui_test_utils.h"
#include "content/browser/renderer_host/render_process_host_impl.h"
#include "content/browser/tab_contents/tab_contents.h"
+#include "content/common/child_process_messages.h"
#include "content/common/test_url_constants.h"
+#include "content/public/browser/browser_message_filter.h"
#include "content/public/browser/browser_thread.h"
+#include "content/public/browser/content_browser_client.h"
+#include "content/public/browser/notification_service.h"
+#include "content/public/browser/notification_types.h"
+#include "content/public/browser/page_navigator.h"
#include "content/public/common/content_switches.h"
+#include "content/public/common/referrer.h"
+#include "ipc/ipc_message_macros.h"
using content::WebContents;
@@ -21,6 +30,69 @@ void PostQuit(MessageLoop* loop) {
loop->PostTask(FROM_HERE, MessageLoop::QuitClosure());
}
+namespace {
+
+// Custom message filter to consume shutdown messages on demand.
+class RenderProcessHostTestMessageFilter
+ : public content::BrowserMessageFilter {
+ public:
+ RenderProcessHostTestMessageFilter() :
+ should_filter_shutdown_requests_(false) {}
+ virtual ~RenderProcessHostTestMessageFilter() {}
+
+ void set_should_filter_shutdown_requests(bool should_filter) {
+ should_filter_shutdown_requests_ = should_filter;
+ }
+
+ bool OnMessageReceived(const IPC::Message& message,
+ bool* message_was_ok) OVERRIDE {
+ if (!should_filter_shutdown_requests_)
+ return false;
+
+ bool handled = true;
+ IPC_BEGIN_MESSAGE_MAP_EX(RenderProcessHostTestMessageFilter,
+ message, *message_was_ok)
+ IPC_MESSAGE_HANDLER(ChildProcessHostMsg_ShutdownRequest,
+ OnShutdownRequest)
+ IPC_MESSAGE_UNHANDLED(handled = false)
+ IPC_END_MESSAGE_MAP()
+ return handled;
+ }
+
+ void OnShutdownRequest() {
+ // Discard the message so the browser doesn't see it.
+ }
+
+ private:
+ bool should_filter_shutdown_requests_;
+
+ DISALLOW_COPY_AND_ASSIGN(RenderProcessHostTestMessageFilter);
+};
+
+class RenderProcessHostTestBrowserClient
+ : public chrome::ChromeContentBrowserClient {
+ public:
+ RenderProcessHostTestBrowserClient() :
+ message_filter_(new RenderProcessHostTestMessageFilter()) {}
+ virtual ~RenderProcessHostTestBrowserClient() {}
+
+ // ChromeContentBrowserClient implementation.
+ virtual void RenderProcessHostCreated(content::RenderProcessHost* host)
+ OVERRIDE {
+ // Install our custom filter first.
+ host->GetChannel()->AddFilter(message_filter_);
+
+ // Install the remaining message filters.
+ ChromeContentBrowserClient::RenderProcessHostCreated(host);
+ }
+
+ scoped_refptr<RenderProcessHostTestMessageFilter> message_filter_;
+
+ DISALLOW_COPY_AND_ASSIGN(RenderProcessHostTestBrowserClient);
+};
+
+} // namespace
+
class RenderProcessHostTest : public InProcessBrowserTest {
public:
RenderProcessHostTest() {
@@ -56,6 +128,17 @@ class RenderProcessHostTest : public InProcessBrowserTest {
return wc->GetRenderProcessHost()->GetHandle();
}
+ virtual void SetUp() OVERRIDE {
+ old_browser_client_ = content::GetContentClient()->browser();
+ content::GetContentClient()->set_browser(&browser_client_);
+ InProcessBrowserTest::SetUp();
+ }
+
+ virtual void TearDown() OVERRIDE {
+ InProcessBrowserTest::TearDown();
+ content::GetContentClient()->set_browser(old_browser_client_);
+ }
+
// When we hit the max number of renderers, verify that the way we do process
// sharing behaves correctly. In particular, this test is verifying that even
// when we hit the max process limit, that renderers of each type will wind up
@@ -143,6 +226,12 @@ class RenderProcessHostTest : public InProcessBrowserTest {
EXPECT_NE(rph1, rph3);
EXPECT_NE(rph2, rph3);
}
+
+ protected:
+ RenderProcessHostTestBrowserClient browser_client_;
+
+ private:
+ content::ContentBrowserClient* old_browser_client_;
};
@@ -257,3 +346,42 @@ IN_PROC_BROWSER_TEST_F(RenderProcessHostTest, ProcessOverflow) {
IN_PROC_BROWSER_TEST_F(RenderProcessHostTestWithCommandLine, ProcessOverflow) {
TestProcessOverflow();
}
+
+// Ensure that if a second NTP is opened as the first one is exiting, we don't
+// end up killing the second NTP as well. http://crbug.com/87176.
+IN_PROC_BROWSER_TEST_F(RenderProcessHostTest, DontPutNTPInDyingProcess) {
+ // Change the first tab to be the new tab page (TYPE_WEBUI).
+ GURL newtab(chrome::kTestNewTabURL);
+ ui_test_utils::NavigateToURL(browser(), newtab);
+ content::RenderProcessHost* rph =
+ browser()->GetSelectedWebContents()->GetRenderProcessHost();
+ RenderProcessHostImpl* process = static_cast<RenderProcessHostImpl*>(rph);
+
+ // Before navigating away, make sure the ShutdownRequest message is "delayed"
+ // by consuming it and sending a new one later.
+ browser_client_.message_filter_->set_should_filter_shutdown_requests(true);
+
+ // Now navigate to a page that causes a process swap, causing the NTP's
+ // process to send a ShutdownRequest message (which we "delay").
+ GURL page1("data:text/html,hello world1");
+ ui_test_utils::NavigateToURL(browser(), page1);
+
+ // Before the ShutdownRequest message arrives, open a new tab with the NTP.
+ ui_test_utils::WindowedNotificationObserver nav_observer(
+ content::NOTIFICATION_NAV_ENTRY_COMMITTED,
+ content::NotificationService::AllSources());
+ ui_test_utils::NavigateToURLWithDisposition(
+ browser(), newtab, NEW_FOREGROUND_TAB,
+ ui_test_utils::BROWSER_TEST_WAIT_FOR_TAB);
+
+ // After the tab opens but before the navigation completes, simulate the
+ // ShutdownRequest message arriving.
+ browser_client_.message_filter_->set_should_filter_shutdown_requests(false);
+ process->OnShutdownRequest();
+
+ // Wait for the NTP navigation to finish.
+ nav_observer.Wait();
+ WebContents* contents = browser()->GetSelectedWebContents();
+ EXPECT_EQ(process, contents->GetRenderProcessHost());
+ EXPECT_TRUE(process->HasConnection());
+}
« no previous file with comments | « no previous file | content/browser/renderer_host/render_process_host_impl.h » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698