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

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() {