Rename ProtocolInterceptJobFactory and make it not use ProtocolHandlers.

Rename ProtocolInterceptJobFactory and make it not use ProtocolHandlers.

ProtocolHandlers are intended to handle all requests for a particular
scheme, while UrlRequestInterceptors are intended to sit in front of
ProtocolHandlers, and optionally take over handling of any request,
regardless of scheme.

Separating the classes removes some ambiguity and weirdness,
particularly when passing a bunch of interceptors all over the place
from content/ to chrome/ during initialization.

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

[michaeln, 2014-05-15 19:20:18]

hooray for the distinction between the two :)

[mmenke, 2014-05-21 19:10:11]

PTAL.  android_protocol_handler.h should probably be renamed, but I think this
CL is already more than big and ugly enough as-is.

If you have suggestions for better class names for the new classes, now is the
time to speak up - much easier to just modify the diff than to rename them again
in the future.

[mmenke, 2014-05-27 15:39:39]

pauljensen:  Ping!

[pauljensen, 2014-05-27 18:53:50]

I'll take a look soon, probably tomorrow morning.  I didn't see the prior
emails, they got filtered into my chrome-apps sub-folder which I rarely read.  I
adjusted my filter to only filter for "chrome-apps@google.com" instead of
"chrome-apps", which is what gmail automatically created :(

[mmenke, 2014-05-27 18:54:57]

On 2014/05/27 18:53:50, pauljensen wrote:
> I'll take a look soon, probably tomorrow morning.  I didn't see the prior
> emails, they got filtered into my chrome-apps sub-folder which I rarely read. 
I
> adjusted my filter to only filter for mailto:"chrome-apps@google.com" instead
of
> "chrome-apps", which is what gmail automatically created :(

Great, thanks!

[pauljensen, 2014-05-28 12:03:49]

lgtm.  I'm not a huge fan of the name URLRequestInterceptor as it's quite
similar to URLRequest::Interceptor (of which I'm not a fan after spending a long
time removing much of them), but I don't know of a better name, and this gives
us more reason to remove the last remnants of URLRequest::Interceptors.

https://codereview.chromium.org/282103004/diff/320001/net/url_request/url_req...
File net/url_request/url_request_interceptor.h (right):

https://codereview.chromium.org/282103004/diff/320001/net/url_request/url_req...
net/url_request/url_request_interceptor.h:19: // requests's scheme.
Insert "the" before requests's and change requests's to "requests'"

https://codereview.chromium.org/282103004/diff/320001/net/url_request/url_req...
net/url_request/url_request_interceptor.h:31:
DISALLOW_COPY_AND_ASSIGN(URLRequestInterceptor);
Insert private: before this

[mmenke, 2014-05-28 15:08:14]

Thanks for catching those!

https://codereview.chromium.org/282103004/diff/320001/net/url_request/url_req...
File net/url_request/url_request_interceptor.h (right):

https://codereview.chromium.org/282103004/diff/320001/net/url_request/url_req...
net/url_request/url_request_interceptor.h:19: // requests's scheme.
On 2014/05/28 12:03:50, pauljensen wrote:
> Insert "the" before requests's and change requests's to "requests'"

Done.

https://codereview.chromium.org/282103004/diff/320001/net/url_request/url_req...
net/url_request/url_request_interceptor.h:31:
DISALLOW_COPY_AND_ASSIGN(URLRequestInterceptor);
On 2014/05/28 12:03:50, pauljensen wrote:
> Insert private: before this

Done.

[mmenke, 2014-05-28 15:48:59]

Adding owners reviewers.  In most files this just substitutes some uses of
ProtocolHandler with the new class UrlRequestInterceptor.

mkosiba:  Please review android_webview/

jamescook:  Please review apps/ and ash/

sky:  Please review chrome/, except chrome/profiles/ and chrome/net/ (Both of
which I'm an owner of).

brettw:  Please review content/

Thanks!

[sky, 2014-05-28 16:06:17]

LGTM

[mkosiba (inactive), 2014-05-28 16:25:53]

android_webview LGTM

https://codereview.chromium.org/282103004/diff/360001/android_webview/browser...
File android_webview/browser/net/aw_url_request_context_getter.cc (right):

https://codereview.chromium.org/282103004/diff/360001/android_webview/browser...
android_webview/browser/net/aw_url_request_context_getter.cc:135: // in the
order in which they appear in the protocol_handlers vector.
the request_interceptors vector?

[mmenke, 2014-05-28 16:29:39]

Thanks!

https://codereview.chromium.org/282103004/diff/360001/android_webview/browser...
File android_webview/browser/net/aw_url_request_context_getter.cc (right):

https://codereview.chromium.org/282103004/diff/360001/android_webview/browser...
android_webview/browser/net/aw_url_request_context_getter.cc:135: // in the
order in which they appear in the protocol_handlers vector.
On 2014/05/28 16:25:54, mkosiba wrote:
> the request_interceptors vector?

Done.

[James Cook, 2014-05-28 18:28:53]

apps/ and ash/ LGTM

[mmenke, 2014-05-29 21:44:51]

jam:  Mind reviewing content changes?  This is basically just replacing one use
of ProtocolHandlers with URLRequestInterceptors, as they make their way from
content/ to chrome/.

[jam, 2014-05-30 01:19:14]

content lgtm with nits

https://codereview.chromium.org/282103004/diff/370001/content/public/browser/...
File content/public/browser/content_browser_client.h (right):

https://codereview.chromium.org/282103004/diff/370001/content/public/browser/...
content/public/browser/content_browser_client.h:116: // A scoped vector of
protocol handlers.
nit: update comment

https://codereview.chromium.org/282103004/diff/370001/content/shell/browser/s...
File content/shell/browser/shell_content_browser_client.h (right):

https://codereview.chromium.org/282103004/diff/370001/content/shell/browser/s...
content/shell/browser/shell_content_browser_client.h:39:
content::URLRequestInterceptorScopedVector request_interceptors) OVERRIDE;
ok i won't mention this in another comment to save me the typing, so just remove
all "content::" inside the content files

https://codereview.chromium.org/282103004/diff/370001/content/shell/browser/s...
File content/shell/browser/shell_url_request_context_getter.cc (right):

https://codereview.chromium.org/282103004/diff/370001/content/shell/browser/s...
content/shell/browser/shell_url_request_context_getter.cc:67:
content::URLRequestInterceptorScopedVector request_interceptors,
ditto

https://codereview.chromium.org/282103004/diff/370001/content/shell/browser/s...
File content/shell/browser/shell_url_request_context_getter.h (right):

https://codereview.chromium.org/282103004/diff/370001/content/shell/browser/s...
content/shell/browser/shell_url_request_context_getter.h:39:
content::URLRequestInterceptorScopedVector request_interceptors,
nit: here and below get rid of "content::"

[mmenke, 2014-05-30 14:40:21]

Thanks for the feeedback!

https://codereview.chromium.org/282103004/diff/370001/content/public/browser/...
File content/public/browser/content_browser_client.h (right):

https://codereview.chromium.org/282103004/diff/370001/content/public/browser/...
content/public/browser/content_browser_client.h:116: // A scoped vector of
protocol handlers.
On 2014/05/30 01:19:15, jam wrote:
> nit: update comment

Done.

https://codereview.chromium.org/282103004/diff/370001/content/shell/browser/s...
File content/shell/browser/shell_content_browser_client.h (right):

https://codereview.chromium.org/282103004/diff/370001/content/shell/browser/s...
content/shell/browser/shell_content_browser_client.h:39:
content::URLRequestInterceptorScopedVector request_interceptors) OVERRIDE;
On 2014/05/30 01:19:15, jam wrote:
> ok i won't mention this in another comment to save me the typing, so just
remove
> all "content::" inside the content files

Too much copy-paste.  Done.  I've gone over all that files I modified in
content, and removed any "content::" I added, and removed some pre-existing ones
as well.

https://codereview.chromium.org/282103004/diff/370001/content/shell/browser/s...
File content/shell/browser/shell_url_request_context_getter.cc (right):

https://codereview.chromium.org/282103004/diff/370001/content/shell/browser/s...
content/shell/browser/shell_url_request_context_getter.cc:67:
content::URLRequestInterceptorScopedVector request_interceptors,
On 2014/05/30 01:19:15, jam wrote:
> ditto

Done.

https://codereview.chromium.org/282103004/diff/370001/content/shell/browser/s...
File content/shell/browser/shell_url_request_context_getter.h (right):

https://codereview.chromium.org/282103004/diff/370001/content/shell/browser/s...
content/shell/browser/shell_url_request_context_getter.h:39:
content::URLRequestInterceptorScopedVector request_interceptors,
On 2014/05/30 01:19:15, jam wrote:
> nit: here and below get rid of "content::"

Done.

[mmenke, 2014-05-30 15:02:41]

The CQ bit was checked by mmenke@chromium.org

[commit-bot: I haz the power, 2014-05-30 15:03:16]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/mmenke@chromium.org/282103004/380001

[commit-bot: I haz the power, 2014-05-30 17:56:20]

Change committed as 273874