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

Unified Diff: chrome/browser/task_manager/task_manager_browsertest.cc

Issue 2857263003: Task Manager should listen to WebContentsObserver::RenderFrameCreated. (Closed)
Patch Set: Tweaked the comments as suggested in the CR feedback Created 3 years, 7 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
Index: chrome/browser/task_manager/task_manager_browsertest.cc
diff --git a/chrome/browser/task_manager/task_manager_browsertest.cc b/chrome/browser/task_manager/task_manager_browsertest.cc
index 4ebe53da14c0d2117eb403ba00079e4474d7c5ad..51e133c5d75743e3fe9bdca0bb700e10838b202d 100644
--- a/chrome/browser/task_manager/task_manager_browsertest.cc
+++ b/chrome/browser/task_manager/task_manager_browsertest.cc
@@ -43,6 +43,7 @@
#include "content/public/common/content_switches.h"
#include "content/public/test/browser_test_utils.h"
#include "content/public/test/content_browser_test_utils.h"
+#include "content/public/test/test_navigation_observer.h"
#include "extensions/browser/extension_system.h"
#include "extensions/common/extension.h"
#include "net/dns/mock_host_resolver.h"
@@ -51,6 +52,7 @@
#include "testing/gtest/include/gtest/gtest.h"
#include "ui/base/l10n/l10n_util.h"
#include "ui/base/page_transition_types.h"
+#include "url/gurl.h"
using content::WebContents;
using task_manager::browsertest_util::ColumnSpecifier;
@@ -214,7 +216,12 @@ class TaskManagerOOPIFBrowserTest : public TaskManagerBrowserTest,
DISALLOW_COPY_AND_ASSIGN(TaskManagerOOPIFBrowserTest);
};
-INSTANTIATE_TEST_CASE_P(, TaskManagerOOPIFBrowserTest, ::testing::Bool());
+INSTANTIATE_TEST_CASE_P(DefaultIsolation,
+ TaskManagerOOPIFBrowserTest,
+ ::testing::Values(false));
+INSTANTIATE_TEST_CASE_P(SitePerProcess,
+ TaskManagerOOPIFBrowserTest,
+ ::testing::Values(true));
#if defined(OS_MACOSX) || defined(OS_LINUX)
#define MAYBE_ShutdownWhileOpen DISABLED_ShutdownWhileOpen
@@ -789,6 +796,8 @@ IN_PROC_BROWSER_TEST_F(TaskManagerBrowserTest, DevToolsOldUndockedWindow) {
IN_PROC_BROWSER_TEST_P(TaskManagerOOPIFBrowserTest, KillSubframe) {
ShowTaskManager();
+ content::TestNavigationObserver navigation_observer(
+ browser()->tab_strip_model()->GetActiveWebContents());
GURL main_url(embedded_test_server()->GetURL(
"/cross-site/a.com/iframe_cross_site.html"));
browser()->OpenURL(content::OpenURLParams(main_url, content::Referrer(),
@@ -799,6 +808,7 @@ IN_PROC_BROWSER_TEST_P(TaskManagerOOPIFBrowserTest, KillSubframe) {
WaitForTaskManagerRows(1, MatchTab("cross-site iframe test")));
ASSERT_NO_FATAL_FAILURE(WaitForTaskManagerRows(1, MatchAnyTab()));
+ GURL b_url;
if (!ShouldExpectSubframes()) {
ASSERT_NO_FATAL_FAILURE(WaitForTaskManagerRows(0, MatchAnySubframe()));
} else {
@@ -807,10 +817,22 @@ IN_PROC_BROWSER_TEST_P(TaskManagerOOPIFBrowserTest, KillSubframe) {
ASSERT_NO_FATAL_FAILURE(
WaitForTaskManagerRows(1, MatchSubframe("http://c.com/")));
ASSERT_NO_FATAL_FAILURE(WaitForTaskManagerRows(2, MatchAnySubframe()));
+
+ // Remember |b_url| to be able to later renavigate to the same URL without
+ // doing any process swaps (we want to avoid redirects that would happen
+ // when going through /cross-site/foo.com/..., because
+ // https://crbug.com/642958 wouldn't repro in presence of process swaps).
+ navigation_observer.Wait();
+ b_url = browser()
+ ->tab_strip_model()
+ ->GetActiveWebContents()
+ ->GetAllFrames()[1]
+ ->GetLastCommittedURL();
+ ASSERT_EQ(b_url.host(), "b.com"); // Sanity check of test code / setup.
+
int subframe_b = FindResourceIndex(MatchSubframe("http://b.com/"));
ASSERT_NE(-1, subframe_b);
ASSERT_NE(-1, model()->GetTabId(subframe_b));
-
model()->Kill(subframe_b);
ASSERT_NO_FATAL_FAILURE(
@@ -835,6 +857,19 @@ IN_PROC_BROWSER_TEST_P(TaskManagerOOPIFBrowserTest, KillSubframe) {
ASSERT_NO_FATAL_FAILURE(WaitForTaskManagerRows(1, MatchAnySubframe()));
ASSERT_NO_FATAL_FAILURE(
WaitForTaskManagerRows(1, MatchTab("cross-site iframe test")));
+
+ // Reload the subframe and verify it has re-appeared in the task manager.
+ // This is a regression test for https://crbug.com/642958.
+ ASSERT_TRUE(content::ExecuteScript(
+ browser()->tab_strip_model()->GetActiveWebContents()->GetMainFrame(),
+ "document.getElementById('frame1').src = '" + b_url.spec() + "';"));
+ ASSERT_NO_FATAL_FAILURE(
+ WaitForTaskManagerRows(1, MatchSubframe("http://b.com/")));
+ ASSERT_NO_FATAL_FAILURE(
+ WaitForTaskManagerRows(1, MatchSubframe("http://c.com/")));
+ ASSERT_NO_FATAL_FAILURE(WaitForTaskManagerRows(2, MatchAnySubframe()));
+ ASSERT_NO_FATAL_FAILURE(
+ WaitForTaskManagerRows(1, MatchTab("cross-site iframe test")));
}
}
@@ -945,6 +980,8 @@ IN_PROC_BROWSER_TEST_P(TaskManagerOOPIFBrowserTest,
// Navigate the tab to a page on a.com with cross-process subframes to
// b.com and c.com.
+ content::TestNavigationObserver navigation_observer(
+ browser()->tab_strip_model()->GetActiveWebContents());
GURL a_dotcom(embedded_test_server()->GetURL(
"/cross-site/a.com/iframe_cross_site.html"));
browser()->OpenURL(content::OpenURLParams(a_dotcom, content::Referrer(),
@@ -966,10 +1003,11 @@ IN_PROC_BROWSER_TEST_P(TaskManagerOOPIFBrowserTest,
}
// Navigate the b.com frame back to a.com. It is no longer a cross-site iframe
+ navigation_observer.Wait();
ASSERT_TRUE(content::ExecuteScript(
browser()->tab_strip_model()->GetActiveWebContents()->GetMainFrame(),
- "document.getElementById('frame1').src='/title1.html';"
- "document.title='aac';"));
+ R"( document.getElementById('frame1').src='/title1.html';
+ document.title='aac'; )"));
ASSERT_NO_FATAL_FAILURE(WaitForTaskManagerRows(1, MatchTab("aac")));
ASSERT_NO_FATAL_FAILURE(WaitForTaskManagerRows(1, MatchAnyTab()));
if (!ShouldExpectSubframes()) {

Powered by Google App Engine
This is Rietveld 408576698