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

Unified Diff: chrome/browser/site_details_browsertest.cc

Issue 1313863006: Add SiteIsolation.IsolateExtensions metrics and tests. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Attempt to fix clang compile warning. Created 5 years, 3 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/site_details_browsertest.cc
diff --git a/chrome/browser/site_details_browsertest.cc b/chrome/browser/site_details_browsertest.cc
index b84be4c35683083c48fd03b4448c0c2d97fc105d..f6671c9e4ad1cd6d4b7868868d72bf0153653cfb 100644
--- a/chrome/browser/site_details_browsertest.cc
+++ b/chrome/browser/site_details_browsertest.cc
@@ -5,22 +5,33 @@
#include "chrome/browser/site_details.h"
#include "base/bind_helpers.h"
+#include "base/files/file_path.h"
#include "base/message_loop/message_loop.h"
#include "base/path_service.h"
#include "base/test/histogram_tester.h"
+#include "chrome/browser/extensions/extension_browsertest.h"
+#include "chrome/browser/extensions/test_extension_dir.h"
#include "chrome/browser/metrics/metrics_memory_details.h"
#include "chrome/browser/ui/browser.h"
#include "chrome/browser/ui/tabs/tab_strip_model.h"
+#include "chrome/common/url_constants.h"
#include "chrome/test/base/in_process_browser_test.h"
#include "chrome/test/base/ui_test_utils.h"
#include "content/public/browser/notification_service.h"
#include "content/public/test/browser_test_utils.h"
#include "content/public/test/test_utils.h"
+#include "extensions/common/value_builder.h"
#include "net/dns/mock_host_resolver.h"
#include "net/test/embedded_test_server/embedded_test_server.h"
#include "testing/gmock/include/gmock/gmock.h"
+#include "testing/gtest/include/gtest/gtest.h"
using base::Bucket;
+using content::WebContents;
+using extensions::DictionaryBuilder;
+using extensions::Extension;
+using extensions::ListBuilder;
+using extensions::TestExtensionDir;
using testing::ContainerEq;
using testing::ElementsAre;
@@ -57,7 +68,7 @@ class TestMemoryDetails : public MetricsMemoryDetails {
} // namespace
-class SiteDetailsBrowserTest : public InProcessBrowserTest {
+class SiteDetailsBrowserTest : public ExtensionBrowserTest {
public:
SiteDetailsBrowserTest() {}
~SiteDetailsBrowserTest() override {}
@@ -73,14 +84,55 @@ class SiteDetailsBrowserTest : public InProcessBrowserTest {
ASSERT_TRUE(embedded_test_server()->InitializeAndWaitUntilReady());
}
+ // Create and install an extension that has a couple of web-accessible
+ // resources and, optionally, a background process.
+ const Extension* CreateExtension(const std::string& name,
nasko 2015/09/04 22:19:43 Why create extension by writing it to the filesyst
ncarter (slow) 2015/09/10 19:08:59 First off: I'm using an established practice -- dy
nasko 2015/09/11 18:14:11 Thanks for the detailed answer!
+ bool has_background_process) {
+ scoped_ptr<TestExtensionDir> dir(new TestExtensionDir);
+
+ DictionaryBuilder manifest;
+ manifest.Set("name", name)
+ .Set("version", "1.0")
+ .Set("manifest_version", 2)
+ .Set("web_accessible_resources", ListBuilder()
+ .Append("blank_iframe.html")
+ .Append("http_iframe.html"));
+
+ if (has_background_process) {
+ manifest.Set("background",
+ DictionaryBuilder().Set("scripts",
+ ListBuilder().Append("script.js")));
+ dir->WriteFile(FILE_PATH_LITERAL("script.js"),
+ "console.log('" + name + " running');");
+ }
+
+ dir->WriteFile(FILE_PATH_LITERAL("blank_iframe.html"),
+ "<html><body>" + name +
+ ", blank iframe: "
+ "<iframe width=40 height=40></iframe></body></html>");
+ GURL iframe_url = embedded_test_server()->GetURL("w.com", "/title1.html");
+ dir->WriteFile(FILE_PATH_LITERAL("http_iframe.html"),
+ "<html><body>" + name +
+ ", http:// iframe: "
+ "<iframe width=40 height=40 src='" +
+ iframe_url.spec() + "'></iframe></body></html>");
+ dir->WriteManifest(manifest.ToJSON());
+
+ const Extension* extension = LoadExtension(dir->unpacked_path());
+ EXPECT_TRUE(extension);
+ temp_dirs_.push_back(dir.release());
+ return extension;
+ }
+
private:
+ ScopedVector<TestExtensionDir> temp_dirs_;
DISALLOW_COPY_AND_ASSIGN(SiteDetailsBrowserTest);
};
// Test the accuracy of SiteDetails process estimation, in the presence of
// multiple iframes, navigation, multiple BrowsingInstances, and multiple tabs
// in the same BrowsingInstance.
-IN_PROC_BROWSER_TEST_F(SiteDetailsBrowserTest, ManyCrossSiteIframes) {
+IN_PROC_BROWSER_TEST_F(SiteDetailsBrowserTest, ManyIframes) {
// Page with 14 nested oopifs across 9 sites (a.com through i.com).
// None of these are https.
GURL abcdefghi_url = embedded_test_server()->GetURL(
@@ -116,6 +168,15 @@ IN_PROC_BROWSER_TEST_F(SiteDetailsBrowserTest, ManyCrossSiteIframes) {
EXPECT_THAT(details->uma()->GetAllSamples(
"SiteIsolation.IsolateHttpsSitesProcessCountNoLimit"),
ElementsAre(Bucket(1, 1)));
+ EXPECT_THAT(details->uma()->GetAllSamples(
+ "SiteIsolation.IsolateExtensionsProcessCountEstimate"),
+ ElementsAre(Bucket(1, 1)));
+ EXPECT_THAT(details->uma()->GetAllSamples(
+ "SiteIsolation.IsolateExtensionsProcessCountLowerBound"),
+ ElementsAre(Bucket(1, 1)));
+ EXPECT_THAT(details->uma()->GetAllSamples(
+ "SiteIsolation.IsolateExtensionsProcessCountNoLimit"),
+ ElementsAre(Bucket(1, 1)));
// Navigate to a different, disjoint set of 7 sites.
GURL pqrstuv_url = embedded_test_server()->GetURL(
@@ -150,6 +211,15 @@ IN_PROC_BROWSER_TEST_F(SiteDetailsBrowserTest, ManyCrossSiteIframes) {
EXPECT_THAT(details->uma()->GetAllSamples(
"SiteIsolation.IsolateHttpsSitesProcessCountNoLimit"),
ElementsAre(Bucket(1, 1)));
+ EXPECT_THAT(details->uma()->GetAllSamples(
+ "SiteIsolation.IsolateExtensionsProcessCountEstimate"),
+ ElementsAre(Bucket(1, 1)));
+ EXPECT_THAT(details->uma()->GetAllSamples(
+ "SiteIsolation.IsolateExtensionsProcessCountLowerBound"),
+ ElementsAre(Bucket(1, 1)));
+ EXPECT_THAT(details->uma()->GetAllSamples(
+ "SiteIsolation.IsolateExtensionsProcessCountNoLimit"),
+ ElementsAre(Bucket(1, 1)));
// Open a second tab (different BrowsingInstance) with 4 sites (a through d).
GURL abcd_url = embedded_test_server()->GetURL(
@@ -183,6 +253,15 @@ IN_PROC_BROWSER_TEST_F(SiteDetailsBrowserTest, ManyCrossSiteIframes) {
EXPECT_THAT(details->uma()->GetAllSamples(
"SiteIsolation.IsolateHttpsSitesProcessCountNoLimit"),
ElementsAre(Bucket(2, 1)));
+ EXPECT_THAT(details->uma()->GetAllSamples(
+ "SiteIsolation.IsolateExtensionsProcessCountEstimate"),
+ ElementsAre(Bucket(2, 1)));
+ EXPECT_THAT(details->uma()->GetAllSamples(
+ "SiteIsolation.IsolateExtensionsProcessCountLowerBound"),
+ ElementsAre(Bucket(1, 1)));
+ EXPECT_THAT(details->uma()->GetAllSamples(
+ "SiteIsolation.IsolateExtensionsProcessCountNoLimit"),
+ ElementsAre(Bucket(2, 1)));
// Open a third tab (different BrowsingInstance) with the same 4 sites.
AddTabAtIndex(2, abcd_url, ui::PAGE_TRANSITION_TYPED);
@@ -215,11 +294,20 @@ IN_PROC_BROWSER_TEST_F(SiteDetailsBrowserTest, ManyCrossSiteIframes) {
EXPECT_THAT(details->uma()->GetAllSamples(
"SiteIsolation.IsolateHttpsSitesProcessCountNoLimit"),
ElementsAre(Bucket(3, 1)));
+ EXPECT_THAT(details->uma()->GetAllSamples(
+ "SiteIsolation.IsolateExtensionsProcessCountEstimate"),
+ ElementsAre(Bucket(3, 1)));
+ EXPECT_THAT(details->uma()->GetAllSamples(
+ "SiteIsolation.IsolateExtensionsProcessCountLowerBound"),
+ ElementsAre(Bucket(1, 1)));
+ EXPECT_THAT(details->uma()->GetAllSamples(
+ "SiteIsolation.IsolateExtensionsProcessCountNoLimit"),
+ ElementsAre(Bucket(3, 1)));
- // Do a window.open() to obtain a fourth tab in the same BrowsingInstance as
- // the third tab. The new page uses the same four sites "a-d" as third tab,
+ // From the third tab, window.open() a fourth tab in the same
+ // BrowsingInstance, to a page using the same four sites "a-d" as third tab,
// plus an additional site "e". The estimated process counts should increase
- // by one (not five) from the previous result, since the new tab can reuse the
+ // by one (not five) from the previous scenario, as the new tab can reuse the
// four processes already in the BrowsingInstance.
GURL dcbae_url = embedded_test_server()->GetURL(
"a.com", "/cross_site_iframe_factory.html?d(c(b(a(e))))");
@@ -260,4 +348,228 @@ IN_PROC_BROWSER_TEST_F(SiteDetailsBrowserTest, ManyCrossSiteIframes) {
EXPECT_THAT(details->uma()->GetAllSamples(
"SiteIsolation.IsolateHttpsSitesProcessCountNoLimit"),
ElementsAre(Bucket(3, 1)));
+ EXPECT_THAT(details->uma()->GetAllSamples(
+ "SiteIsolation.IsolateExtensionsProcessCountEstimate"),
+ ElementsAre(Bucket(3, 1)));
+ EXPECT_THAT(details->uma()->GetAllSamples(
+ "SiteIsolation.IsolateExtensionsProcessCountLowerBound"),
+ ElementsAre(Bucket(1, 1)));
+ EXPECT_THAT(details->uma()->GetAllSamples(
+ "SiteIsolation.IsolateExtensionsProcessCountNoLimit"),
+ ElementsAre(Bucket(3, 1)));
+}
+
+IN_PROC_BROWSER_TEST_F(SiteDetailsBrowserTest, IsolateExtensions) {
+ // We start on "about:blank", which should be credited with a process in this
+ // case.
+ scoped_refptr<TestMemoryDetails> details = new TestMemoryDetails();
+ details->StartFetchAndWait();
+ EXPECT_THAT(details->uma()->GetAllSamples(
+ "SiteIsolation.CurrentRendererProcessCount"),
+ ElementsAre(Bucket(1, 1)));
+ EXPECT_THAT(details->uma()->GetAllSamples(
+ "SiteIsolation.IsolateExtensionsProcessCountEstimate"),
+ ElementsAre(Bucket(1, 1)));
+ EXPECT_THAT(details->uma()->GetAllSamples(
+ "SiteIsolation.IsolateExtensionsProcessCountLowerBound"),
+ ElementsAre(Bucket(1, 1)));
+ EXPECT_THAT(details->uma()->GetAllSamples(
+ "SiteIsolation.IsolateExtensionsProcessCountNoLimit"),
+ ElementsAre(Bucket(1, 1)));
+
+ // Install one script-injecting extension with background page, and an
+ // extension with web accessible resources.
+ const Extension* extension1 = CreateExtension("Extension One", true);
+ const Extension* extension2 = CreateExtension("Extension Two", false);
+
+ // Open two a.com tabs (with cross site http iframes). IsolateExtensions mode
+ // should have no effect so far, since there are no frames straddling the
+ // extension/web boundary.
+ GURL tab1_url = embedded_test_server()->GetURL(
+ "a.com", "/cross_site_iframe_factory.html?a(b,c)");
+ ui_test_utils::NavigateToURL(browser(), tab1_url);
+ WebContents* tab1 = browser()->tab_strip_model()->GetWebContentsAt(0);
+ GURL tab2_url = embedded_test_server()->GetURL(
+ "a.com", "/cross_site_iframe_factory.html?a(d,e)");
+ AddTabAtIndex(1, tab2_url, ui::PAGE_TRANSITION_TYPED);
+ WebContents* tab2 = browser()->tab_strip_model()->GetWebContentsAt(1);
+
+ details = new TestMemoryDetails();
+ details->StartFetchAndWait();
+ EXPECT_THAT(details->uma()->GetAllSamples(
+ "SiteIsolation.CurrentRendererProcessCount"),
+ ElementsAre(Bucket(3, 1)));
+ EXPECT_THAT(details->uma()->GetAllSamples(
+ "SiteIsolation.IsolateExtensionsProcessCountEstimate"),
+ ElementsAre(Bucket(3, 1)));
+ EXPECT_THAT(details->uma()->GetAllSamples(
+ "SiteIsolation.IsolateExtensionsProcessCountLowerBound"),
+ ElementsAre(Bucket(2, 1)));
+ EXPECT_THAT(details->uma()->GetAllSamples(
+ "SiteIsolation.IsolateExtensionsProcessCountNoLimit"),
+ ElementsAre(Bucket(3, 1)));
+
+ // Test that "one process per extension" applies even when web content has an
+ // extension iframe.
+
+ // Tab1 navigates its first iframe to a resource of extension1. This shouldn't
+ // result in a new extension process (it should share with extension1's
+ // background page).
+ content::NavigateIframeToURL(
+ tab1, "child-0", extension1->GetResourceURL("/blank_iframe.html"));
+ details = new TestMemoryDetails();
+ details->StartFetchAndWait();
+ EXPECT_THAT(details->uma()->GetAllSamples(
+ "SiteIsolation.CurrentRendererProcessCount"),
+ ElementsAre(Bucket(3, 1)));
+ EXPECT_THAT(details->uma()->GetAllSamples(
+ "SiteIsolation.IsolateExtensionsProcessCountEstimate"),
+ ElementsAre(Bucket(3, 1)));
+ EXPECT_THAT(details->uma()->GetAllSamples(
+ "SiteIsolation.IsolateExtensionsProcessCountLowerBound"),
+ ElementsAre(Bucket(2, 1)));
+ EXPECT_THAT(details->uma()->GetAllSamples(
+ "SiteIsolation.IsolateExtensionsProcessCountNoLimit"),
+ ElementsAre(Bucket(3, 1)));
+
+ // Tab2 navigates its first iframe to a resource of extension1. This also
+ // shouldn't result in a new extension process (it should share with the
+ // background page and the other iframe).
+ content::NavigateIframeToURL(
+ tab2, "child-0", extension1->GetResourceURL("/blank_iframe.html"));
+ details = new TestMemoryDetails();
+ details->StartFetchAndWait();
+ EXPECT_THAT(details->uma()->GetAllSamples(
+ "SiteIsolation.CurrentRendererProcessCount"),
+ ElementsAre(Bucket(3, 1)));
+ EXPECT_THAT(details->uma()->GetAllSamples(
+ "SiteIsolation.IsolateExtensionsProcessCountEstimate"),
+ ElementsAre(Bucket(3, 1)));
+ EXPECT_THAT(details->uma()->GetAllSamples(
+ "SiteIsolation.IsolateExtensionsProcessCountLowerBound"),
+ ElementsAre(Bucket(2, 1)));
+ EXPECT_THAT(details->uma()->GetAllSamples(
+ "SiteIsolation.IsolateExtensionsProcessCountNoLimit"),
+ ElementsAre(Bucket(3, 1)));
+
+ // Tab1 navigates its second iframe to a resource of extension2. This SHOULD
+ // result in a new process since extension2 had no existing process.
+ content::NavigateIframeToURL(
+ tab1, "child-1", extension2->GetResourceURL("/blank_iframe.html"));
+ details = new TestMemoryDetails();
+ details->StartFetchAndWait();
+ EXPECT_THAT(details->uma()->GetAllSamples(
+ "SiteIsolation.CurrentRendererProcessCount"),
+ ElementsAre(Bucket(3, 1)));
+ EXPECT_THAT(details->uma()->GetAllSamples(
+ "SiteIsolation.IsolateExtensionsProcessCountEstimate"),
+ ElementsAre(Bucket(4, 1)));
+ EXPECT_THAT(details->uma()->GetAllSamples(
+ "SiteIsolation.IsolateExtensionsProcessCountLowerBound"),
+ ElementsAre(Bucket(3, 1)));
+ EXPECT_THAT(details->uma()->GetAllSamples(
+ "SiteIsolation.IsolateExtensionsProcessCountNoLimit"),
+ ElementsAre(Bucket(4, 1)));
+
+ // Tab2 navigates its second iframe to a resource of extension2. This should
+ // share the existing extension2 process.
+ content::NavigateIframeToURL(
+ tab2, "child-1", extension2->GetResourceURL("/blank_iframe.html"));
+ details = new TestMemoryDetails();
+ details->StartFetchAndWait();
+ EXPECT_THAT(details->uma()->GetAllSamples(
+ "SiteIsolation.CurrentRendererProcessCount"),
+ ElementsAre(Bucket(3, 1)));
+ EXPECT_THAT(details->uma()->GetAllSamples(
+ "SiteIsolation.IsolateExtensionsProcessCountEstimate"),
+ ElementsAre(Bucket(4, 1)));
+ EXPECT_THAT(details->uma()->GetAllSamples(
+ "SiteIsolation.IsolateExtensionsProcessCountLowerBound"),
+ ElementsAre(Bucket(3, 1)));
+ EXPECT_THAT(details->uma()->GetAllSamples(
+ "SiteIsolation.IsolateExtensionsProcessCountNoLimit"),
+ ElementsAre(Bucket(4, 1)));
+
+ // Install extension3 (identical config to extension2)
+ const Extension* extension3 = CreateExtension("Extension Three", false);
+
+ // Navigate Tab2 to a top-level page from extension3.
nasko 2015/09/04 22:19:43 It is useful to state verbally what we expect to c
ncarter (slow) 2015/09/10 19:08:59 Done.
+ ui_test_utils::NavigateToURL(browser(),
+ extension3->GetResourceURL("blank_iframe.html"));
+ details = new TestMemoryDetails();
+ details->StartFetchAndWait();
+ EXPECT_THAT(details->uma()->GetAllSamples(
+ "SiteIsolation.CurrentRendererProcessCount"),
+ ElementsAre(Bucket(3, 1)));
+ EXPECT_THAT(details->uma()->GetAllSamples(
+ "SiteIsolation.IsolateExtensionsProcessCountEstimate"),
+ ElementsAre(Bucket(4, 1)));
+ EXPECT_THAT(details->uma()->GetAllSamples(
+ "SiteIsolation.IsolateExtensionsProcessCountLowerBound"),
+ ElementsAre(Bucket(4, 1)));
+ EXPECT_THAT(details->uma()->GetAllSamples(
+ "SiteIsolation.IsolateExtensionsProcessCountNoLimit"),
+ ElementsAre(Bucket(4, 1)));
+
+ // Add a web iframe to the chrome-extension:// page. This should create a new
nasko 2015/09/04 22:19:43 You aren't really adding it to a page, rather navi
ncarter (slow) 2015/09/10 19:08:59 Done.
+ // process.
+ ui_test_utils::NavigateToURL(browser(),
+ extension3->GetResourceURL("http_iframe.html"));
+ details = new TestMemoryDetails();
+ details->StartFetchAndWait();
+ EXPECT_THAT(details->uma()->GetAllSamples(
+ "SiteIsolation.CurrentRendererProcessCount"),
+ ElementsAre(Bucket(3, 1)));
+ EXPECT_THAT(details->uma()->GetAllSamples(
+ "SiteIsolation.IsolateExtensionsProcessCountEstimate"),
+ ElementsAre(Bucket(5, 1)));
+ EXPECT_THAT(details->uma()->GetAllSamples(
+ "SiteIsolation.IsolateExtensionsProcessCountLowerBound"),
+ ElementsAre(Bucket(4, 1)));
+ EXPECT_THAT(details->uma()->GetAllSamples(
+ "SiteIsolation.IsolateExtensionsProcessCountNoLimit"),
+ ElementsAre(Bucket(5, 1)));
+
+ // Navigate tab1 to the extension3 page. There should be 3 processes estimated
+ // by IsolateExtensions: one for extension3, one for extension1's background
+ // page, and one for the web iframe in tab2.
+ browser()->tab_strip_model()->ActivateTabAt(0, true);
+ ui_test_utils::NavigateToURL(browser(),
+ extension3->GetResourceURL("blank_iframe.html"));
+ details = new TestMemoryDetails();
+ details->StartFetchAndWait();
+
nasko 2015/09/04 22:19:43 No need for empty line, to stay consistent with co
ncarter (slow) 2015/09/10 19:08:59 Done.
+ EXPECT_THAT(details->uma()->GetAllSamples(
+ "SiteIsolation.CurrentRendererProcessCount"),
+ ElementsAre(Bucket(2, 1)));
+ EXPECT_THAT(details->uma()->GetAllSamples(
+ "SiteIsolation.IsolateExtensionsProcessCountEstimate"),
+ ElementsAre(Bucket(3, 1)));
+ EXPECT_THAT(details->uma()->GetAllSamples(
+ "SiteIsolation.IsolateExtensionsProcessCountLowerBound"),
+ ElementsAre(Bucket(3, 1)));
+ EXPECT_THAT(details->uma()->GetAllSamples(
+ "SiteIsolation.IsolateExtensionsProcessCountNoLimit"),
+ ElementsAre(Bucket(3, 1)));
+
+ // Put a web iframe in tab1 as well. This could share a process with tab2's
+ // iframe (the LowerBound number), or it could get its own process (the
+ // Estimate number).
+ ui_test_utils::NavigateToURL(browser(),
+ extension3->GetResourceURL("http_iframe.html"));
+ details = new TestMemoryDetails();
+ details->StartFetchAndWait();
+
nasko 2015/09/04 22:19:43 No need for empty line, to stay consistent with co
ncarter (slow) 2015/09/10 19:08:59 Done.
+ EXPECT_THAT(details->uma()->GetAllSamples(
+ "SiteIsolation.CurrentRendererProcessCount"),
+ ElementsAre(Bucket(2, 1)));
+ EXPECT_THAT(details->uma()->GetAllSamples(
+ "SiteIsolation.IsolateExtensionsProcessCountEstimate"),
+ ElementsAre(Bucket(4, 1)));
+ EXPECT_THAT(details->uma()->GetAllSamples(
+ "SiteIsolation.IsolateExtensionsProcessCountLowerBound"),
+ ElementsAre(Bucket(3, 1)));
+ EXPECT_THAT(details->uma()->GetAllSamples(
+ "SiteIsolation.IsolateExtensionsProcessCountNoLimit"),
+ ElementsAre(Bucket(4, 1)));
}

Powered by Google App Engine
This is Rietveld 408576698