Do not refer "chrome" in content/browser/fileapi

Do not refer "chrome" in content/browser/fileapi

- Introduce ContentBrowserClient::GetAdditionalAllowedSchemesForFileSystem()
  to return allowed schemes for FileSystem API (which includes "chrome"
  and "chrome-extension" if it's for Chrome)

BUG=173574
TEST=green bots

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

[kinuko, 2013-04-04 10:04:56]

PTL

[jam, 2013-04-04 17:03:42]

https://codereview.chromium.org/13415006/diff/12002/chrome/browser/chrome_con...
File chrome/browser/chrome_content_browser_client.cc (right):

https://codereview.chromium.org/13415006/diff/12002/chrome/browser/chrome_con...
chrome/browser/chrome_content_browser_client.cc:2069:
additional_allowed_schemes.push_back(kChromeUIScheme);
content knows about kChromeUIScheme. do you want to do that there?

https://codereview.chromium.org/13415006/diff/12002/chrome/common/url_constan...
File chrome/common/url_constants.cc (right):

https://codereview.chromium.org/13415006/diff/12002/chrome/common/url_constan...
chrome/common/url_constants.cc:511: const char kExtensionScheme[] =
"chrome-extension";
this is in extensions/common/constants.h

[kinuko, 2013-04-05 10:16:17]

Thx, updated.

https://codereview.chromium.org/13415006/diff/12002/chrome/browser/chrome_con...
File chrome/browser/chrome_content_browser_client.cc (right):

https://codereview.chromium.org/13415006/diff/12002/chrome/browser/chrome_con...
chrome/browser/chrome_content_browser_client.cc:2069:
additional_allowed_schemes.push_back(kChromeUIScheme);
On 2013/04/04 17:03:42, jam wrote:
> content knows about kChromeUIScheme. do you want to do that there?

I thought about this, but this looks only necessary when it's called in chrome.

https://codereview.chromium.org/13415006/diff/12002/chrome/common/url_constan...
File chrome/common/url_constants.cc (right):

https://codereview.chromium.org/13415006/diff/12002/chrome/common/url_constan...
chrome/common/url_constants.cc:511: const char kExtensionScheme[] =
"chrome-extension";
On 2013/04/04 17:03:42, jam wrote:
> this is in extensions/common/constants.h

Done.

[jam, 2013-04-05 19:03:52]

https://codereview.chromium.org/13415006/diff/24001/content/public/browser/co...
File content/public/browser/content_browser_client.cc (right):

https://codereview.chromium.org/13415006/diff/24001/content/public/browser/co...
content/public/browser/content_browser_client.cc:274:
ContentBrowserClient::GetAdditionalAllowedSchemesForFileSystem() {
nit: by convention, this method should have a void return type, and should take
the vector as a parameter.

also, this logic below shouldn't be in this file, but should be in
CreateBrowserFileSystemOptions

[kinuko, 2013-04-10 05:01:51]

Thx, updated.

https://codereview.chromium.org/13415006/diff/24001/content/public/browser/co...
File content/public/browser/content_browser_client.cc (right):

https://codereview.chromium.org/13415006/diff/24001/content/public/browser/co...
content/public/browser/content_browser_client.cc:274:
ContentBrowserClient::GetAdditionalAllowedSchemesForFileSystem() {
On 2013/04/05 19:03:52, jam wrote:
> nit: by convention, this method should have a void return type, and should
take
> the vector as a parameter.
> 
> also, this logic below shouldn't be in this file, but should be in
> CreateBrowserFileSystemOptions

Done.

Reg: method signature, GetAdditionalWebUISchemes() also returns
std::vector<std::string>. Shall I make a follow-up patch to make the method
return void and take the vector as a parameter?

[jam, 2013-04-12 16:50:01]

lgtm with nits

https://codereview.chromium.org/13415006/diff/24001/content/public/browser/co...
File content/public/browser/content_browser_client.cc (right):

https://codereview.chromium.org/13415006/diff/24001/content/public/browser/co...
content/public/browser/content_browser_client.cc:274:
ContentBrowserClient::GetAdditionalAllowedSchemesForFileSystem() {
On 2013/04/10 05:01:51, kinuko wrote:
> On 2013/04/05 19:03:52, jam wrote:
> > nit: by convention, this method should have a void return type, and should
> take
> > the vector as a parameter.
> > 
> > also, this logic below shouldn't be in this file, but should be in
> > CreateBrowserFileSystemOptions
> 
> Done.
> 
> Reg: method signature, GetAdditionalWebUISchemes() also returns
> std::vector<std::string>. Shall I make a follow-up patch to make the method
> return void and take the vector as a parameter?

sure

https://codereview.chromium.org/13415006/diff/37001/chrome/browser/chrome_con...
File chrome/browser/chrome_content_browser_client.cc (right):

https://codereview.chromium.org/13415006/diff/37001/chrome/browser/chrome_con...
chrome/browser/chrome_content_browser_client.cc:2105:
DCHECK(additional_allowed_schemes);
nit: remove this. if null is passed in, the next line will crash and it's just
as good of a signal

https://codereview.chromium.org/13415006/diff/37001/content/public/browser/co...
File content/public/browser/content_browser_client.cc (right):

https://codereview.chromium.org/13415006/diff/37001/content/public/browser/co...
content/public/browser/content_browser_client.cc:271: std::vector<std::string>*
additional_schemes) {
since this method has no return type, you can inline the default implementation
in the header

[kinuko, 2013-04-14 05:11:53]

Thanks, addressed comments.

https://codereview.chromium.org/13415006/diff/37001/chrome/browser/chrome_con...
File chrome/browser/chrome_content_browser_client.cc (right):

https://codereview.chromium.org/13415006/diff/37001/chrome/browser/chrome_con...
chrome/browser/chrome_content_browser_client.cc:2105:
DCHECK(additional_allowed_schemes);
On 2013/04/12 16:50:01, jam wrote:
> nit: remove this. if null is passed in, the next line will crash and it's just
> as good of a signal

Done.

https://codereview.chromium.org/13415006/diff/37001/content/public/browser/co...
File content/public/browser/content_browser_client.cc (right):

https://codereview.chromium.org/13415006/diff/37001/content/public/browser/co...
content/public/browser/content_browser_client.cc:271: std::vector<std::string>*
additional_schemes) {
On 2013/04/12 16:50:01, jam wrote:
> since this method has no return type, you can inline the default
implementation
> in the header

Done.

[commit-bot: I haz the power, 2013-04-14 05:12:31]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/kinuko@chromium.org/13415006/58001

[commit-bot: I haz the power, 2013-04-14 13:14:17]

Change committed as 194123