Skip to content

Commit

Permalink
Revert 188912 "Removed static factories for data, ftp, file, and..."
Browse files Browse the repository at this point in the history
Broke layout tests userscripts/user-script-plugin-document.html and plugins/plugin-document-back-forward.html on all platforms.

> Removed static factories for data, ftp, file, and about jobs.
> Instead add corresponding ProtocolHandlers as needed.
> Remove URLRequestContext members used by these static 
> factories.  Bake FtpAuthCache into FtpProtocolHandler as it
> was already unique per FtpProtocolHandler.
> This is a revived version of http://crrev.com/10836206
> 
> BUG=142945
> 
> 
> Review URL: https://chromiumcodereview.appspot.com/11931024

TBR=pauljensen@chromium.org
Review URL: https://codereview.chromium.org/12605011

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@188927 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
kbr@chromium.org committed Mar 19, 2013
1 parent 433739c commit e13c146
Show file tree
Hide file tree
Showing 58 changed files with 238 additions and 299 deletions.
2 changes: 0 additions & 2 deletions android_webview/browser/net/aw_url_request_context_getter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -52,9 +52,7 @@ void AwURLRequestContextGetter::Init() {
net::URLRequestContextBuilder builder;
builder.set_user_agent(content::GetUserAgent(GURL()));
builder.set_network_delegate(new AwNetworkDelegate());
#if !defined(DISABLE_FTP_SUPPORT)
builder.set_ftp_enabled(false); // Android WebView does not support ftp yet.
#endif
builder.set_proxy_config_service(proxy_config_service_.release());
builder.set_accept_language(net::HttpUtil::GenerateAcceptLanguageHeader(
content::GetContentClient()->browser()->GetAcceptLangs(
Expand Down
4 changes: 0 additions & 4 deletions android_webview/browser/net/aw_url_request_job_factory.cc
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,4 @@ bool AwURLRequestJobFactory::SetProtocolHandler(
return next_factory_->SetProtocolHandler(scheme, protocol_handler);
}

bool AwURLRequestJobFactory::IsSafeRedirectTarget(const GURL& location) const {
return next_factory_->IsSafeRedirectTarget(location);
}

} // namespace android_webview
1 change: 0 additions & 1 deletion android_webview/browser/net/aw_url_request_job_factory.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ class AwURLRequestJobFactory : public net::URLRequestJobFactory {
net::NetworkDelegate* network_delegate) const OVERRIDE;
virtual bool IsHandledProtocol(const std::string& scheme) const OVERRIDE;
virtual bool IsHandledURL(const GURL& url) const OVERRIDE;
virtual bool IsSafeRedirectTarget(const GURL& location) const OVERRIDE;

private:
// By default calls are forwarded to this factory, to avoid having to
Expand Down
6 changes: 0 additions & 6 deletions chrome/browser/custom_handlers/protocol_handler_registry.cc
Original file line number Diff line number Diff line change
Expand Up @@ -205,12 +205,6 @@ bool ProtocolHandlerRegistry::JobInterceptorFactory::IsHandledURL(
job_factory_->IsHandledURL(url);
}

bool ProtocolHandlerRegistry::JobInterceptorFactory::IsSafeRedirectTarget(
const GURL& location) const {
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
return job_factory_->IsSafeRedirectTarget(location);
}

// DefaultClientObserver ------------------------------------------------------

ProtocolHandlerRegistry::DefaultClientObserver::DefaultClientObserver(
Expand Down
1 change: 0 additions & 1 deletion chrome/browser/custom_handlers/protocol_handler_registry.h
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,6 @@ class ProtocolHandlerRegistry : public ProfileKeyedService {
net::NetworkDelegate* network_delegate) const OVERRIDE;
virtual bool IsHandledProtocol(const std::string& scheme) const OVERRIDE;
virtual bool IsHandledURL(const GURL& url) const OVERRIDE;
virtual bool IsSafeRedirectTarget(const GURL& location) const OVERRIDE;

private:
// When JobInterceptorFactory decides to pass on particular requests,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,9 +73,6 @@ class FakeURLRequestJobFactory : public net::URLRequestJobFactory {
virtual bool IsHandledURL(const GURL& url) const OVERRIDE {
return false;
}
virtual bool IsSafeRedirectTarget(const GURL& location) const OVERRIDE {
return true;
}
};

void AssertWillHandleIO(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@
#include "chrome/browser/extensions/api/web_request/web_request_api_helpers.h"
#include "chrome/browser/extensions/event_router_forwarder.h"
#include "chrome/browser/extensions/extension_warning_set.h"
#include "chrome/browser/net/about_protocol_handler.h"
#include "chrome/browser/net/chrome_network_delegate.h"
#include "chrome/common/extensions/extension_messages.h"
#include "chrome/common/extensions/features/feature.h"
Expand All @@ -37,7 +36,6 @@
#include "chrome/test/base/testing_pref_service_syncable.h"
#include "chrome/test/base/testing_profile.h"
#include "chrome/test/base/testing_profile_manager.h"
#include "content/public/common/url_constants.h"
#include "content/public/test/test_browser_thread.h"
#include "net/base/auth.h"
#include "net/base/capturing_net_log.h"
Expand All @@ -46,7 +44,6 @@
#include "net/base/upload_bytes_element_reader.h"
#include "net/base/upload_data_stream.h"
#include "net/base/upload_file_element_reader.h"
#include "net/url_request/url_request_job_factory_impl.h"
#include "net/url_request/url_request_test_util.h"
#include "testing/gtest/include/gtest/gtest-message.h"
#include "testing/gtest/include/gtest/gtest.h"
Expand Down Expand Up @@ -233,12 +230,6 @@ TEST_F(ExtensionWebRequestTest, BlockingEventPrecedenceRedirect) {
filter, ExtensionWebRequestEventRouter::ExtraInfoSpec::BLOCKING, -1, -1,
ipc_sender_factory.GetWeakPtr());

net::URLRequestJobFactoryImpl job_factory;
job_factory.SetProtocolHandler(
chrome::kAboutScheme,
new chrome_browser_net::AboutProtocolHandler());
context_->set_job_factory(&job_factory);

GURL redirect_url("about:redirected");
GURL not_chosen_redirect_url("about:not_chosen");

Expand Down
9 changes: 9 additions & 0 deletions chrome/browser/io_thread.cc
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@
#include "net/base/net_util.h"
#include "net/base/sdch_manager.h"
#include "net/cookies/cookie_monster.h"
#include "net/ftp/ftp_network_layer.h"
#include "net/http/http_auth_filter.h"
#include "net/http/http_auth_handler_factory.h"
#include "net/http/http_network_layer.h"
Expand Down Expand Up @@ -185,6 +186,8 @@ ConstructProxyScriptFetcherContext(IOThread::Globals* globals,
context->set_proxy_service(globals->proxy_script_fetcher_proxy_service.get());
context->set_http_transaction_factory(
globals->proxy_script_fetcher_http_transaction_factory.get());
context->set_ftp_transaction_factory(
globals->proxy_script_fetcher_ftp_transaction_factory.get());
context->set_cookie_store(globals->system_cookie_store.get());
context->set_server_bound_cert_service(
globals->system_server_bound_cert_service.get());
Expand All @@ -211,6 +214,8 @@ ConstructSystemRequestContext(IOThread::Globals* globals,
context->set_proxy_service(globals->system_proxy_service.get());
context->set_http_transaction_factory(
globals->system_http_transaction_factory.get());
context->set_ftp_transaction_factory(
globals->system_ftp_transaction_factory.get());
context->set_cookie_store(globals->system_cookie_store.get());
context->set_server_bound_cert_service(
globals->system_server_bound_cert_service.get());
Expand Down Expand Up @@ -545,6 +550,8 @@ void IOThread::Init() {
new net::HttpNetworkSession(session_params));
globals_->proxy_script_fetcher_http_transaction_factory.reset(
new net::HttpNetworkLayer(network_session));
globals_->proxy_script_fetcher_ftp_transaction_factory.reset(
new net::FtpNetworkLayer(globals_->host_resolver.get()));

globals_->throttler_manager.reset(new net::URLRequestThrottlerManager());
globals_->throttler_manager->set_net_log(net_log_);
Expand Down Expand Up @@ -889,6 +896,8 @@ void IOThread::InitSystemRequestContextOnIOThread() {
globals_->system_http_transaction_factory.reset(
new net::HttpNetworkLayer(
new net::HttpNetworkSession(system_params)));
globals_->system_ftp_transaction_factory.reset(
new net::FtpNetworkLayer(globals_->host_resolver.get()));
globals_->system_request_context.reset(
ConstructSystemRequestContext(globals_, net_log_));

Expand Down
3 changes: 3 additions & 0 deletions chrome/browser/io_thread.h
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,8 @@ class IOThread : public content::BrowserThreadDelegate {
scoped_ptr<net::ProxyService> proxy_script_fetcher_proxy_service;
scoped_ptr<net::HttpTransactionFactory>
proxy_script_fetcher_http_transaction_factory;
scoped_ptr<net::FtpTransactionFactory>
proxy_script_fetcher_ftp_transaction_factory;
scoped_ptr<net::URLRequestThrottlerManager> throttler_manager;
scoped_ptr<net::URLSecurityManager> url_security_manager;
// TODO(willchan): Remove proxy script fetcher context since it's not
Expand All @@ -132,6 +134,7 @@ class IOThread : public content::BrowserThreadDelegate {
scoped_ptr<net::URLRequestContext> proxy_script_fetcher_context;
scoped_ptr<net::ProxyService> system_proxy_service;
scoped_ptr<net::HttpTransactionFactory> system_http_transaction_factory;
scoped_ptr<net::FtpTransactionFactory> system_ftp_transaction_factory;
scoped_ptr<net::URLRequestContext> system_request_context;
SystemRequestContextLeakChecker system_request_context_leak_checker;
// |system_cookie_store| and |system_server_bound_cert_service| are shared
Expand Down
4 changes: 0 additions & 4 deletions chrome/browser/net/about_protocol_handler.cc
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,4 @@ net::URLRequestJob* AboutProtocolHandler::MaybeCreateJob(
return new net::URLRequestAboutJob(request, network_delegate);
}

bool AboutProtocolHandler::IsSafeRedirectTarget(const GURL& location) const {
return false;
}

} // namespace chrome_browser_net
1 change: 0 additions & 1 deletion chrome/browser/net/about_protocol_handler.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ class AboutProtocolHandler : public net::URLRequestJobFactory::ProtocolHandler {
virtual net::URLRequestJob* MaybeCreateJob(
net::URLRequest* request,
net::NetworkDelegate* network_delegate) const OVERRIDE;
virtual bool IsSafeRedirectTarget(const GURL& location) const OVERRIDE;

private:
DISALLOW_COPY_AND_ASSIGN(AboutProtocolHandler);
Expand Down
5 changes: 5 additions & 0 deletions chrome/browser/net/connection_tester.cc
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#include "net/base/net_errors.h"
#include "net/base/net_util.h"
#include "net/cookies/cookie_monster.h"
#include "net/ftp/ftp_network_layer.h"
#include "net/http/http_auth_handler_factory.h"
#include "net/http/http_cache.h"
#include "net/http/http_network_session.h"
Expand Down Expand Up @@ -108,6 +109,10 @@ class ExperimentURLRequestContext : public net::URLRequestContext {
// The rest of the dependencies are standard, and don't depend on the
// experiment being run.
storage_.set_cert_verifier(net::CertVerifier::CreateDefault());
#if !defined(DISABLE_FTP_SUPPORT)
storage_.set_ftp_transaction_factory(
new net::FtpNetworkLayer(host_resolver()));
#endif
storage_.set_ssl_config_service(new net::SSLConfigServiceDefaults);
storage_.set_http_auth_handler_factory(
net::HttpAuthHandlerFactory::CreateDefault(host_resolver()));
Expand Down
15 changes: 12 additions & 3 deletions chrome/browser/profiles/off_the_record_profile_io_data.cc
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,7 @@ void OffTheRecordProfileIOData::InitializeInternal(
#if !defined(DISABLE_FTP_SUPPORT)
ftp_factory_.reset(
new net::FtpNetworkLayer(main_context->host_resolver()));
main_context->set_ftp_transaction_factory(ftp_factory_.get());
#endif // !defined(DISABLE_FTP_SUPPORT)

scoped_ptr<net::URLRequestJobFactoryImpl> main_job_factory(
Expand All @@ -226,7 +227,8 @@ void OffTheRecordProfileIOData::InitializeInternal(
main_job_factory.Pass(),
profile_params->protocol_handler_interceptor.Pass(),
network_delegate(),
ftp_factory_.get());
main_context->ftp_transaction_factory(),
main_context->ftp_auth_cache());
main_context->set_job_factory(main_job_factory_.get());

#if defined(ENABLE_EXTENSIONS)
Expand Down Expand Up @@ -260,6 +262,11 @@ void OffTheRecordProfileIOData::
extensions_cookie_store->SetCookieableSchemes(schemes, 2);
extensions_context->set_cookie_store(extensions_cookie_store);

#if !defined(DISABLE_FTP_SUPPORT)
DCHECK(ftp_factory_.get());
extensions_context->set_ftp_transaction_factory(ftp_factory_.get());
#endif // !defined(DISABLE_FTP_SUPPORT)

scoped_ptr<net::URLRequestJobFactoryImpl> extensions_job_factory(
new net::URLRequestJobFactoryImpl());
// TODO(shalev): The extensions_job_factory has a NULL NetworkDelegate.
Expand All @@ -272,7 +279,8 @@ void OffTheRecordProfileIOData::
extensions_job_factory.Pass(),
scoped_ptr<ProtocolHandlerRegistry::JobInterceptorFactory>(NULL),
NULL,
ftp_factory_.get());
extensions_context->ftp_transaction_factory(),
extensions_context->ftp_auth_cache());
extensions_context->set_job_factory(extensions_job_factory_.get());
}

Expand Down Expand Up @@ -310,7 +318,8 @@ OffTheRecordProfileIOData::InitializeAppRequestContext(
top_job_factory = SetUpJobFactoryDefaults(job_factory.Pass(),
protocol_handler_interceptor.Pass(),
network_delegate(),
ftp_factory_.get());
context->ftp_transaction_factory(),
context->ftp_auth_cache());
context->SetJobFactory(top_job_factory.Pass());
return context;
}
Expand Down
5 changes: 0 additions & 5 deletions chrome/browser/profiles/off_the_record_profile_io_data.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,6 @@ class ChromeURLRequestContext;
class ChromeURLRequestContextGetter;
class Profile;

namespace net {
class FtpTransactionFactory;
class HttpTransactionFactory;
} // namespace net

// OffTheRecordProfile owns a OffTheRecordProfileIOData::Handle, which holds a
// reference to the OffTheRecordProfileIOData. OffTheRecordProfileIOData is
// intended to own all the objects owned by OffTheRecordProfile which live on
Expand Down
15 changes: 12 additions & 3 deletions chrome/browser/profiles/profile_impl_io_data.cc
Original file line number Diff line number Diff line change
Expand Up @@ -411,6 +411,7 @@ void ProfileImplIOData::InitializeInternal(
#if !defined(DISABLE_FTP_SUPPORT)
ftp_factory_.reset(
new net::FtpNetworkLayer(io_thread_globals->host_resolver.get()));
main_context->set_ftp_transaction_factory(ftp_factory_.get());
#endif // !defined(DISABLE_FTP_SUPPORT)

scoped_ptr<net::URLRequestJobFactoryImpl> main_job_factory(
Expand All @@ -420,7 +421,8 @@ void ProfileImplIOData::InitializeInternal(
main_job_factory.Pass(),
profile_params->protocol_handler_interceptor.Pass(),
network_delegate(),
ftp_factory_.get());
main_context->ftp_transaction_factory(),
main_context->ftp_auth_cache());
main_context->set_job_factory(main_job_factory_.get());

#if defined(ENABLE_EXTENSIONS)
Expand Down Expand Up @@ -464,6 +466,11 @@ void ProfileImplIOData::
extensions_cookie_store->SetCookieableSchemes(schemes, 2);
extensions_context->set_cookie_store(extensions_cookie_store);

#if !defined(DISABLE_FTP_SUPPORT)
DCHECK(ftp_factory_.get());
extensions_context->set_ftp_transaction_factory(ftp_factory_.get());
#endif // !defined(DISABLE_FTP_SUPPORT)

scoped_ptr<net::URLRequestJobFactoryImpl> extensions_job_factory(
new net::URLRequestJobFactoryImpl());
// TODO(shalev): The extensions_job_factory has a NULL NetworkDelegate.
Expand All @@ -476,7 +483,8 @@ void ProfileImplIOData::
extensions_job_factory.Pass(),
scoped_ptr<ProtocolHandlerRegistry::JobInterceptorFactory>(NULL),
NULL,
ftp_factory_.get());
extensions_context->ftp_transaction_factory(),
extensions_context->ftp_auth_cache());
extensions_context->set_job_factory(extensions_job_factory_.get());
}

Expand Down Expand Up @@ -568,7 +576,8 @@ ProfileImplIOData::InitializeAppRequestContext(
top_job_factory = SetUpJobFactoryDefaults(
job_factory.Pass(), protocol_handler_interceptor.Pass(),
network_delegate(),
ftp_factory_.get());
context->ftp_transaction_factory(),
context->ftp_auth_cache());
} else {
top_job_factory = job_factory.PassAs<net::URLRequestJobFactory>();
}
Expand Down
1 change: 0 additions & 1 deletion chrome/browser/profiles/profile_impl_io_data.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ class Predictor;
} // namespace chrome_browser_net

namespace net {
class FtpTransactionFactory;
class HttpServerProperties;
class HttpTransactionFactory;
} // namespace net
Expand Down
17 changes: 7 additions & 10 deletions chrome/browser/profiles/profile_io_data.cc
Original file line number Diff line number Diff line change
Expand Up @@ -421,21 +421,16 @@ ProfileIOData* ProfileIOData::FromResourceContext(
bool ProfileIOData::IsHandledProtocol(const std::string& scheme) {
DCHECK_EQ(scheme, StringToLowerASCII(scheme));
static const char* const kProtocolList[] = {
chrome::kFileScheme,
chrome::kChromeDevToolsScheme,
extensions::kExtensionScheme,
chrome::kExtensionResourceScheme,
chrome::kChromeUIScheme,
chrome::kDataScheme,
chrome::kChromeDevToolsScheme,
#if defined(OS_CHROMEOS)
chrome::kMetadataScheme,
chrome::kDriveScheme,
#endif // defined(OS_CHROMEOS)
chrome::kAboutScheme,
#if !defined(DISABLE_FTP_SUPPORT)
chrome::kFtpScheme,
#endif // !defined(DISABLE_FTP_SUPPORT)
chrome::kBlobScheme,
chrome::kFileSystemScheme,
chrome::kExtensionResourceScheme,
chrome::kChromeSearchScheme,
};
for (size_t i = 0; i < arraysize(kProtocolList); ++i) {
Expand Down Expand Up @@ -717,7 +712,8 @@ scoped_ptr<net::URLRequestJobFactory> ProfileIOData::SetUpJobFactoryDefaults(
scoped_ptr<ProtocolHandlerRegistry::JobInterceptorFactory>
protocol_handler_interceptor,
net::NetworkDelegate* network_delegate,
net::FtpTransactionFactory* ftp_transaction_factory) const {
net::FtpTransactionFactory* ftp_transaction_factory,
net::FtpAuthCache* ftp_auth_cache) const {
// NOTE(willchan): Keep these protocol handlers in sync with
// ProfileIOData::IsHandledProtocol().
bool set_protocol = job_factory->SetProtocolHandler(
Expand Down Expand Up @@ -752,7 +748,8 @@ scoped_ptr<net::URLRequestJobFactory> ProfileIOData::SetUpJobFactoryDefaults(
DCHECK(ftp_transaction_factory);
job_factory->SetProtocolHandler(
chrome::kFtpScheme,
new net::FtpProtocolHandler(ftp_transaction_factory));
new net::FtpProtocolHandler(ftp_transaction_factory,
ftp_auth_cache));
#endif // !defined(DISABLE_FTP_SUPPORT)

scoped_ptr<net::URLRequestJobFactory> top_job_factory =
Expand Down
4 changes: 2 additions & 2 deletions chrome/browser/profiles/profile_io_data.h
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@ class ResourcePrefetchPredictorObserver;
namespace net {
class CookieStore;
class FraudulentCertificateReporter;
class FtpTransactionFactory;
class HttpServerProperties;
class HttpTransactionFactory;
class ServerBoundCertService;
Expand Down Expand Up @@ -287,7 +286,8 @@ class ProfileIOData {
scoped_ptr<ProtocolHandlerRegistry::JobInterceptorFactory>
protocol_handler_interceptor,
net::NetworkDelegate* network_delegate,
net::FtpTransactionFactory* ftp_transaction_factory) const;
net::FtpTransactionFactory* ftp_transaction_factory,
net::FtpAuthCache* ftp_auth_cache) const;

// Called when the profile is destroyed.
void ShutdownOnUIThread();
Expand Down
3 changes: 3 additions & 0 deletions chrome/service/net/service_url_request_context.cc
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
#include "net/base/cert_verifier.h"
#include "net/base/host_resolver.h"
#include "net/cookies/cookie_monster.h"
#include "net/ftp/ftp_network_layer.h"
#include "net/http/http_auth_handler_factory.h"
#include "net/http/http_cache.h"
#include "net/http/http_network_session.h"
Expand Down Expand Up @@ -113,6 +114,8 @@ ServiceURLRequestContext::ServiceURLRequestContext(
storage_.set_proxy_service(net::ProxyService::CreateUsingSystemProxyResolver(
net_proxy_config_service, 0u, NULL));
storage_.set_cert_verifier(net::CertVerifier::CreateDefault());
storage_.set_ftp_transaction_factory(
new net::FtpNetworkLayer(host_resolver()));
storage_.set_ssl_config_service(new net::SSLConfigServiceDefaults);
storage_.set_http_auth_handler_factory(
net::HttpAuthHandlerFactory::CreateDefault(host_resolver()));
Expand Down
Loading

0 comments on commit e13c146

Please sign in to comment.