diff options
author | eroman@chromium.org <eroman@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2009-09-24 03:49:17 +0000 |
---|---|---|
committer | eroman@chromium.org <eroman@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2009-09-24 03:49:17 +0000 |
commit | 1638d605a0f616cbdc1ac7c9fdb6422426808df5 (patch) | |
tree | d43290491a2d8516df387994ff45ed9af030b5d3 /net/http | |
parent | 559506ebab6e2aa5aa23e9776774bcdaf1901fe2 (diff) | |
download | chromium_src-1638d605a0f616cbdc1ac7c9fdb6422426808df5.zip chromium_src-1638d605a0f616cbdc1ac7c9fdb6422426808df5.tar.gz chromium_src-1638d605a0f616cbdc1ac7c9fdb6422426808df5.tar.bz2 |
Replace some net::ERR_FAILED generic error codes with more specific codes.
The goal is to end up with more meaningful errors if a page fails to load.
BUG=22623
Review URL: http://codereview.chromium.org/222009
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@27038 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'net/http')
-rw-r--r-- | net/http/http_cache.cc | 14 | ||||
-rw-r--r-- | net/http/http_cache.h | 2 | ||||
-rw-r--r-- | net/http/http_cache_unittest.cc | 134 | ||||
-rw-r--r-- | net/http/http_network_layer.cc | 7 | ||||
-rw-r--r-- | net/http/http_network_layer.h | 2 | ||||
-rw-r--r-- | net/http/http_network_layer_unittest.cc | 23 | ||||
-rw-r--r-- | net/http/http_transaction_factory.h | 7 | ||||
-rw-r--r-- | net/http/http_transaction_unittest.h | 9 |
8 files changed, 118 insertions, 80 deletions
diff --git a/net/http/http_cache.cc b/net/http/http_cache.cc index 9cb9786..6703661 100644 --- a/net/http/http_cache.cc +++ b/net/http/http_cache.cc @@ -1044,11 +1044,12 @@ int HttpCache::Transaction::BeginNetworkRequest() { DCHECK(mode_ & WRITE || mode_ == NONE); DCHECK(!network_trans_.get()); - network_trans_.reset(cache_->network_layer_->CreateTransaction()); - if (!network_trans_.get()) - return net::ERR_CACHE_CANNOT_CREATE_NETWORK_TRANSACTION; + // Create a network transaction. + int rv = cache_->network_layer_->CreateTransaction(&network_trans_); + if (rv != OK) + return rv; - int rv = network_trans_->Start(request_, &network_info_callback_, load_log_); + rv = network_trans_->Start(request_, &network_info_callback_, load_log_); if (rv != ERR_IO_PENDING) OnNetworkInfoAvailable(rv); return rv; @@ -1656,7 +1657,7 @@ HttpCache::~HttpCache() { delete *it; } -HttpTransaction* HttpCache::CreateTransaction() { +int HttpCache::CreateTransaction(scoped_ptr<HttpTransaction>* trans) { // Do lazy initialization of disk cache if needed. if (!disk_cache_.get()) { DCHECK_GE(cache_size_, 0); @@ -1671,7 +1672,8 @@ HttpTransaction* HttpCache::CreateTransaction() { disk_cache_dir_.clear(); // Reclaim memory. } } - return new HttpCache::Transaction(this, enable_range_support_); + trans->reset(new HttpCache::Transaction(this, enable_range_support_)); + return OK; } HttpCache* HttpCache::GetCache() { diff --git a/net/http/http_cache.h b/net/http/http_cache.h index b6599b2..652f3a4 100644 --- a/net/http/http_cache.h +++ b/net/http/http_cache.h @@ -93,7 +93,7 @@ class HttpCache : public HttpTransactionFactory { disk_cache::Backend* disk_cache() { return disk_cache_.get(); } // HttpTransactionFactory implementation: - virtual HttpTransaction* CreateTransaction(); + virtual int CreateTransaction(scoped_ptr<HttpTransaction>* trans); virtual HttpCache* GetCache(); virtual void Suspend(bool suspend); diff --git a/net/http/http_cache_unittest.cc b/net/http/http_cache_unittest.cc index aada69b..02933a5 100644 --- a/net/http/http_cache_unittest.cc +++ b/net/http/http_cache_unittest.cc @@ -394,10 +394,12 @@ void RunTransactionTestWithRequestAndLog(net::HttpCache* cache, // write to the cache - scoped_ptr<net::HttpTransaction> trans(cache->CreateTransaction()); + scoped_ptr<net::HttpTransaction> trans; + int rv = cache->CreateTransaction(&trans); + EXPECT_EQ(net::OK, rv); ASSERT_TRUE(trans.get()); - int rv = trans->Start(&request, &callback, load_log); + rv = trans->Start(&request, &callback, load_log); if (rv == net::ERR_IO_PENDING) rv = callback.WaitForResult(); ASSERT_EQ(net::OK, rv); @@ -651,8 +653,9 @@ struct Response { TEST(HttpCache, CreateThenDestroy) { MockHttpCache cache; - scoped_ptr<net::HttpTransaction> trans( - cache.http_cache()->CreateTransaction()); + scoped_ptr<net::HttpTransaction> trans; + int rv = cache.http_cache()->CreateTransaction(&trans); + EXPECT_EQ(net::OK, rv); ASSERT_TRUE(trans.get()); } @@ -775,11 +778,12 @@ TEST(HttpCache, SimpleGET_LoadOnlyFromCache_Miss) { MockHttpRequest request(transaction); TestCompletionCallback callback; - scoped_ptr<net::HttpTransaction> trans( - cache.http_cache()->CreateTransaction()); + scoped_ptr<net::HttpTransaction> trans; + int rv = cache.http_cache()->CreateTransaction(&trans); + EXPECT_EQ(net::OK, rv); ASSERT_TRUE(trans.get()); - int rv = trans->Start(&request, &callback, NULL); + rv = trans->Start(&request, &callback, NULL); if (rv == net::ERR_IO_PENDING) rv = callback.WaitForResult(); ASSERT_EQ(net::ERR_CACHE_MISS, rv); @@ -918,8 +922,7 @@ struct Context { TestCompletionCallback callback; scoped_ptr<net::HttpTransaction> trans; - explicit Context(net::HttpTransaction* t) - : result(net::ERR_IO_PENDING), trans(t) { + Context() : result(net::ERR_IO_PENDING) { } }; @@ -932,13 +935,13 @@ TEST(HttpCache, SimpleGET_ManyReaders) { const int kNumTransactions = 5; for (int i = 0; i < kNumTransactions; ++i) { - context_list.push_back( - new Context(cache.http_cache()->CreateTransaction())); - + context_list.push_back(new Context()); Context* c = context_list[i]; - int rv = c->trans->Start(&request, &c->callback, NULL); - if (rv != net::ERR_IO_PENDING) - c->result = rv; + + c->result = cache.http_cache()->CreateTransaction(&c->trans); + EXPECT_EQ(net::OK, c->result); + + c->result = c->trans->Start(&request, &c->callback, NULL); } // the first request should be a writer at this point, and the subsequent @@ -982,17 +985,17 @@ TEST(HttpCache, SimpleGET_RacingReaders) { const int kNumTransactions = 5; for (int i = 0; i < kNumTransactions; ++i) { - context_list.push_back( - new Context(cache.http_cache()->CreateTransaction())); - + context_list.push_back(new Context()); Context* c = context_list[i]; + + c->result = cache.http_cache()->CreateTransaction(&c->trans); + EXPECT_EQ(net::OK, c->result); + MockHttpRequest* this_request = &request; if (i == 1 || i == 2) this_request = &reader_request; - int rv = c->trans->Start(this_request, &c->callback, NULL); - if (rv != net::ERR_IO_PENDING) - c->result = rv; + c->result = c->trans->Start(this_request, &c->callback, NULL); } // The first request should be a writer at this point, and the subsequent @@ -1058,13 +1061,13 @@ TEST(HttpCache, FastNoStoreGET_DoneWithPending) { const int kNumTransactions = 3; for (int i = 0; i < kNumTransactions; ++i) { - context_list.push_back( - new Context(cache.http_cache()->CreateTransaction())); - + context_list.push_back(new Context()); Context* c = context_list[i]; - int rv = c->trans->Start(&request, &c->callback, NULL); - if (rv != net::ERR_IO_PENDING) - c->result = rv; + + c->result = cache.http_cache()->CreateTransaction(&c->trans); + EXPECT_EQ(net::OK, c->result); + + c->result = c->trans->Start(&request, &c->callback, NULL); } // The first request should be a writer at this point, and the subsequent @@ -1101,13 +1104,13 @@ TEST(HttpCache, SimpleGET_ManyWriters_CancelFirst) { const int kNumTransactions = 2; for (int i = 0; i < kNumTransactions; ++i) { - context_list.push_back( - new Context(cache.http_cache()->CreateTransaction())); - + context_list.push_back(new Context()); Context* c = context_list[i]; - int rv = c->trans->Start(&request, &c->callback, NULL); - if (rv != net::ERR_IO_PENDING) - c->result = rv; + + c->result = cache.http_cache()->CreateTransaction(&c->trans); + EXPECT_EQ(net::OK, c->result); + + c->result = c->trans->Start(&request, &c->callback, NULL); } // the first request should be a writer at this point, and the subsequent @@ -1155,9 +1158,10 @@ TEST(HttpCache, SimpleGET_AbandonedCacheRead) { MockHttpRequest request(kSimpleGET_Transaction); TestCompletionCallback callback; - scoped_ptr<net::HttpTransaction> trans( - cache.http_cache()->CreateTransaction()); - int rv = trans->Start(&request, &callback, NULL); + scoped_ptr<net::HttpTransaction> trans; + int rv = cache.http_cache()->CreateTransaction(&trans); + EXPECT_EQ(net::OK, rv); + rv = trans->Start(&request, &callback, NULL); if (rv == net::ERR_IO_PENDING) rv = callback.WaitForResult(); ASSERT_EQ(net::OK, rv); @@ -1668,11 +1672,12 @@ TEST(HttpCache, SimplePOST_LoadOnlyFromCache_Miss) { MockHttpRequest request(transaction); TestCompletionCallback callback; - scoped_ptr<net::HttpTransaction> trans( - cache.http_cache()->CreateTransaction()); + scoped_ptr<net::HttpTransaction> trans; + int rv = cache.http_cache()->CreateTransaction(&trans); + EXPECT_EQ(net::OK, rv); ASSERT_TRUE(trans.get()); - int rv = trans->Start(&request, &callback, NULL); + rv = trans->Start(&request, &callback, NULL); if (rv == net::ERR_IO_PENDING) rv = callback.WaitForResult(); ASSERT_EQ(net::ERR_CACHE_MISS, rv); @@ -2306,9 +2311,11 @@ TEST(HttpCache, RangeGET_Cancel) { MockHttpRequest request(kRangeGET_TransactionOK); - Context* c = new Context(cache.http_cache()->CreateTransaction()); + Context* c = new Context(); + int rv = cache.http_cache()->CreateTransaction(&c->trans); + EXPECT_EQ(net::OK, rv); - int rv = c->trans->Start(&request, &c->callback, NULL); + rv = c->trans->Start(&request, &c->callback, NULL); if (rv == net::ERR_IO_PENDING) rv = c->callback.WaitForResult(); @@ -2354,8 +2361,9 @@ TEST(HttpCache, RangeGET_OK_LoadOnlyFromCache) { MockHttpRequest request(transaction); TestCompletionCallback callback; - scoped_ptr<net::HttpTransaction> trans( - cache.http_cache()->CreateTransaction()); + scoped_ptr<net::HttpTransaction> trans; + int rv = cache.http_cache()->CreateTransaction(&trans); + EXPECT_EQ(net::OK, rv); ASSERT_TRUE(trans.get()); int rv = trans->Start(&request, &callback, NULL); @@ -2406,9 +2414,11 @@ TEST(HttpCache, DoomOnDestruction) { MockHttpRequest request(kSimpleGET_Transaction); - Context* c = new Context(cache.http_cache()->CreateTransaction()); + Context* c = new Context(); + int rv = cache.http_cache()->CreateTransaction(&c->trans); + EXPECT_EQ(net::OK, rv); - int rv = c->trans->Start(&request, &c->callback, NULL); + rv = c->trans->Start(&request, &c->callback, NULL); if (rv == net::ERR_IO_PENDING) c->result = c->callback.WaitForResult(); @@ -2434,9 +2444,11 @@ TEST(HttpCache, Set_Truncated_Flag) { MockHttpRequest request(kSimpleGET_Transaction); - Context* c = new Context(cache.http_cache()->CreateTransaction()); + Context* c = new Context(); + int rv = cache.http_cache()->CreateTransaction(&c->trans); + EXPECT_EQ(net::OK, rv); - int rv = c->trans->Start(&request, &c->callback, NULL); + rv = c->trans->Start(&request, &c->callback, NULL); if (rv == net::ERR_IO_PENDING) rv = c->callback.WaitForResult(); @@ -2593,11 +2605,12 @@ TEST(HttpCache, CachedRedirect) { // write to the cache { - scoped_ptr<net::HttpTransaction> trans( - cache.http_cache()->CreateTransaction()); + scoped_ptr<net::HttpTransaction> trans; + int rv = cache.http_cache()->CreateTransaction(&trans); + EXPECT_EQ(net::OK, rv); ASSERT_TRUE(trans.get()); - int rv = trans->Start(&request, &callback, NULL); + rv = trans->Start(&request, &callback, NULL); if (rv == net::ERR_IO_PENDING) rv = callback.WaitForResult(); ASSERT_EQ(net::OK, rv); @@ -2620,11 +2633,12 @@ TEST(HttpCache, CachedRedirect) { // read from the cache { - scoped_ptr<net::HttpTransaction> trans( - cache.http_cache()->CreateTransaction()); + scoped_ptr<net::HttpTransaction> trans; + int rv = cache.http_cache()->CreateTransaction(&trans); + EXPECT_EQ(net::OK, rv); ASSERT_TRUE(trans.get()); - int rv = trans->Start(&request, &callback, NULL); + rv = trans->Start(&request, &callback, NULL); if (rv == net::ERR_IO_PENDING) rv = callback.WaitForResult(); ASSERT_EQ(net::OK, rv); @@ -2747,11 +2761,12 @@ TEST(HttpCache, SimpleGET_SSLError) { MockHttpRequest request(transaction); TestCompletionCallback callback; - scoped_ptr<net::HttpTransaction> trans( - cache.http_cache()->CreateTransaction()); + scoped_ptr<net::HttpTransaction> trans; + int rv = cache.http_cache()->CreateTransaction(&trans); + EXPECT_EQ(net::OK, rv); ASSERT_TRUE(trans.get()); - int rv = trans->Start(&request, &callback, NULL); + rv = trans->Start(&request, &callback, NULL); if (rv == net::ERR_IO_PENDING) rv = callback.WaitForResult(); ASSERT_EQ(net::ERR_CACHE_MISS, rv); @@ -2761,9 +2776,12 @@ TEST(HttpCache, SimpleGET_SSLError) { TEST(HttpCache, OutlivedTransactions) { MockHttpCache* cache = new MockHttpCache; - net::HttpTransaction* trans = cache->http_cache()->CreateTransaction(); + scoped_ptr<net::HttpTransaction> trans; + int rv = cache->http_cache()->CreateTransaction(&trans); + EXPECT_EQ(net::OK, rv); + delete cache; - delete trans; + trans.reset(); } // Test that the disabled mode works. diff --git a/net/http/http_network_layer.cc b/net/http/http_network_layer.cc index 87969ba..edd8c34 100644 --- a/net/http/http_network_layer.cc +++ b/net/http/http_network_layer.cc @@ -59,11 +59,12 @@ HttpNetworkLayer::HttpNetworkLayer(HttpNetworkSession* session) HttpNetworkLayer::~HttpNetworkLayer() { } -HttpTransaction* HttpNetworkLayer::CreateTransaction() { +int HttpNetworkLayer::CreateTransaction(scoped_ptr<HttpTransaction>* trans) { if (suspended_) - return NULL; + return ERR_NETWORK_IO_SUSPENDED; - return new HttpNetworkTransaction(GetSession(), socket_factory_); + trans->reset(new HttpNetworkTransaction(GetSession(), socket_factory_)); + return OK; } HttpCache* HttpNetworkLayer::GetCache() { diff --git a/net/http/http_network_layer.h b/net/http/http_network_layer.h index 7253998..d73c0cd 100644 --- a/net/http/http_network_layer.h +++ b/net/http/http_network_layer.h @@ -45,7 +45,7 @@ class HttpNetworkLayer : public HttpTransactionFactory { static HttpTransactionFactory* CreateFactory(HttpNetworkSession* session); // HttpTransactionFactory methods: - virtual HttpTransaction* CreateTransaction(); + virtual int CreateTransaction(scoped_ptr<HttpTransaction>* trans); virtual HttpCache* GetCache(); virtual void Suspend(bool suspend); diff --git a/net/http/http_network_layer_unittest.cc b/net/http/http_network_layer_unittest.cc index 73d83ec..dcc9405 100644 --- a/net/http/http_network_layer_unittest.cc +++ b/net/http/http_network_layer_unittest.cc @@ -19,7 +19,10 @@ TEST_F(HttpNetworkLayerTest, CreateAndDestroy) { NULL, new net::MockHostResolver, net::ProxyService::CreateNull(), new net::SSLConfigServiceDefaults); - scoped_ptr<net::HttpTransaction> trans(factory.CreateTransaction()); + scoped_ptr<net::HttpTransaction> trans; + int rv = factory.CreateTransaction(&trans); + EXPECT_EQ(net::OK, rv); + EXPECT_TRUE(trans.get() != NULL); } TEST_F(HttpNetworkLayerTest, Suspend) { @@ -27,17 +30,23 @@ TEST_F(HttpNetworkLayerTest, Suspend) { NULL, new net::MockHostResolver, net::ProxyService::CreateNull(), new net::SSLConfigServiceDefaults); - scoped_ptr<net::HttpTransaction> trans(factory.CreateTransaction()); + scoped_ptr<net::HttpTransaction> trans; + int rv = factory.CreateTransaction(&trans); + EXPECT_EQ(net::OK, rv); + trans.reset(); factory.Suspend(true); - trans.reset(factory.CreateTransaction()); + rv = factory.CreateTransaction(&trans); + EXPECT_EQ(net::ERR_NETWORK_IO_SUSPENDED, rv); + ASSERT_TRUE(trans == NULL); factory.Suspend(false); - trans.reset(factory.CreateTransaction()); + rv = factory.CreateTransaction(&trans); + EXPECT_EQ(net::OK, rv); } TEST_F(HttpNetworkLayerTest, GET) { @@ -62,7 +71,9 @@ TEST_F(HttpNetworkLayerTest, GET) { TestCompletionCallback callback; - scoped_ptr<net::HttpTransaction> trans(factory.CreateTransaction()); + scoped_ptr<net::HttpTransaction> trans; + int rv = factory.CreateTransaction(&trans); + EXPECT_EQ(net::OK, rv); net::HttpRequestInfo request_info; request_info.url = GURL("http://www.google.com/"); @@ -70,7 +81,7 @@ TEST_F(HttpNetworkLayerTest, GET) { request_info.user_agent = "Foo/1.0"; request_info.load_flags = net::LOAD_NORMAL; - int rv = trans->Start(&request_info, &callback, NULL); + rv = trans->Start(&request_info, &callback, NULL); if (rv == net::ERR_IO_PENDING) rv = callback.WaitForResult(); ASSERT_EQ(net::OK, rv); diff --git a/net/http/http_transaction_factory.h b/net/http/http_transaction_factory.h index 523270a..90d36e9 100644 --- a/net/http/http_transaction_factory.h +++ b/net/http/http_transaction_factory.h @@ -5,6 +5,8 @@ #ifndef NET_HTTP_HTTP_TRANSACTION_FACTORY_H__ #define NET_HTTP_HTTP_TRANSACTION_FACTORY_H__ +#include "base/scoped_ptr.h" + namespace net { class HttpCache; @@ -15,8 +17,9 @@ class HttpTransactionFactory { public: virtual ~HttpTransactionFactory() {} - // Creates a HttpTransaction object. - virtual HttpTransaction* CreateTransaction() = 0; + // Creates a HttpTransaction object. On success, saves the new + // transaction to |*trans| and returns OK. + virtual int CreateTransaction(scoped_ptr<HttpTransaction>* trans) = 0; // Returns the associated cache if any (may be NULL). virtual HttpCache* GetCache() = 0; diff --git a/net/http/http_transaction_unittest.h b/net/http/http_transaction_unittest.h index d2e2115..ab27b27 100644 --- a/net/http/http_transaction_unittest.h +++ b/net/http/http_transaction_unittest.h @@ -104,8 +104,10 @@ class TestTransactionConsumer : public CallbackRunner< Tuple1<int> > { public: explicit TestTransactionConsumer(net::HttpTransactionFactory* factory) : state_(IDLE), - trans_(factory->CreateTransaction()), + trans_(NULL), error_(net::OK) { + // Disregard the error code. + factory->CreateTransaction(&trans_); ++quit_counter_; } @@ -310,9 +312,10 @@ class MockNetworkLayer : public net::HttpTransactionFactory { MockNetworkLayer() : transaction_count_(0) { } - virtual net::HttpTransaction* CreateTransaction() { + virtual int CreateTransaction(scoped_ptr<net::HttpTransaction>* trans) { transaction_count_++; - return new MockNetworkTransaction(); + trans->reset(new MockNetworkTransaction()); + return net::OK; } virtual net::HttpCache* GetCache() { |