Remove "open in new tab" items from context menu if the process doesn't

have permission to open them directly.  For example, right-click on a
"chrome://" link in an ordinary window should show two items: Copy link
location and inspect element, but a full menu from a WebUI window itself.

NOTE: Fixing this issue requires a fix to ChildProcessSecurityPolicy, which as been silently too permissive.

BUG=104466
Review URL: http://codereview.chromium.org/8588039

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@113818 0039d316-1c4b-4281-b951-d872f2087c98

4 files changed, 62 insertions, 22 deletions

diff --git a/content/browser/child_process_security_policy.cc b/content/browser/child_process_security_policy.cc
index cd61fd4..eeeac3f 100644
--- a/content/browser/child_process_security_policy.cc
+++ b/content/browser/child_process_security_policy.cc
@@ -10,6 +10,7 @@
 #include "base/platform_file.h"
 #include "base/stl_util.h"
 #include "base/string_util.h"
+#include "content/public/browser/content_browser_client.h"
 #include "content/browser/site_instance.h"
 #include "content/public/common/bindings_policy.h"
 #include "content/public/common/url_constants.h"
@@ -382,8 +383,10 @@ bool ChildProcessSecurityPolicy::CanRequestURL(
     return false;
   }
 
-  if (!net::URLRequest::IsHandledURL(url))
+  if (!content::GetContentClient()->browser()->IsHandledURL(url) &&
+      !net::URLRequest::IsHandledURL(url)) {
     return true;  // This URL request is destined for ShellExecute.
+  }
 
   {
     base::AutoLock lock(lock_);
diff --git a/content/browser/child_process_security_policy_unittest.cc b/content/browser/child_process_security_policy_unittest.cc
index 592b582..032978f 100644
--- a/content/browser/child_process_security_policy_unittest.cc
+++ b/content/browser/child_process_security_policy_unittest.cc
@@ -2,40 +2,75 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include <set>
 #include <string>
 
 #include "base/basictypes.h"
 #include "base/file_path.h"
 #include "base/platform_file.h"
 #include "content/browser/child_process_security_policy.h"
+#include "content/browser/mock_content_browser_client.h"
 #include "content/common/test_url_constants.h"
 #include "content/public/common/url_constants.h"
-#include "net/url_request/url_request.h"
-#include "net/url_request/url_request_test_job.h"
+#include "googleurl/src/gurl.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
+namespace {
+
+const int kRendererID = 42;
+const int kWorkerRendererID = kRendererID + 1;
+
+class ChildProcessSecurityPolicyTestBrowserClient
+    : public content::MockContentBrowserClient {
+ public:
+  ChildProcessSecurityPolicyTestBrowserClient() {}
+
+  virtual bool IsHandledURL(const GURL& url) {
+    return schemes_.find(url.scheme()) != schemes_.end();
+  }
+
+  void ClearSchemes() {
+    schemes_.clear();
+  }
+
+  void AddScheme(const std::string& scheme) {
+    schemes_.insert(scheme);
+  }
+
+ private:
+  std::set<std::string> schemes_;
+};
+
+}  // namespace
+
 class ChildProcessSecurityPolicyTest : public testing::Test {
- protected:
-  // testing::Test
+ public:
+  ChildProcessSecurityPolicyTest() : old_browser_client_(NULL) {
+  }
+
   virtual void SetUp() {
-    // In the real world, "chrome:" is a handled scheme.
-    RegisterProtocolFactory(chrome::kChromeUIScheme,
-                            &net::URLRequestTestJob::Factory);
+    old_browser_client_ = content::GetContentClient()->browser();
+    content::GetContentClient()->set_browser(&test_browser_client_);
+
+    // Claim to always handle chrome:// URLs because the CPSP's notion of
+    // allowing WebUI bindings is hard-wired to this particular scheme.
+    test_browser_client_.AddScheme("chrome");
   }
+
   virtual void TearDown() {
-    RegisterProtocolFactory(chrome::kChromeUIScheme, NULL);
+    test_browser_client_.ClearSchemes();
+    content::GetContentClient()->set_browser(old_browser_client_);
   }
 
-  static net::URLRequest::ProtocolFactory* RegisterProtocolFactory(
-      const std::string& scheme,
-      net::URLRequest::ProtocolFactory* factory) {
-    return net::URLRequest::Deprecated::RegisterProtocolFactory(
-        scheme, factory);
+ protected:
+  void RegisterTestScheme(const std::string& scheme) {
+    test_browser_client_.AddScheme(scheme);
   }
-};
 
-static int kRendererID = 42;
-static int kWorkerRendererID = kRendererID + 1;
+ private:
+  ChildProcessSecurityPolicyTestBrowserClient test_browser_client_;
+  content::ContentBrowserClient* old_browser_client_;
+};
 
 TEST_F(ChildProcessSecurityPolicyTest, IsWebSafeSchemeTest) {
   ChildProcessSecurityPolicy* p = ChildProcessSecurityPolicy::GetInstance();
@@ -174,8 +209,8 @@ TEST_F(ChildProcessSecurityPolicyTest, RegisterWebSafeSchemeTest) {
   // Currently, "asdf" is destined for ShellExecute, so it is allowed.
   EXPECT_TRUE(p->CanRequestURL(kRendererID, GURL("asdf:rockers")));
 
-  // Once we register a ProtocolFactory for "asdf", we default to deny.
-  RegisterProtocolFactory("asdf", &net::URLRequestTestJob::Factory);
+  // Once we register "asdf", we default to deny.
+  RegisterTestScheme("asdf");
   EXPECT_FALSE(p->CanRequestURL(kRendererID, GURL("asdf:rockers")));
 
   // We can allow new schemes by adding them to the whitelist.
@@ -183,9 +218,6 @@ TEST_F(ChildProcessSecurityPolicyTest, RegisterWebSafeSchemeTest) {
   EXPECT_TRUE(p->CanRequestURL(kRendererID, GURL("asdf:rockers")));
 
   // Cleanup.
-  RegisterProtocolFactory("asdf", NULL);
-  EXPECT_TRUE(p->CanRequestURL(kRendererID, GURL("asdf:rockers")));
-
   p->Remove(kRendererID);
 }
 
diff --git a/content/browser/mock_content_browser_client.cc b/content/browser/mock_content_browser_client.cc
index 507eac5..13de94f 100644
--- a/content/browser/mock_content_browser_client.cc
+++ b/content/browser/mock_content_browser_client.cc
@@ -89,6 +89,10 @@ bool MockContentBrowserClient::IsURLSameAsAnySiteInstance(const GURL& url) {
   return false;
 }
 
+bool MockContentBrowserClient::IsHandledURL(const GURL& url) {
+  return false;
+}
+
 bool MockContentBrowserClient::IsSuitableHost(
     RenderProcessHost* process_host,
     const GURL& site_url) {
diff --git a/content/browser/mock_content_browser_client.h b/content/browser/mock_content_browser_client.h
index fc6f5e77..37e866e 100644
--- a/content/browser/mock_content_browser_client.h
+++ b/content/browser/mock_content_browser_client.h
@@ -38,6 +38,7 @@ class MockContentBrowserClient : public ContentBrowserClient {
   virtual bool ShouldUseProcessPerSite(BrowserContext* browser_context,
                                        const GURL& effective_url) OVERRIDE;
   virtual bool IsURLSameAsAnySiteInstance(const GURL& url) OVERRIDE;
+  virtual bool IsHandledURL(const GURL& url) OVERRIDE;
   virtual bool IsSuitableHost(RenderProcessHost* process_host,
                               const GURL& site_url) OVERRIDE;
   virtual void SiteInstanceGotProcess(SiteInstance* site_instance) OVERRIDE;