Move the render process host tests in content to chrome. These test chrome specific behavior with t…

Move the render process host tests in content to chrome. These test chrome specific behavior with things like extensions/webui/singleton tabs.

BUG=90448
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=148852

[Charlie Reis, 2012-07-27 19:51:09]

Seems fine, apart from the removed security test.

http://codereview.chromium.org/10820056/diff/1/content/browser/child_process_...
File content/browser/child_process_security_policy_unittest.cc (left):

http://codereview.chromium.org/10820056/diff/1/content/browser/child_process_...
content/browser/child_process_security_policy_unittest.cc:183:
EXPECT_TRUE(p->CanRequestURL(kRendererID, GURL(content::kTestBookmarksURL)));
Shouldn't we check these in a chrome test instead of just removing them?

[jam, 2012-07-27 19:54:45]

http://codereview.chromium.org/10820056/diff/1/content/browser/child_process_...
File content/browser/child_process_security_policy_unittest.cc (left):

http://codereview.chromium.org/10820056/diff/1/content/browser/child_process_...
content/browser/child_process_security_policy_unittest.cc:183:
EXPECT_TRUE(p->CanRequestURL(kRendererID, GURL(content::kTestBookmarksURL)));
On 2012/07/27 19:51:09, creis wrote:
> Shouldn't we check these in a chrome test instead of just removing them?

I guess I didn't understand how this is different from the above check. is it
because in chrome, that's different?

are you saying I should create a new test in chrome, since that uses its own
Content*Client interfaces, and just move those 4 lines to it?

[Charlie Reis, 2012-07-27 20:03:19]

http://codereview.chromium.org/10820056/diff/1/content/browser/child_process_...
File content/browser/child_process_security_policy_unittest.cc (left):

http://codereview.chromium.org/10820056/diff/1/content/browser/child_process_...
content/browser/child_process_security_policy_unittest.cc:183:
EXPECT_TRUE(p->CanRequestURL(kRendererID, GURL(content::kTestBookmarksURL)));
On 2012/07/27 19:54:45, John Abd-El-Malek wrote:
> On 2012/07/27 19:51:09, creis wrote:
> > Shouldn't we check these in a chrome test instead of just removing them?
> 
> I guess I didn't understand how this is different from the above check. is it
> because in chrome, that's different?

To be honest, I'm not sure why there are separate tests for NTP, history, and
bookmarks, but it might be to ensure that the NTP isn't special cased (i.e.,
that the whole chrome:// origin is granted).

@msw: You appear to have added these checks in
http://codereview.chromium.org/7068007.  Can you say anything about why all
three are needed?

> are you saying I should create a new test in chrome, since that uses its own
> Content*Client interfaces, and just move those 4 lines to it?

Yes, that's what I was suggesting.  That way we get the chrome:// URLs out of a
content test, but still keep the test coverage (assuming this bit of coverage is
worthwhile).

[msw, 2012-07-27 20:59:04]

http://codereview.chromium.org/10820056/diff/1/content/browser/child_process_...
File content/browser/child_process_security_policy_unittest.cc (left):

http://codereview.chromium.org/10820056/diff/1/content/browser/child_process_...
content/browser/child_process_security_policy_unittest.cc:183:
EXPECT_TRUE(p->CanRequestURL(kRendererID, GURL(content::kTestBookmarksURL)));
I don't remember exactly why I added these checks. "chrome:" isn't registered as
a WebSafeScheme afaict, but "about:" is a PseudoScheme. I suspect the purpose is
to ensure that PseudoScheme "about:" URL grants are denied, while (unknown?)
"chrome:" URL grants are accepted. If that's the case, it seems fine to remove
redundant "chrome:" URL checks. John, you removed some redundant "about:" URL
checks in crrev.com/120891 but I would gladly defer this decision to someone
with a clearer understanding of this security policy code (perhaps "chrome:"
should actually be a psuedo scheme too?).

[Charlie Reis, 2012-07-27 21:35:42]

http://codereview.chromium.org/10820056/diff/1/content/browser/child_process_...
File content/browser/child_process_security_policy_unittest.cc (left):

http://codereview.chromium.org/10820056/diff/1/content/browser/child_process_...
content/browser/child_process_security_policy_unittest.cc:183:
EXPECT_TRUE(p->CanRequestURL(kRendererID, GURL(content::kTestBookmarksURL)));
On 2012/07/27 20:59:05, msw wrote:
> I don't remember exactly why I added these checks. "chrome:" isn't registered
as
> a WebSafeScheme afaict, but "about:" is a PseudoScheme. I suspect the purpose
is
> to ensure that PseudoScheme "about:" URL grants are denied, while (unknown?)
> "chrome:" URL grants are accepted. If that's the case, it seems fine to remove
> redundant "chrome:" URL checks. John, you removed some redundant "about:" URL
> checks in crrev.com/120891 but I would gladly defer this decision to someone
> with a clearer understanding of this security policy code (perhaps "chrome:"
> should actually be a psuedo scheme too?).

Ok.  LGTM, then, but it might be good to get Tom's eyes on it to be sure we're
not losing any important coverage for CanRequestURL.