Override IsRedirectResponse in extension protocols

Override IsRedirectResponse in extension protocols

The default URLFileRequestJob::IsRedirectResponse implementation is incorrect
for URLRequestExtensionJob (chrome-extension:-scheme) and ExtensionResourcesJob (chrome-extension-resource:-scheme).

BUG=388852, 424961
Committed: https://crrev.com/54a4e4044b90ab88d59af098586cd82bc1cfac8e
Cr-Commit-Position: refs/heads/master@{#300329}

[robwu, 2014-10-19 19:35:37]

rob@robwu.nl changed reviewers:
+ abarth@chromium.org, kalman@chromium.org, miket@chromium.org

[robwu, 2014-10-19 19:35:38]

kalman: Please perform the OWNER review.

abarth, miket: FYI, because you initially implemented these protocol handlers.

[miket_OOO, 2014-10-20 17:21:55]

On 2014/10/19 19:35:38, robwu wrote:
> kalman: Please perform the OWNER review.
> 
> abarth, miket: FYI, because you initially implemented these protocol handlers.

Thanks, Rob. Patch looks OK but I'll wait for Kalman's approval.

[not at google - send to devlin, 2014-10-20 18:20:50]

I think this will lose the directory redirection to including a '/':

https://code.google.com/p/chromium/codesearch#chromium/src/net/url_request/ur...

I'm not sure this is the right fix? The symptoms of those bugs sound more like
URLRequestFileJob getting confused / not expecting to be overriden.

[robwu, 2014-10-20 18:27:12]

On 2014/10/20 18:20:50, kalman wrote:
> I think this will lose the directory redirection to including a '/':
> 

Directory browsing is not supported for extensions, so this is not an issue.

>
https://code.google.com/p/chromium/codesearch#chromium/src/net/url_request/ur...
> 
> I'm not sure this is the right fix? The symptoms of those bugs sound more like
> URLRequestFileJob getting confused / not expecting to be overriden.

These bugs exist because URLFileRequest shouldn't be used in this way.

[not at google - send to devlin, 2014-10-20 19:07:54]

On 2014/10/20 18:27:12, robwu wrote:
> On 2014/10/20 18:20:50, kalman wrote:
> > I think this will lose the directory redirection to including a '/':
> > 
> 
> Directory browsing is not supported for extensions, so this is not an issue.

So long as the directory redirection code is only necessary for aesthetic
purposes - cool.

> 
> >
>
https://code.google.com/p/chromium/codesearch#chromium/src/net/url_request/ur...
> > 
> > I'm not sure this is the right fix? The symptoms of those bugs sound more
like
> > URLRequestFileJob getting confused / not expecting to be overriden.
> 
> These bugs exist because URLFileRequest shouldn't be used in this way.

Do you mean that extending URLFileRequest in this way is a recipe for issues
like this one? In which case - I agree.

[not at google - send to devlin, 2014-10-20 19:08:59]

Alright, lgtm. Perhaps you should mention:

URLFileRequest::IsRedirectResponse is incorrect for the chrome-extension://
scheme.

for these.

[robwu, 2014-10-20 19:15:42]

The CQ bit was checked by rob@robwu.nl

[commit-bot: I haz the power, 2014-10-20 19:18:05]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/650453003/1

[commit-bot: I haz the power, 2014-10-20 20:53:00]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2014-10-20 20:53:45]

Patchset 1 (id:??) landed as
https://crrev.com/54a4e4044b90ab88d59af098586cd82bc1cfac8e
Cr-Commit-Position: refs/heads/master@{#300329}