Chromium ContentBrowserclient Embedder api IsHandledUrl was not impl… #289
Conversation
atom/browser/atom_browser_client.cc
Outdated
| return false; | ||
| // Keep in sync with ProtocolHandlers added by | ||
| // AtomBrowserContext::GetURLRequestContext(). | ||
| static const char* const kProtocolList[] = { |
There was a problem hiding this comment.
I think we should call URLRequestJobFactory::IsHandledProtocol because we have some custom protocols registered in atom::api::Protocol
There was a problem hiding this comment.
I think it also needs net::URLRequest::IsHandledProtocol(scheme); and can remove the http/ws/ftp protocols. See ProfileIOData::IsHandledProtocol
There was a problem hiding this comment.
@darkdh, the atom_browser_client do not have access to URLRequestJobFactory or browsercontext so we cannot use URLRequestJobFactory::IsHandledProtocol. Thats the reason we need to keep them in sync manually with all protocol handlers added by embedder in AtomBrowserContext::CreateURLRequestJobFactory. I have no knowledge on electron/atom api, i request you to add more protocol in the list if the atom protocol api adds one.
@bridiver, added the change for net::URLRequest::IsHandledProtocol(scheme).
cannot remove the http/ws/ftp because these protocol handlers are added in AtomBrowserContext::CreateURLRequestJobFactory
There was a problem hiding this comment.
@gnanasekar-somanathan http/https/ws/wss are included in net::URLRequest::IsHandledProtocol
There was a problem hiding this comment.
I think we also need content::kChromeDevToolsScheme, extensions::kExtensionScheme, url::kAboutScheme and content::kChromeUIScheme
There was a problem hiding this comment.
should mostly match ProfileIOData::IsHandledProtocol, but we don't need the chromeos, android, dom_distiller::kDomDistillerScheme, or chrome::kChromeSearchScheme
There was a problem hiding this comment.
also please move to brave_content_browser_client. atom_browser_client is deprecated and will be merged into brave_content_browser_client
gnanasekar-somanathan
force-pushed
the
fix/10644
branch
from
020c2bc
to
be6ec65
Compare
…mented. This should be in sync with registered protocol handlers.
gnanasekar-somanathan
force-pushed
the
fix/10644
branch
from
be6ec65
to
829e868
Compare
|
Addressed all the review comments. |
|
++ |
|
forgot to merge this for 4.4.26, I'll get it in 4.4.27 |
| extensions::kExtensionScheme, | ||
| #endif | ||
| content::kChromeUIScheme, | ||
| url::kAboutScheme, |
There was a problem hiding this comment.
should this also have url::kDataScheme and url::kBlobScheme?
Chromium ContentBrowserclient Embedder api IsHandledUrl was not implemented. Add the implementation for the same. This should be in sync with registered protocol handlers.
fix brave/browser-laptop#10644