Remove URLRequestJobFactory::IsHandledURL.

Remove URLRequestJobFactory::IsHandledURL.

In practice, this was just a combination of GURL::is_valid() and
URLRequestJobFactory::IsHandledProtocol(GURL::is_scheme()).

This is just motivated by a desire to simplify the knobs exposed by
the network stack, for servicification.  This does add a number of
GURL::is_valid() checks, but I suspect most of them will go away, as we
adapt the launch externally handled protocol path.

BUG=706942
Review-Url: https://codereview.chromium.org/2783343002
Cr-Commit-Position: refs/heads/master@{#461518}
Committed: https://chromium.googlesource.com/chromium/src/+/1918ae78a8154c4dfc31fd238f9a00c53bbd96fd

[mmenke, 2017-03-30 18:59:25]

The CQ bit was checked by mmenke@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-30 19:00:31]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-30 19:15:21]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-30 19:15:22]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[mmenke, 2017-03-30 19:29:35]

The CQ bit was checked by mmenke@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-30 19:30:28]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-30 19:45:12]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-30 19:45:15]

Dry run: Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[mmenke, 2017-03-30 20:48:51]

The CQ bit was checked by mmenke@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-30 20:49:40]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[mmenke, 2017-03-30 21:10:21]

The CQ bit was checked by mmenke@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-30 21:11:59]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-30 21:38:21]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-30 21:38:22]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[mmenke, 2017-03-30 21:41:34]

The CQ bit was checked by mmenke@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-30 21:42:43]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-30 22:02:17]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-30 22:02:18]

Dry run: Try jobs failed on following builders:
  linux_chromium_tsan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[mmenke, 2017-03-31 03:17:12]

The CQ bit was checked by mmenke@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-31 03:17:38]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[mmenke, 2017-03-31 03:51:39]

Description was changed from

==========
Remove URLRequestJobFactory::IsHandledURL.

In practice, this was just a combination of GURL::is_valid() and
URLRequestJobFactory::IsHandledProtocol(GURL::is_scheme()).

This is just motivated by a desire to simplify the knobs exposed by
the network stack, for servicification.  This does add a number of
is_valid() checks, but I suspect most of them will go away, as we
adapt the launch externally handled protocol path.

BUG=
==========

to

==========
Remove URLRequestJobFactory::IsHandledURL.

In practice, this was just a combination of GURL::is_valid() and
URLRequestJobFactory::IsHandledProtocol(GURL::is_scheme()).

This is just motivated by a desire to simplify the knobs exposed by
the network stack, for servicification.  This does add a number of
is_valid() checks, but I suspect most of them will go away, as we
adapt the launch externally handled protocol path.

BUG=706942
==========

[mmenke, 2017-03-31 03:51:53]

Description was changed from

==========
Remove URLRequestJobFactory::IsHandledURL.

In practice, this was just a combination of GURL::is_valid() and
URLRequestJobFactory::IsHandledProtocol(GURL::is_scheme()).

This is just motivated by a desire to simplify the knobs exposed by
the network stack, for servicification.  This does add a number of
is_valid() checks, but I suspect most of them will go away, as we
adapt the launch externally handled protocol path.

BUG=706942
==========

to

==========
Remove URLRequestJobFactory::IsHandledURL.

In practice, this was just a combination of GURL::is_valid() and
URLRequestJobFactory::IsHandledProtocol(GURL::is_scheme()).

This is just motivated by a desire to simplify the knobs exposed by
the network stack, for servicification.  This does add a number of
GURL::is_valid() checks, but I suspect most of them will go away, as we
adapt the launch externally handled protocol path.

BUG=706942
==========

[mmenke, 2017-03-31 03:52:40]

Patchset #2 (id:20001) has been deleted

[mmenke, 2017-03-31 03:52:56]

Patchset #3 (id:60001) has been deleted

[commit-bot: I haz the power, 2017-03-31 04:19:06]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-31 04:19:07]

Dry run: This issue passed the CQ dry run.

[mmenke, 2017-03-31 04:30:03]

mmenke@chromium.org changed reviewers:
+ asanka@chromium.org

[mmenke, 2017-03-31 04:30:04]

Silly little CL.  I had thought it was a pre-req to make moving launching URLs
externally over to the UI thread more sane, but turns out it's not needed.

Still, this is one less random knob for embedders to much with, at least. 
Suggest looking at net/ files first.

https://codereview.chromium.org/2783343002/diff/100001/content/browser/downlo...
File content/browser/download/download_manager_impl.cc (right):

https://codereview.chromium.org/2783343002/diff/100001/content/browser/downlo...
content/browser/download/download_manager_impl.cc:567: if
(!request_context->job_factory()->IsHandledProtocol(url.scheme())) {
Blocking invalid URLs here seems reasonable (The above check catches them
anyways)

https://codereview.chromium.org/2783343002/diff/100001/content/browser/downlo...
File content/browser/download/save_file_manager.cc (right):

https://codereview.chromium.org/2783343002/diff/100001/content/browser/downlo...
content/browser/download/save_file_manager.cc:302: if
(!request_context->job_factory()->IsHandledProtocol(url.scheme())) {
There's a DCHECK about URL validity before calling this method (Line 107), so
this seems fine.

https://codereview.chromium.org/2783343002/diff/100001/content/browser/loader...
File content/browser/loader/navigation_resource_throttle.cc (right):

https://codereview.chromium.org/2783343002/diff/100001/content/browser/loader...
content/browser/loader/navigation_resource_throttle.cc:211:
->IsHandledProtocol(request_->url().scheme());
This duplication seems ugly, but with PlzNavigate, should go away.  The one in
the NavigationResourceHandler will stay until we move that to mojo.

https://codereview.chromium.org/2783343002/diff/100001/content/browser/loader...
File content/browser/loader/resource_dispatcher_host_impl.cc (right):

https://codereview.chromium.org/2783343002/diff/100001/content/browser/loader...
content/browser/loader/resource_dispatcher_host_impl.cc:581: if (!url.is_valid()
|| job_factory->IsHandledProtocol(url.scheme()))
I hope to remove the calls when moving launching processes over to the UI
thread.

[asanka, 2017-03-31 20:38:30]

lgtm

https://codereview.chromium.org/2783343002/diff/100001/content/browser/downlo...
File content/browser/download/download_manager_impl.cc (right):

https://codereview.chromium.org/2783343002/diff/100001/content/browser/downlo...
content/browser/download/download_manager_impl.cc:567: if
(!request_context->job_factory()->IsHandledProtocol(url.scheme())) {
On 2017/03/31 04:30:04, mmenke wrote:
> Blocking invalid URLs here seems reasonable (The above check catches them
> anyways)

Acknowledged. It's also fine to let the request go through and fail later. We
need to handle such cases anyway.

https://codereview.chromium.org/2783343002/diff/100001/content/browser/downlo...
File content/browser/download/save_file_manager.cc (right):

https://codereview.chromium.org/2783343002/diff/100001/content/browser/downlo...
content/browser/download/save_file_manager.cc:302: if
(!request_context->job_factory()->IsHandledProtocol(url.scheme())) {
On 2017/03/31 04:30:04, mmenke wrote:
> There's a DCHECK about URL validity before calling this method (Line 107), so
> this seems fine.

Acknowledged.

https://codereview.chromium.org/2783343002/diff/100001/content/browser/loader...
File content/browser/loader/navigation_resource_throttle.cc (right):

https://codereview.chromium.org/2783343002/diff/100001/content/browser/loader...
content/browser/loader/navigation_resource_throttle.cc:211:
->IsHandledProtocol(request_->url().scheme());
On 2017/03/31 04:30:04, mmenke wrote:
> This duplication seems ugly, but with PlzNavigate, should go away.  The one in
> the NavigationResourceHandler will stay until we move that to mojo.

Acknowledged.

https://codereview.chromium.org/2783343002/diff/100001/content/browser/loader...
File content/browser/loader/resource_dispatcher_host_impl.cc (right):

https://codereview.chromium.org/2783343002/diff/100001/content/browser/loader...
content/browser/loader/resource_dispatcher_host_impl.cc:581: if (!url.is_valid()
|| job_factory->IsHandledProtocol(url.scheme()))
On 2017/03/31 04:30:04, mmenke wrote:
> I hope to remove the calls when moving launching processes over to the UI
> thread.

Acknowledged.

[mmenke, 2017-03-31 20:59:06]

mmenke@chromium.org changed reviewers:
+ benwells@chromium.org, michaeln@chromium.org, mtomasz@chromium.org,
pasko@chromium.org, tedchoc@chromium.org, tobiasjs@chromium.org

[mmenke, 2017-03-31 20:59:06]

[+tobiasjs]:  Please review android_webview/browser/net/ (Removed IsHandledURL
in favor of the already-existent IsHandledProtocol, both of which always return
true).

[+benwells]:  Please review chrome/browser/custom_handlers

The rest of this I consider TBR-able (the above things probably are, too), as
the change just removes a single method from a couple test fixtures that aren't
using it anyways, but I'll give everyone until Tuesday morning to review, just
out of paranoia:

[+michaeln]:  content/browser/fileapi/ and content/browser/appcache/
[+mtomasz]: 
chrome/browser/chromeos/fileapi/external_file_url_request_job_unittest.cc
[+pasko]:  chrome/browser/predictors/resource_prefetch_predictor_unittest.cc
[+tedchoc]:  content/browser/android/url_request_content_job_unittest.cc

[Ted C, 2017-03-31 23:02:36]

On 2017/03/31 20:59:06, mmenke wrote:
> [+tobiasjs]:  Please review android_webview/browser/net/ (Removed IsHandledURL
> in favor of the already-existent IsHandledProtocol, both of which always
return
> true).
> 
> [+benwells]:  Please review chrome/browser/custom_handlers
> 
> The rest of this I consider TBR-able (the above things probably are, too), as
> the change just removes a single method from a couple test fixtures that
aren't
> using it anyways, but I'll give everyone until Tuesday morning to review, just
> out of paranoia:
> 
> [+michaeln]:  content/browser/fileapi/ and content/browser/appcache/
> [+mtomasz]: 
> chrome/browser/chromeos/fileapi/external_file_url_request_job_unittest.cc
> [+pasko]:  chrome/browser/predictors/resource_prefetch_predictor_unittest.cc
> [+tedchoc]:  content/browser/android/url_request_content_job_unittest.cc

content/browser/android/url_request_content_job_unittest.cc - lgtm

[benwells, 2017-04-03 00:28:10]

lgtm

[mtomasz, 2017-04-03 01:01:47]

lgtm

[pasko, 2017-04-03 07:51:04]

chrome/browser/predictors lgtm

[Tobias Sargeant, 2017-04-03 10:07:42]

On 2017/04/03 07:51:04, pasko wrote:
> chrome/browser/predictors lgtm

android_webview/ lgtm

[michaeln, 2017-04-03 18:30:48]

lgtm 2!

[mmenke, 2017-04-03 18:38:24]

On 2017/04/03 18:30:48, michaeln wrote:
> lgtm 2!

Thanks all!

[mmenke, 2017-04-03 18:38:33]

The CQ bit was checked by mmenke@chromium.org

[commit-bot: I haz the power, 2017-04-03 18:39:22]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-03 20:10:24]

CQ is committing da patch.

Bot data: {"patchset_id": 100001, "attempt_start_ts": 1491244713292720,
"parent_rev": "01cc50e197d277407a63f2fcf1641fe7b3ad2f61", "commit_rev":
"1918ae78a8154c4dfc31fd238f9a00c53bbd96fd"}

[commit-bot: I haz the power, 2017-04-03 20:11:18]

Description was changed from

==========
Remove URLRequestJobFactory::IsHandledURL.

In practice, this was just a combination of GURL::is_valid() and
URLRequestJobFactory::IsHandledProtocol(GURL::is_scheme()).

This is just motivated by a desire to simplify the knobs exposed by
the network stack, for servicification.  This does add a number of
GURL::is_valid() checks, but I suspect most of them will go away, as we
adapt the launch externally handled protocol path.

BUG=706942
==========

to

==========
Remove URLRequestJobFactory::IsHandledURL.

In practice, this was just a combination of GURL::is_valid() and
URLRequestJobFactory::IsHandledProtocol(GURL::is_scheme()).

This is just motivated by a desire to simplify the knobs exposed by
the network stack, for servicification.  This does add a number of
GURL::is_valid() checks, but I suspect most of them will go away, as we
adapt the launch externally handled protocol path.

BUG=706942

Review-Url: https://codereview.chromium.org/2783343002
Cr-Commit-Position: refs/heads/master@{#461518}
Committed:
https://chromium.googlesource.com/chromium/src/+/1918ae78a8154c4dfc31fd238f9a...
==========

[commit-bot: I haz the power, 2017-04-03 20:11:19]

Committed patchset #4 (id:100001) as
https://chromium.googlesource.com/chromium/src/+/1918ae78a8154c4dfc31fd238f9a...