Don't swap processes for javascript: URLs.

content/browser/site_instance_unittest.cc

Index: content/browser/site_instance_unittest.cc
diff --git a/content/browser/site_instance_unittest.cc b/content/browser/site_instance_unittest.cc
index dcfcd3efbfb01f380891f7330c078f75f2a3c91c..43a8e1dbdb6689c5d3ff6964c22efcf77233d8df 100644
--- a/content/browser/site_instance_unittest.cc
+++ b/content/browser/site_instance_unittest.cc
@@ -119,25 +119,29 @@ class TestBrowsingInstance : public BrowsingInstance {
   TestBrowsingInstance(content::BrowserContext* browser_context,
                        int* deleteCounter)
       : BrowsingInstance(browser_context),
-        use_process_per_site(false),
-        deleteCounter_(deleteCounter) {
+        use_process_per_site_(false),
+        delete_counter_(deleteCounter) {
   }
 
   // Overrides BrowsingInstance::ShouldUseProcessPerSite so that we can test
   // both alternatives without using command-line switches.
   bool ShouldUseProcessPerSite(const GURL& url) {
-    return use_process_per_site;
+    return use_process_per_site_;
   }
 
-  // Set by individual tests.
-  bool use_process_per_site;
+  void SetProcessPerSite(bool use_process_per_site) {
+    use_process_per_site_ = use_process_per_site;
+  }
 
  private:
-  ~TestBrowsingInstance() {
-    (*deleteCounter_)++;
+  virtual ~TestBrowsingInstance() {
+    (*delete_counter_)++;
   }
 
-  int* deleteCounter_;
+  // Set by individual tests.
+  bool use_process_per_site_;
+
+  int* delete_counter_;
 };
 
 class TestSiteInstance : public SiteInstance {
@@ -151,14 +155,24 @@ class TestSiteInstance : public SiteInstance {
     return new TestSiteInstance(browsing_instance, siteDeleteCounter);
   }
 
+  bool HasProcess() const OVERRIDE {

[Charlie Reis, 2012/01/13 18:37:46]

We can't override this any more if it's not virtual, right?

[nasko, 2012/01/13 20:54:49]

Done.

+    return has_process_;
+  }
+
+  void SetHasProcess(bool process) {
+    has_process_ = process;
+  }
+
+
  private:
   TestSiteInstance(BrowsingInstance* browsing_instance, int* deleteCounter)
-    : SiteInstance(browsing_instance), deleteCounter_(deleteCounter) {}
-  ~TestSiteInstance() {
-    (*deleteCounter_)++;
+    : SiteInstance(browsing_instance), delete_counter_(deleteCounter) {}
+  virtual ~TestSiteInstance() {
+    (*delete_counter_)++;
   }
 
-  int* deleteCounter_;
+  int* delete_counter_;
+  bool has_process_;
 };
 
 }  // namespace
@@ -366,7 +380,7 @@ TEST_F(SiteInstanceTest, OneSiteInstancePerSite) {
   int deleteCounter = 0;
   TestBrowsingInstance* browsing_instance =
       new TestBrowsingInstance(NULL, &deleteCounter);
-  browsing_instance->use_process_per_site = false;
+  browsing_instance->SetProcessPerSite(false);
 
   const GURL url_a1("http://www.google.com/1.html");
   scoped_refptr<SiteInstance> site_instance_a1(
@@ -395,7 +409,7 @@ TEST_F(SiteInstanceTest, OneSiteInstancePerSite) {
   // browser context) should return a different SiteInstance.
   TestBrowsingInstance* browsing_instance2 =
       new TestBrowsingInstance(NULL, &deleteCounter);
-  browsing_instance2->use_process_per_site = false;
+  browsing_instance2->SetProcessPerSite(false);
   // Ensure the new SiteInstance is ref counted so that it gets deleted.
   scoped_refptr<SiteInstance> site_instance_a2_2(
       browsing_instance2->GetSiteInstanceForURL(url_a2));
@@ -424,7 +438,7 @@ TEST_F(SiteInstanceTest, OneSiteInstancePerSiteInBrowserContext) {
   int deleteCounter = 0;
   TestBrowsingInstance* browsing_instance =
       new TestBrowsingInstance(NULL, &deleteCounter);
-  browsing_instance->use_process_per_site = true;
+  browsing_instance->SetProcessPerSite(true);
 
   const GURL url_a1("http://www.google.com/1.html");
   scoped_refptr<SiteInstance> site_instance_a1(
@@ -456,7 +470,7 @@ TEST_F(SiteInstanceTest, OneSiteInstancePerSiteInBrowserContext) {
   // to make sure it gets deleted.
   scoped_refptr<TestBrowsingInstance> browsing_instance2(
       new TestBrowsingInstance(NULL, &deleteCounter));
-  browsing_instance2->use_process_per_site = true;
+  browsing_instance2->SetProcessPerSite(true);
   EXPECT_EQ(site_instance_a1.get(),
             browsing_instance2->GetSiteInstanceForURL(url_a2));
 
@@ -465,7 +479,7 @@ TEST_F(SiteInstanceTest, OneSiteInstancePerSiteInBrowserContext) {
   scoped_ptr<TestBrowserContext> browser_context(new TestBrowserContext());
   TestBrowsingInstance* browsing_instance3 =
       new TestBrowsingInstance(browser_context.get(), &deleteCounter);
-  browsing_instance3->use_process_per_site = true;
+  browsing_instance3->SetProcessPerSite(true);
   // Ensure the new SiteInstance is ref counted so that it gets deleted.
   scoped_refptr<SiteInstance> site_instance_a2_3(
       browsing_instance3->GetSiteInstanceForURL(url_a2));
@@ -546,3 +560,31 @@ TEST_F(SiteInstanceTest, ProcessSharingByType) {
 
   STLDeleteContainerPointers(hosts.begin(), hosts.end());
 }
+
+// Test to ensure that javascript URLs always run in the same process.

[Charlie Reis, 2012/01/13 18:37:46]

Please update this comment as well.

[nasko, 2012/01/13 20:54:49]

Done.

+TEST_F(SiteInstanceTest, HasWrongProcessForURL) {
+
+  scoped_ptr<TestBrowserContext> browser_context(new TestBrowserContext());
+  scoped_ptr<content::RenderProcessHost> host;
+  scoped_refptr<SiteInstance> instance(
+      SiteInstance::CreateSiteInstance(browser_context.get()));
+
+  EXPECT_FALSE(instance->has_site());
+  EXPECT_TRUE(instance->site().is_empty());
+
+  instance->SetSite(GURL("http://evernote.com/"));
+  EXPECT_TRUE(instance->has_site());
+
+  // Check prior to "assigning" a process to the instance, which is expected
+  // to return false due to not being attached to any process yet.
+  EXPECT_FALSE(instance->HasWrongProcessForURL(GURL("http://google.com")));
+
+  host.reset(instance->GetProcess());

[Charlie Reis, 2012/01/13 18:37:46]

Wow, I'm impressed if this works.  Do you know if it actually spins up a whole
renderer process, or just creates a test RenderProcessHost?

[nasko, 2012/01/13 20:54:49]

Yes, it indeed spawns a new process.

+  EXPECT_TRUE(host.get() != NULL);
+  EXPECT_TRUE(instance->HasProcess());
+
+  EXPECT_FALSE(instance->HasWrongProcessForURL(GURL("http://evernote.com")));
+  EXPECT_FALSE(instance->HasWrongProcessForURL(
+      GURL("javascript:alert(document.location.href);")));
+

[Charlie Reis, 2012/01/13 18:37:46]

Extra line can be deleted.

[nasko, 2012/01/13 20:54:49]

Done.

+}