Skip to content

Commit

Permalink
Revert of Make HttpCache constructors take scoped_ptrs. (patchset chr…
Browse files Browse the repository at this point in the history
…omium#6 id:120001 of https://codereview.chromium.org/1406203008/ )

Reason for revert:
Broke iOS Device (ninja) compilation. See https://build.chromium.org/p/chromium.mac/builders/iOS_Device_%28ninja%29/builds/27454

Original issue's description:
> Make HttpCache constructors take scoped_ptrs.
>
> BUG=479898
>
> Committed: https://crrev.com/d76f6c556e4f24218ee0a66e4208151aaff13f40
> Cr-Commit-Position: refs/heads/master@{#356630}

TBR=pauljensen@chromium.org,sgurun@chromium.org,kmarshall@chromium.org,peter@chromium.org,mmenke@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=479898

Review URL: https://codereview.chromium.org/1425533004

Cr-Commit-Position: refs/heads/master@{#356635}
  • Loading branch information
dgozman authored and Commit bot committed Oct 28, 2015
1 parent 7a13051 commit 2c4e6b4
Show file tree
Hide file tree
Showing 16 changed files with 105 additions and 101 deletions.
4 changes: 2 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 @@ -233,12 +233,12 @@ void AwURLRequestContextGetter::InitializeURLRequestContext() {
new net::HttpNetworkSession(network_session_params));
main_http_factory_.reset(new net::HttpCache(
http_network_session_.get(),
make_scoped_ptr(new net::HttpCache::DefaultBackend(
new net::HttpCache::DefaultBackend(
net::DISK_CACHE,
net::CACHE_BACKEND_SIMPLE,
cache_path_,
20 * 1024 * 1024, // 20M
BrowserThread::GetMessageLoopProxyForThread(BrowserThread::CACHE))),
BrowserThread::GetMessageLoopProxyForThread(BrowserThread::CACHE)),
true /* set_up_quic_server_info */));

url_request_context_->set_http_transaction_factory(main_http_factory_.get());
Expand Down
6 changes: 3 additions & 3 deletions blimp/engine/browser/blimp_url_request_context_getter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -142,11 +142,11 @@ net::URLRequestContext* BlimpURLRequestContextGetter::GetURLRequestContext() {
make_scoped_ptr(new net::HttpServerPropertiesImpl()));

base::FilePath cache_path = base_path_.Append(FILE_PATH_LITERAL("Cache"));
scoped_ptr<net::HttpCache::DefaultBackend> main_backend(
net::HttpCache::DefaultBackend* main_backend =
new net::HttpCache::DefaultBackend(
net::DISK_CACHE, net::CACHE_BACKEND_DEFAULT, cache_path, 0,
content::BrowserThread::GetMessageLoopProxyForThread(
content::BrowserThread::CACHE)));
content::BrowserThread::CACHE));

net::HttpNetworkSession::Params network_session_params;
network_session_params.cert_verifier =
Expand Down Expand Up @@ -183,7 +183,7 @@ net::URLRequestContext* BlimpURLRequestContextGetter::GetURLRequestContext() {
storage_->set_http_network_session(
make_scoped_ptr(new net::HttpNetworkSession(network_session_params)));
storage_->set_http_transaction_factory(make_scoped_ptr(new net::HttpCache(
storage_->http_network_session(), main_backend.Pass(), true)));
storage_->http_network_session(), main_backend, true)));

scoped_ptr<net::URLRequestJobFactoryImpl> job_factory(
new net::URLRequestJobFactoryImpl());
Expand Down
11 changes: 7 additions & 4 deletions chrome/browser/profiles/off_the_record_profile_io_data.cc
Original file line number Diff line number Diff line change
Expand Up @@ -245,9 +245,11 @@ void OffTheRecordProfileIOData::InitializeInternal(
NULL,
profile_params->cookie_monster_delegate.get())));

net::HttpCache::BackendFactory* main_backend =
net::HttpCache::DefaultBackend::InMemory(0);
http_network_session_ = CreateHttpNetworkSession(*profile_params);
main_http_factory_ = CreateMainHttpFactory(
http_network_session_.get(), net::HttpCache::DefaultBackend::InMemory(0));
main_http_factory_ = CreateMainHttpFactory(http_network_session_.get(),
main_backend);

main_context->set_http_transaction_factory(main_http_factory_.get());
#if !defined(DISABLE_FTP_SUPPORT)
Expand Down Expand Up @@ -343,9 +345,10 @@ net::URLRequestContext* OffTheRecordProfileIOData::InitializeAppRequestContext(
content::CreateCookieStore(content::CookieStoreConfig()));

// Use a separate in-memory cache for the app.
net::HttpCache::BackendFactory* app_backend =
net::HttpCache::DefaultBackend::InMemory(0);
scoped_ptr<net::HttpCache> app_http_cache =
CreateHttpFactory(http_network_session_.get(),
net::HttpCache::DefaultBackend::InMemory(0));
CreateHttpFactory(http_network_session_.get(), app_backend);

context->SetHttpTransactionFactory(app_http_cache.Pass());

Expand Down
18 changes: 9 additions & 9 deletions chrome/browser/profiles/profile_impl_io_data.cc
Original file line number Diff line number Diff line change
Expand Up @@ -539,7 +539,7 @@ void ProfileImplIOData::InitializeInternal(
// TODO(ttuttle): Remove ScopedTracker below once crbug.com/436671 is fixed.
tracked_objects::ScopedTracker tracking_profile(
FROM_HERE_WITH_EXPLICIT_FUNCTION("436671 HttpCache construction"));
scoped_ptr<net::HttpCache::BackendFactory> main_backend(
net::HttpCache::BackendFactory* main_backend(
new net::HttpCache::DefaultBackend(
net::DISK_CACHE,
ChooseCacheBackendType(),
Expand All @@ -548,7 +548,7 @@ void ProfileImplIOData::InitializeInternal(
BrowserThread::GetMessageLoopProxyForThread(BrowserThread::CACHE)));
http_network_session_ = CreateHttpNetworkSession(*profile_params);
main_http_factory_ = CreateMainHttpFactory(http_network_session_.get(),
main_backend.Pass());
main_backend);
}

main_context->set_http_transaction_factory(main_http_factory_.get());
Expand Down Expand Up @@ -660,19 +660,19 @@ net::URLRequestContext* ProfileImplIOData::InitializeAppRequestContext(
partition_descriptor.path.Append(chrome::kCacheDirname);

// Use a separate HTTP disk cache for isolated apps.
scoped_ptr<net::HttpCache::BackendFactory> app_backend;
net::HttpCache::BackendFactory* app_backend;
if (partition_descriptor.in_memory) {
app_backend = net::HttpCache::DefaultBackend::InMemory(0);
} else {
app_backend.reset(new net::HttpCache::DefaultBackend(
app_backend = new net::HttpCache::DefaultBackend(
net::DISK_CACHE,
ChooseCacheBackendType(),
cache_path,
app_cache_max_size_,
BrowserThread::GetMessageLoopProxyForThread(BrowserThread::CACHE)));
BrowserThread::GetMessageLoopProxyForThread(BrowserThread::CACHE));
}
scoped_ptr<net::HttpCache> app_http_cache =
CreateHttpFactory(http_network_session_.get(), app_backend.Pass());
CreateHttpFactory(http_network_session_.get(), app_backend);

scoped_refptr<net::CookieStore> cookie_store = NULL;
if (partition_descriptor.in_memory) {
Expand Down Expand Up @@ -744,15 +744,15 @@ ProfileImplIOData::InitializeMediaRequestContext(
}

// Use a separate HTTP disk cache for isolated apps.
scoped_ptr<net::HttpCache::BackendFactory> media_backend(
net::HttpCache::BackendFactory* media_backend =
new net::HttpCache::DefaultBackend(
net::MEDIA_CACHE,
ChooseCacheBackendType(),
cache_path,
cache_max_size,
BrowserThread::GetMessageLoopProxyForThread(BrowserThread::CACHE)));
BrowserThread::GetMessageLoopProxyForThread(BrowserThread::CACHE));
scoped_ptr<net::HttpCache> media_http_cache =
CreateHttpFactory(http_network_session_.get(), media_backend.Pass());
CreateHttpFactory(http_network_session_.get(), media_backend);

// Transfer ownership of the cache to MediaRequestContext.
context->SetHttpTransactionFactory(media_http_cache.Pass());
Expand Down
23 changes: 13 additions & 10 deletions chrome/browser/profiles/profile_io_data.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1331,20 +1331,23 @@ scoped_ptr<net::HttpNetworkSession> ProfileIOData::CreateHttpNetworkSession(

scoped_ptr<net::HttpCache> ProfileIOData::CreateMainHttpFactory(
net::HttpNetworkSession* session,
scoped_ptr<net::HttpCache::BackendFactory> main_backend) const {
return make_scoped_ptr(new net::HttpCache(
make_scoped_ptr(new DevToolsNetworkTransactionFactory(
network_controller_handle_.GetController(), session)),
main_backend.Pass(), true /* set_up_quic_server_info */));
net::HttpCache::BackendFactory* main_backend) const {
net::URLRequestContext* context = main_request_context();
return scoped_ptr<net::HttpCache>(new net::HttpCache(
new DevToolsNetworkTransactionFactory(
network_controller_handle_.GetController(), session),
context->net_log(), main_backend,
true /* set_up_quic_server_info */));
}

scoped_ptr<net::HttpCache> ProfileIOData::CreateHttpFactory(
net::HttpNetworkSession* shared_session,
scoped_ptr<net::HttpCache::BackendFactory> backend) const {
return make_scoped_ptr(new net::HttpCache(
make_scoped_ptr(new DevToolsNetworkTransactionFactory(
network_controller_handle_.GetController(), shared_session)),
backend.Pass(), true /* set_up_quic_server_info */));
net::HttpCache::BackendFactory* backend) const {
return scoped_ptr<net::HttpCache>(new net::HttpCache(
new DevToolsNetworkTransactionFactory(
network_controller_handle_.GetController(), shared_session),
shared_session->net_log(), backend,
true /* set_up_quic_server_info */));
}

void ProfileIOData::SetCookieSettingsForTesting(
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 @@ -389,12 +389,12 @@ class ProfileIOData {
// Creates main network transaction factory.
scoped_ptr<net::HttpCache> CreateMainHttpFactory(
net::HttpNetworkSession* session,
scoped_ptr<net::HttpCache::BackendFactory> main_backend) const;
net::HttpCache::BackendFactory* main_backend) const;

// Creates network transaction factory.
scoped_ptr<net::HttpCache> CreateHttpFactory(
net::HttpNetworkSession* shared_session,
scoped_ptr<net::HttpCache::BackendFactory> backend) const;
net::HttpCache::BackendFactory* backend) const;

void SetCookieSettingsForTesting(
content_settings::CookieSettings* cookie_settings);
Expand Down
7 changes: 3 additions & 4 deletions content/shell/browser/shell_url_request_context_getter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ net::URLRequestContext* ShellURLRequestContextGetter::GetURLRequestContext() {
make_scoped_ptr(new net::HttpServerPropertiesImpl()));

base::FilePath cache_path = base_path_.Append(FILE_PATH_LITERAL("Cache"));
scoped_ptr<net::HttpCache::DefaultBackend> main_backend(
net::HttpCache::DefaultBackend* main_backend =
new net::HttpCache::DefaultBackend(
net::DISK_CACHE,
#if defined(OS_ANDROID)
Expand All @@ -153,7 +153,7 @@ net::URLRequestContext* ShellURLRequestContextGetter::GetURLRequestContext() {
#endif
cache_path,
0,
BrowserThread::GetMessageLoopProxyForThread(BrowserThread::CACHE)));
BrowserThread::GetMessageLoopProxyForThread(BrowserThread::CACHE));

net::HttpNetworkSession::Params network_session_params;
network_session_params.cert_verifier =
Expand Down Expand Up @@ -204,8 +204,7 @@ net::URLRequestContext* ShellURLRequestContextGetter::GetURLRequestContext() {
storage_->set_http_network_session(
make_scoped_ptr(new net::HttpNetworkSession(network_session_params)));
storage_->set_http_transaction_factory(make_scoped_ptr(
new net::HttpCache(storage_->http_network_session(),
main_backend.Pass(),
new net::HttpCache(storage_->http_network_session(), main_backend,
true /* set_up_quic_server_info */)));

scoped_ptr<net::URLRequestJobFactoryImpl> job_factory(
Expand Down
8 changes: 4 additions & 4 deletions net/http/disk_cache_based_quic_server_info_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ TEST(DiskCacheBasedQuicServerInfo, DeleteInCallback) {
// Use the blocking mock backend factory to force asynchronous completion
// of quic_server_info->WaitForDataReady(), so that the callback will run.
MockBlockingBackendFactory* factory = new MockBlockingBackendFactory();
MockHttpCache cache(make_scoped_ptr(factory));
MockHttpCache cache(factory);
QuicServerId server_id("www.verisign.com", 443, PRIVACY_MODE_DISABLED);
scoped_ptr<QuicServerInfo> quic_server_info(
new DiskCacheBasedQuicServerInfo(server_id, cache.http_cache()));
Expand Down Expand Up @@ -390,7 +390,7 @@ TEST(DiskCacheBasedQuicServerInfo, MultiplePersist) {

TEST(DiskCacheBasedQuicServerInfo, CancelWaitForDataReady) {
MockBlockingBackendFactory* factory = new MockBlockingBackendFactory();
MockHttpCache cache(make_scoped_ptr(factory));
MockHttpCache cache(factory);
TestCompletionCallback callback;
QuicServerId server_id("www.google.com", 443, PRIVACY_MODE_DISABLED);
scoped_ptr<QuicServerInfo> quic_server_info(
Expand Down Expand Up @@ -508,7 +508,7 @@ TEST(DiskCacheBasedQuicServerInfo, StartAndPersist) {
// persists the data when Start() finishes.
TEST(DiskCacheBasedQuicServerInfo, PersistWhenNotReadyToPersist) {
MockBlockingBackendFactory* factory = new MockBlockingBackendFactory();
MockHttpCache cache(make_scoped_ptr(factory));
MockHttpCache cache(factory);
AddMockTransaction(&kHostInfoTransaction1);
TestCompletionCallback callback;

Expand Down Expand Up @@ -639,7 +639,7 @@ TEST(DiskCacheBasedQuicServerInfo, DeleteServerInfoInCallback) {
// Use the blocking mock backend factory to force asynchronous completion
// of quic_server_info->WaitForDataReady(), so that the callback will run.
MockBlockingBackendFactory* factory = new MockBlockingBackendFactory();
MockHttpCache cache(make_scoped_ptr(factory));
MockHttpCache cache(factory);
QuicServerId server_id("www.verisign.com", 443, PRIVACY_MODE_DISABLED);
QuicServerInfo* quic_server_info =
new DiskCacheBasedQuicServerInfo(server_id, cache.http_cache());
Expand Down
52 changes: 26 additions & 26 deletions net/http/http_cache.cc
Original file line number Diff line number Diff line change
Expand Up @@ -74,11 +74,9 @@ HttpCache::DefaultBackend::DefaultBackend(
HttpCache::DefaultBackend::~DefaultBackend() {}

// static
scoped_ptr<HttpCache::BackendFactory> HttpCache::DefaultBackend::InMemory(
int max_bytes) {
return make_scoped_ptr(new DefaultBackend(MEMORY_CACHE, CACHE_BACKEND_DEFAULT,
base::FilePath(), max_bytes,
nullptr));
HttpCache::BackendFactory* HttpCache::DefaultBackend::InMemory(int max_bytes) {
return new DefaultBackend(MEMORY_CACHE, CACHE_BACKEND_DEFAULT,
base::FilePath(), max_bytes, NULL);
}

int HttpCache::DefaultBackend::CreateBackend(
Expand Down Expand Up @@ -296,38 +294,31 @@ class HttpCache::QuicServerInfoFactoryAdaptor : public QuicServerInfoFactory {
};

//-----------------------------------------------------------------------------
// This call doesn't change the shared |session|'s QuicServerInfoFactory because
// |session| is shared.
HttpCache::HttpCache(HttpNetworkSession* session,
scoped_ptr<BackendFactory> backend_factory,
BackendFactory* backend_factory,
bool set_up_quic_server_info)
: HttpCache(make_scoped_ptr(new HttpNetworkLayer(session)),
backend_factory.Pass(),
: HttpCache(new HttpNetworkLayer(session),
session->net_log(),
backend_factory,
set_up_quic_server_info) {}

HttpCache::HttpCache(scoped_ptr<HttpTransactionFactory> network_layer,
scoped_ptr<BackendFactory> backend_factory,
HttpCache::HttpCache(HttpTransactionFactory* network_layer,
NetLog* net_log,
BackendFactory* backend_factory,
bool set_up_quic_server_info)
: net_log_(nullptr),
backend_factory_(backend_factory.Pass()),
: net_log_(net_log),
backend_factory_(backend_factory),
building_backend_(false),
bypass_lock_for_test_(false),
fail_conditionalization_for_test_(false),
mode_(NORMAL),
network_layer_(network_layer.Pass()),
network_layer_(network_layer),
clock_(new base::DefaultClock()),
weak_factory_(this) {
HttpNetworkSession* session = network_layer_->GetSession();
// Session may be NULL in unittests.
// TODO(mmenke): Seems like tests could be changed to provide a session,
// rather than having logic only used in unit tests here.
if (session) {
net_log_ = session->net_log();
if (set_up_quic_server_info &&
!session->quic_stream_factory()->has_quic_server_info_factory()) {
// QuicStreamFactory takes ownership of QuicServerInfoFactoryAdaptor.
session->quic_stream_factory()->set_quic_server_info_factory(
new QuicServerInfoFactoryAdaptor(this));
}
}
if (set_up_quic_server_info)
SetupQuicServerInfoFactory(network_layer_->GetSession());
}

HttpCache::~HttpCache() {
Expand Down Expand Up @@ -987,6 +978,15 @@ bool HttpCache::RemovePendingTransactionFromPendingOp(PendingOp* pending_op,
return false;
}

void HttpCache::SetupQuicServerInfoFactory(HttpNetworkSession* session) {
if (session &&
!session->quic_stream_factory()->has_quic_server_info_factory()) {
// QuicStreamFactory takes ownership of QuicServerInfoFactoryAdaptor.
session->quic_stream_factory()->set_quic_server_info_factory(
new QuicServerInfoFactoryAdaptor(this));
}
}

void HttpCache::ProcessPendingQueue(ActiveEntry* entry) {
// Multiple readers may finish with an entry at once, so we want to batch up
// calls to OnProcessPendingQueue. This flag also tells us that we should
Expand Down
18 changes: 12 additions & 6 deletions net/http/http_cache.h
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ class NET_EXPORT HttpCache : public HttpTransactionFactory,
~DefaultBackend() override;

// Returns a factory for an in-memory cache.
static scoped_ptr<BackendFactory> InMemory(int max_bytes);
static BackendFactory* InMemory(int max_bytes);

// BackendFactory implementation.
int CreateBackend(NetLog* net_log,
Expand Down Expand Up @@ -136,13 +136,15 @@ class NET_EXPORT HttpCache : public HttpTransactionFactory,
// If |set_up_quic_server_info| is true, configures the cache to track
// information about servers supporting QUIC.
HttpCache(HttpNetworkSession* session,
scoped_ptr<BackendFactory> backend_factory,
BackendFactory* backend_factory,
bool set_up_quic_server_info);

// Initialize the cache from its component parts. |network_layer| and
// |backend_factory| will be destroyed when the HttpCache is.
HttpCache(scoped_ptr<HttpTransactionFactory> network_layer,
scoped_ptr<BackendFactory> backend_factory,
// Initialize the cache from its component parts. The lifetime of the
// |network_layer| and |backend_factory| are managed by the HttpCache and
// will be destroyed using |delete| when the HttpCache is destroyed.
HttpCache(HttpTransactionFactory* network_layer,
NetLog* net_log,
BackendFactory* backend_factory,
bool set_up_quic_server_info);

~HttpCache() override;
Expand Down Expand Up @@ -378,6 +380,10 @@ class NET_EXPORT HttpCache : public HttpTransactionFactory,
// Removes the transaction |trans|, from the pending list of |pending_op|.
bool RemovePendingTransactionFromPendingOp(PendingOp* pending_op,
Transaction* trans);

// Instantiates and sets QUIC server info factory.
void SetupQuicServerInfoFactory(HttpNetworkSession* session);

// Resumes processing the pending list of |entry|.
void ProcessPendingQueue(ActiveEntry* entry);

Expand Down
Loading

0 comments on commit 2c4e6b4

Please sign in to comment.