Don't swap processes for javascript: URLs.

content/browser/site_instance_unittest.cc

 // Copyright (c) 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "base/compiler_specific.h"
 #include "base/stl_util.h"
 #include "base/string16.h"
 #include "content/browser/browser_thread_impl.h"
 #include "content/browser/browsing_instance.h"
 #include "content/browser/child_process_security_policy.h"
@@ skipping 133 matching lines @@
  public:
   static TestSiteInstance* CreateTestSiteInstance(
       content::BrowserContext* browser_context,
       int* siteDeleteCounter,
       int* browsingDeleteCounter) {
     TestBrowsingInstance* browsing_instance =
         new TestBrowsingInstance(browser_context, browsingDeleteCounter);
     return new TestSiteInstance(browsing_instance, siteDeleteCounter);
   }
 
+  bool HasProcess() const {

[Charlie Reis, 2012/01/12 22:01:14]

Please add OVERRIDE, so that we'll remember to update this if the function name
ever changes.

[nasko, 2012/01/12 22:47:47]

Done.

+    return hasProcess;
+  }
+
+  bool hasProcess;
+
  private:
   TestSiteInstance(BrowsingInstance* browsing_instance, int* deleteCounter)
     : SiteInstance(browsing_instance), deleteCounter_(deleteCounter) {}
   ~TestSiteInstance() {
     (*deleteCounter_)++;
   }
 
   int* deleteCounter_;
 };
 
@@ skipping 375 matching lines @@
   // Make sure none of differing privilege processes are mixed.
   EXPECT_NE(extension1_instance->GetProcess(), webui1_instance->GetProcess());
 
   for (size_t i = 0; i < content::kMaxRendererProcessCount; ++i) {
     EXPECT_NE(extension1_instance->GetProcess(), hosts[i]);
     EXPECT_NE(webui1_instance->GetProcess(), hosts[i]);
   }
 
   STLDeleteContainerPointers(hosts.begin(), hosts.end());
 }
+
+// Test to ensure that javascript URLs always run in the same process.
+TEST_F(SiteInstanceTest, SameProcessForJavascript) {

[Charlie Reis, 2012/01/12 22:01:14]

Maybe we can make this a more general test and call it HasWrongProcessForURL,
since we're not testing that anywhere else in here.

That would let us also test calling it before HasProcess() is true.

[nasko, 2012/01/12 22:47:47]

Added a test case before we've assigned a process to the site instance. The
actual check will be harder to test and considering Albert's changes will touch
this code path, I'd wait for adding more tests.

[awong, 2012/01/12 22:56:41]

Don't worry about me :)  If you commit first, then its my job is to fix the new
code!  (also, not sure when I'd actually commit)

[nasko, 2012/01/13 01:34:50]

Done.

+
+  int siteDeleteCounter = 0;
+  int browsingDeleteCounter = 0;
+
+  TestSiteInstance* instance = TestSiteInstance::CreateTestSiteInstance(
+                                  NULL, &siteDeleteCounter,
+                                  &browsingDeleteCounter);
+
+  GURL url_javascript = GURL("javascript:alert(document.location.href);");
+
+  EXPECT_FALSE(instance->has_site());
+  EXPECT_TRUE(instance->site().is_empty());
+
+  instance->SetSite(GURL("http://www.evernote.com/"));
+  EXPECT_TRUE(instance->has_site());
+
+  instance->hasProcess = true;
+  EXPECT_TRUE(instance->HasProcess());
+
+  EXPECT_FALSE(instance->HasWrongProcessForURL(url_javascript));
+}