New issue
Advanced search Search tips

Issue 706030 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Mar 2018
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug
Proj-Servicification

Blocked on:
issue 706942

Blocking:
issue 598073



Sign in to add a comment

Move RDHDelegate::HandleExternalProtocol over to navigation logic

Project Member Reported by mmenke@chromium.org, Mar 28 2017

Issue description

Seems like there are two options:  Add a method to ContentBrowserDelegate, or leave it up to embedders to write NavigationThrottles to do this.  I've been going back and forth between them - there's some weird logic about what URLs can be handled internally, and not sure we want to have it in content.  On Android, we also just use a Chrome-specific NavigationThrottle to do this, so there's precedent for that approach.

The only embedder in the repo that needs this logic is Chrome.  Since Chrome's logic integrates into a blacklist/whitelist with weird interactions, I'm planning on moving that over at the same time, and also probably remove URLRequestJobFactory::IsHandledURL, which feeds into this, in favor of IsHandledProtocol (Which has a corresponding method that can be called on the UI thread already).
 

Comment 1 by mmenke@chromium.org, Mar 30 2017

Blockedon: 706942
Owner: ----
Status: Available (was: Assigned)
Marking this as available for now - decided that due to the URL Blacklist being used in a completely different manner in Android Webview, and not wanting to add a way for NavigationThrottles to fail a request with a specific error code unless we decide we really need it, it's best to punt this for the moment.

Comment 3 by yzshen@chromium.org, May 24 2017

Components: Internals>Network>Service

Comment 4 by laforge@google.com, Nov 7 2017

Components: -Internals>Network>Service Internals>Services>Network
Apologies, applied the wrong component in bulk.
Project Member

Comment 5 by bugdroid1@chromium.org, Mar 6 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9350d9dc037bfed22f896f8b324e589f7df0f895

commit 9350d9dc037bfed22f896f8b324e589f7df0f895
Author: John Abd-El-Malek <jam@chromium.org>
Date: Tue Mar 06 23:49:23 2018

Run extensions_browsertests with network service along with the other network_service gtests.

There is one test failing which is because we don't handle external protocols with the network service.

Bug:  819248 , 706030 
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_mojo
Change-Id: I3ff98b61b2fa19fa3bfd54373cd920d4777f04c9
Reviewed-on: https://chromium-review.googlesource.com/951883
Commit-Queue: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Ken Rockot <rockot@chromium.org>
Cr-Commit-Position: refs/heads/master@{#541246}
[modify] https://crrev.com/9350d9dc037bfed22f896f8b324e589f7df0f895/extensions/BUILD.gn
[modify] https://crrev.com/9350d9dc037bfed22f896f8b324e589f7df0f895/extensions/shell/browser/shell_content_browser_client.cc
[modify] https://crrev.com/9350d9dc037bfed22f896f8b324e589f7df0f895/extensions/shell/browser/shell_content_browser_client.h
[modify] https://crrev.com/9350d9dc037bfed22f896f8b324e589f7df0f895/testing/buildbot/chromium.fyi.json
[modify] https://crrev.com/9350d9dc037bfed22f896f8b324e589f7df0f895/testing/buildbot/chromium.linux.json
[modify] https://crrev.com/9350d9dc037bfed22f896f8b324e589f7df0f895/testing/buildbot/filters/BUILD.gn
[modify] https://crrev.com/9350d9dc037bfed22f896f8b324e589f7df0f895/testing/buildbot/filters/mojo.fyi.network_browser_tests.filter
[add] https://crrev.com/9350d9dc037bfed22f896f8b324e589f7df0f895/testing/buildbot/filters/mojo.fyi.network_extensions_browsertests.filter
[modify] https://crrev.com/9350d9dc037bfed22f896f8b324e589f7df0f895/testing/buildbot/test_suites.pyl

Comment 6 by jam@chromium.org, Mar 8 2018

Owner: jam@chromium.org
Status: Started (was: Available)
Project Member

Comment 7 by bugdroid1@chromium.org, Mar 9 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a67add8097a5846e30c0bb8d5847a59dc6567c40

commit a67add8097a5846e30c0bb8d5847a59dc6567c40
Author: John Abd-El-Malek <jam@chromium.org>
Date: Fri Mar 09 18:22:01 2018

Handle external protocols with the network service.

Bug:  706030 
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_mojo
Change-Id: I82f817cae6477e2e2bcbb44f266ae0956e55182a
Reviewed-on: https://chromium-review.googlesource.com/956091
Reviewed-by: Jay Civelli <jcivelli@chromium.org>
Commit-Queue: John Abd-El-Malek <jam@chromium.org>
Cr-Commit-Position: refs/heads/master@{#542156}
[modify] https://crrev.com/a67add8097a5846e30c0bb8d5847a59dc6567c40/android_webview/browser/aw_content_browser_client.cc
[modify] https://crrev.com/a67add8097a5846e30c0bb8d5847a59dc6567c40/android_webview/browser/aw_content_browser_client.h
[modify] https://crrev.com/a67add8097a5846e30c0bb8d5847a59dc6567c40/android_webview/browser/renderer_host/aw_resource_dispatcher_host_delegate.cc
[modify] https://crrev.com/a67add8097a5846e30c0bb8d5847a59dc6567c40/android_webview/browser/renderer_host/aw_resource_dispatcher_host_delegate.h
[modify] https://crrev.com/a67add8097a5846e30c0bb8d5847a59dc6567c40/chrome/browser/chrome_content_browser_client.cc
[modify] https://crrev.com/a67add8097a5846e30c0bb8d5847a59dc6567c40/chrome/browser/chrome_content_browser_client.h
[modify] https://crrev.com/a67add8097a5846e30c0bb8d5847a59dc6567c40/chrome/browser/chrome_site_per_process_browsertest.cc
[modify] https://crrev.com/a67add8097a5846e30c0bb8d5847a59dc6567c40/chrome/browser/external_protocol/external_protocol_handler.cc
[modify] https://crrev.com/a67add8097a5846e30c0bb8d5847a59dc6567c40/chrome/browser/external_protocol/external_protocol_handler.h
[modify] https://crrev.com/a67add8097a5846e30c0bb8d5847a59dc6567c40/chrome/browser/external_protocol/external_protocol_handler_unittest.cc
[modify] https://crrev.com/a67add8097a5846e30c0bb8d5847a59dc6567c40/chrome/browser/loader/chrome_resource_dispatcher_host_delegate.cc
[modify] https://crrev.com/a67add8097a5846e30c0bb8d5847a59dc6567c40/chrome/browser/loader/chrome_resource_dispatcher_host_delegate.h
[modify] https://crrev.com/a67add8097a5846e30c0bb8d5847a59dc6567c40/chrome/browser/prerender/prerender_test_utils.cc
[modify] https://crrev.com/a67add8097a5846e30c0bb8d5847a59dc6567c40/content/browser/loader/mojo_async_resource_handler_unittest.cc
[modify] https://crrev.com/a67add8097a5846e30c0bb8d5847a59dc6567c40/content/browser/loader/navigation_url_loader_network_service.cc
[modify] https://crrev.com/a67add8097a5846e30c0bb8d5847a59dc6567c40/content/browser/loader/navigation_url_loader_unittest.cc
[modify] https://crrev.com/a67add8097a5846e30c0bb8d5847a59dc6567c40/content/browser/loader/resource_dispatcher_host_impl.cc
[modify] https://crrev.com/a67add8097a5846e30c0bb8d5847a59dc6567c40/content/browser/loader/resource_dispatcher_host_unittest.cc
[modify] https://crrev.com/a67add8097a5846e30c0bb8d5847a59dc6567c40/content/browser/loader/resource_requester_info.cc
[modify] https://crrev.com/a67add8097a5846e30c0bb8d5847a59dc6567c40/content/public/browser/content_browser_client.cc
[modify] https://crrev.com/a67add8097a5846e30c0bb8d5847a59dc6567c40/content/public/browser/content_browser_client.h
[modify] https://crrev.com/a67add8097a5846e30c0bb8d5847a59dc6567c40/content/public/browser/resource_dispatcher_host_delegate.cc
[modify] https://crrev.com/a67add8097a5846e30c0bb8d5847a59dc6567c40/content/public/browser/resource_dispatcher_host_delegate.h
[modify] https://crrev.com/a67add8097a5846e30c0bb8d5847a59dc6567c40/extensions/BUILD.gn
[modify] https://crrev.com/a67add8097a5846e30c0bb8d5847a59dc6567c40/extensions/browser/guest_view/web_view/web_view_apitest.cc
[modify] https://crrev.com/a67add8097a5846e30c0bb8d5847a59dc6567c40/extensions/test/data/web_view/apitest/main.js
[modify] https://crrev.com/a67add8097a5846e30c0bb8d5847a59dc6567c40/testing/buildbot/chromium.fyi.json
[modify] https://crrev.com/a67add8097a5846e30c0bb8d5847a59dc6567c40/testing/buildbot/chromium.linux.json
[modify] https://crrev.com/a67add8097a5846e30c0bb8d5847a59dc6567c40/testing/buildbot/filters/BUILD.gn
[modify] https://crrev.com/a67add8097a5846e30c0bb8d5847a59dc6567c40/testing/buildbot/filters/mojo.fyi.network_browser_tests.filter
[delete] https://crrev.com/d103aafd9e94060aba30f87d58cfb041d815061c/testing/buildbot/filters/mojo.fyi.network_extensions_browsertests.filter
[modify] https://crrev.com/a67add8097a5846e30c0bb8d5847a59dc6567c40/testing/buildbot/test_suites.pyl

Comment 8 by jam@chromium.org, Mar 10 2018

Status: Fixed (was: Started)

Sign in to add a comment