Http cache: Enable experimental support for byte range requests.

Requires --enable-byte-range-support

BUG=12258
TEST=covered by unit tests.

Review URL: http://codereview.chromium.org/173173

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@24015 0039d316-1c4b-4281-b951-d872f2087c98

6 files changed, 73 insertions, 40 deletions

diff --git a/chrome/browser/net/chrome_url_request_context.cc b/chrome/browser/net/chrome_url_request_context.cc
index 5bb921b..da77fcf 100644
--- a/chrome/browser/net/chrome_url_request_context.cc
+++ b/chrome/browser/net/chrome_url_request_context.cc
@@ -125,6 +125,9 @@ ChromeURLRequestContext* ChromeURLRequestContext::CreateOriginal(
                          context->ssl_config_service_,
                          disk_cache_path.ToWStringHack(), cache_size);
 
+  if (command_line.HasSwitch(switches::kEnableByteRangeSupport))
+    cache->set_enable_range_support(true);
+
   bool record_mode = chrome::kRecordModeEnabled &&
                      command_line.HasSwitch(switches::kRecordMode);
   bool playback_mode = command_line.HasSwitch(switches::kPlaybackMode);
@@ -207,10 +210,15 @@ ChromeURLRequestContext* ChromeURLRequestContext::CreateOffTheRecord(
   context->proxy_service_ =
       profile->GetOriginalProfile()->GetRequestContext()->proxy_service();
 
-  context->http_transaction_factory_ =
+  net::HttpCache* cache =
       new net::HttpCache(context->host_resolver_, context->proxy_service_,
                          context->ssl_config_service_, 0);
   context->cookie_store_ = new net::CookieMonster;
+  context->http_transaction_factory_ = cache;
+
+  if (CommandLine::ForCurrentProcess()->HasSwitch(
+          switches::kEnableByteRangeSupport))
+    cache->set_enable_range_support(true);
 
   // The kWininetFtp switch is Windows specific because we have two FTP
   // implementations on Windows.
@@ -279,6 +287,10 @@ ChromeURLRequestContext* ChromeURLRequestContext::CreateRequestContextForMedia(
                                disk_cache_path.ToWStringHack(), cache_size);
   }
 
+  if (CommandLine::ForCurrentProcess()->HasSwitch(
+          switches::kEnableByteRangeSupport))
+    cache->set_enable_range_support(true);
+
   cache->set_type(net::MEDIA_CACHE);
   context->http_transaction_factory_ = cache;
   return context;
diff --git a/chrome/common/chrome_switches.cc b/chrome/common/chrome_switches.cc
index c62c4db..a232be0 100644
--- a/chrome/common/chrome_switches.cc
+++ b/chrome/common/chrome_switches.cc
@@ -591,4 +591,7 @@ const wchar_t kSyncServiceURL[]  = L"sync-url";
 const wchar_t kCookiePipe[] = L"cookie-pipe";
 #endif
 
+// Enable experimental support for cached byte-ranges.
+const wchar_t kEnableByteRangeSupport[] = L"enable-byte-range-support";
+
 }  // namespace switches
diff --git a/chrome/common/chrome_switches.h b/chrome/common/chrome_switches.h
index e4dc936..fce1b69 100644
--- a/chrome/common/chrome_switches.h
+++ b/chrome/common/chrome_switches.h
@@ -239,6 +239,8 @@ extern const wchar_t kSyncServiceURL[];
 extern const wchar_t kCookiePipe[];
 #endif
 
+extern const wchar_t kEnableByteRangeSupport[];
+
 }  // namespace switches
 
 #endif  // CHROME_COMMON_CHROME_SWITCHES_H_
diff --git a/net/http/http_cache.cc b/net/http/http_cache.cc
index 4de8080..303f9ba 100644
--- a/net/http/http_cache.cc
+++ b/net/http/http_cache.cc
@@ -13,6 +13,8 @@
 #include <unistd.h>
 #endif
 
+#include "base/base_switches.h"
+#include "base/command_line.h"
 #include "base/message_loop.h"
 #include "base/pickle.h"
 #include "base/ref_counted.h"
@@ -34,9 +36,6 @@
 
 using base::Time;
 
-// Uncomment this to enable experimental byte-range support.
-// #define ENABLE_RANGE_SUPPORT
-
 namespace net {
 
 // disk cache entry data indices.
@@ -170,7 +169,7 @@ HttpCache::ActiveEntry::~ActiveEntry() {
 class HttpCache::Transaction
     : public HttpTransaction, public RevocableStore::Revocable {
  public:
-  explicit Transaction(HttpCache* cache)
+  Transaction(HttpCache* cache, bool enable_range_support)
       : RevocableStore::Revocable(&cache->transactions_),
         request_(NULL),
         cache_(cache),
@@ -180,6 +179,7 @@ class HttpCache::Transaction
         mode_(NONE),
         reading_(false),
         invalid_range_(false),
+        enable_range_support_(enable_range_support),
         read_offset_(0),
         effective_load_flags_(0),
         final_upload_progress_(0),
@@ -386,6 +386,7 @@ class HttpCache::Transaction
   Mode mode_;
   bool reading_;  // We are already reading.
   bool invalid_range_;  // We may bypass the cache for this request.
+  bool enable_range_support_;
   scoped_refptr<IOBuffer> read_buf_;
   int read_buf_len_;
   int read_offset_;
@@ -777,12 +778,12 @@ void HttpCache::Transaction::SetRequest(LoadLog* load_log,
       new_extra_headers.append(it.name_begin(), it.values_end());
       new_extra_headers.append("\r\n");
     } else {
-#ifdef ENABLE_RANGE_SUPPORT
-      range_found = true;
-#else
-      effective_load_flags_ |= LOAD_DISABLE_CACHE;
-      continue;
-#endif
+      if (enable_range_support_) {
+        range_found = true;
+      } else {
+        effective_load_flags_ |= LOAD_DISABLE_CACHE;
+        continue;
+      }
     }
     for (size_t i = 0; i < ARRAYSIZE_UNSAFE(kSpecialHeaders); ++i) {
       if (HeaderMatches(it, kSpecialHeaders[i].search)) {
@@ -907,9 +908,8 @@ int HttpCache::Transaction::BeginPartialCacheValidation() {
   if (response_.headers->response_code() != 206 && !partial_.get())
     return BeginCacheValidation();
 
-#if !defined(ENABLE_RANGE_SUPPORT)
-  return BeginCacheValidation();
-#endif
+  if (!enable_range_support_)
+    return BeginCacheValidation();
 
   bool byte_range_requested = partial_.get() != NULL;
   if (!byte_range_requested) {
@@ -1070,11 +1070,10 @@ bool HttpCache::Transaction::RequiresValidation() {
 bool HttpCache::Transaction::ConditionalizeRequest() {
   DCHECK(response_.headers);
 
-#if !defined(ENABLE_RANGE_SUPPORT)
-  // This only makes sense for cached 200 responses.
-  if (response_.headers->response_code() != 200)
+  if (!enable_range_support_ && response_.headers->response_code() != 200) {
+    // This only makes sense for cached 200 responses.
     return false;
-#endif
+  }
 
   // This only makes sense for cached 200 or 206 responses.
   if (response_.headers->response_code() != 200 &&
@@ -1147,12 +1146,8 @@ bool HttpCache::Transaction::ConditionalizeRequest() {
 // course, maybe we already returned the headers.
 bool HttpCache::Transaction::ValidatePartialResponse(
     const HttpResponseHeaders* headers) {
-    int response_code = headers->response_code();
-#ifdef ENABLE_RANGE_SUPPORT
-  bool partial_content = response_code == 206;
-#else
-  bool partial_content = false;
-#endif
+  int response_code = headers->response_code();
+  bool partial_content = enable_range_support_ ? response_code == 206 : false;
 
   if (invalid_range_) {
     // We gave up trying to match this request with the stored data. If the
@@ -1585,6 +1580,7 @@ HttpCache::HttpCache(HttpTransactionFactory* network_layer,
       ALLOW_THIS_IN_INITIALIZER_LIST(task_factory_(this)),
       in_memory_cache_(false),
       deleted_(false),
+      enable_range_support_(false),
       cache_size_(0) {
 }
 
@@ -1625,7 +1621,7 @@ HttpTransaction* HttpCache::CreateTransaction() {
       disk_cache_dir_.clear();  // Reclaim memory.
     }
   }
-  return new HttpCache::Transaction(this);
+  return new HttpCache::Transaction(this, enable_range_support_);
 }
 
 HttpCache* HttpCache::GetCache() {
diff --git a/net/http/http_cache.h b/net/http/http_cache.h
index 19c4331..af5c55e 100644
--- a/net/http/http_cache.h
+++ b/net/http/http_cache.h
@@ -116,6 +116,10 @@ class HttpCache : public HttpTransactionFactory {
   // Close All Idle Sockets.  This is for debugging.
   void CloseIdleConnections();
 
+  void set_enable_range_support(bool value) {
+    enable_range_support_ = value;
+  }
+
  private:
 
   // Types --------------------------------------------------------------------
@@ -188,6 +192,7 @@ class HttpCache : public HttpTransactionFactory {
 
   bool in_memory_cache_;
   bool deleted_;  // TODO(rvargas): remove this member. See bug 9952.
+  bool enable_range_support_;
   int cache_size_;
 
   typedef base::hash_map<std::string, int> PlaybackCacheMap;
diff --git a/net/http/http_cache_unittest.cc b/net/http/http_cache_unittest.cc
index 14299b0..90b0774 100644
--- a/net/http/http_cache_unittest.cc
+++ b/net/http/http_cache_unittest.cc
@@ -1639,6 +1639,7 @@ TEST(HttpCache, SimplePOST_LoadOnlyFromCache_Hit) {
 
 TEST(HttpCache, RangeGET_SkipsCache) {
   MockHttpCache cache;
+  cache.http_cache()->set_enable_range_support(true);
 
   // Test that we skip the cache for range GET requests.  Eventually, we will
   // want to cache these, but we'll still have cases where skipping the cache
@@ -1670,6 +1671,7 @@ TEST(HttpCache, RangeGET_SkipsCache) {
 // Tests that receiving 206 for a regular request is handled correctly.
 TEST(HttpCache, GET_Crazy206) {
   MockHttpCache cache;
+  cache.http_cache()->set_enable_range_support(true);
 
   // Write to the cache.
   MockTransaction transaction(kRangeGET_TransactionOK);
@@ -1686,15 +1688,16 @@ TEST(HttpCache, GET_Crazy206) {
   RunTransactionTest(cache.http_cache(), transaction);
 
   EXPECT_EQ(2, cache.network_layer()->transaction_count());
-  EXPECT_EQ(1, cache.disk_cache()->open_count());
-  EXPECT_EQ(1, cache.disk_cache()->create_count());
+  EXPECT_EQ(0, cache.disk_cache()->open_count());
+  EXPECT_EQ(2, cache.disk_cache()->create_count());
   RemoveMockTransaction(&transaction);
 }
 
 // Tests that we can cache range requests and fetch random blocks from the
 // cache and the network.
-TEST(HttpCache, DISABLED_RangeGET_OK) {
+TEST(HttpCache, RangeGET_OK) {
   MockHttpCache cache;
+  cache.http_cache()->set_enable_range_support(true);
   AddMockTransaction(&kRangeGET_TransactionOK);
   std::string headers;
 
@@ -1747,8 +1750,9 @@ TEST(HttpCache, DISABLED_RangeGET_OK) {
 }
 
 // Tests that we deal with 304s for range requests.
-TEST(HttpCache, DISABLED_RangeGET_304) {
+TEST(HttpCache, RangeGET_304) {
   MockHttpCache cache;
+  cache.http_cache()->set_enable_range_support(true);
   AddMockTransaction(&kRangeGET_TransactionOK);
   std::string headers;
 
@@ -1776,8 +1780,9 @@ TEST(HttpCache, DISABLED_RangeGET_304) {
 }
 
 // Tests that we deal with 206s when revalidating range requests.
-TEST(HttpCache, DISABLED_RangeGET_ModifiedResult) {
+TEST(HttpCache, RangeGET_ModifiedResult) {
   MockHttpCache cache;
+  cache.http_cache()->set_enable_range_support(true);
   AddMockTransaction(&kRangeGET_TransactionOK);
   std::string headers;
 
@@ -1812,8 +1817,9 @@ TEST(HttpCache, DISABLED_RangeGET_ModifiedResult) {
 
 // Tests that we can cache range requests when the start or end is unknown.
 // We start with one suffix request, followed by a request from a given point.
-TEST(HttpCache, DISABLED_UnknownRangeGET_1) {
+TEST(HttpCache, UnknownRangeGET_1) {
   MockHttpCache cache;
+  cache.http_cache()->set_enable_range_support(true);
   AddMockTransaction(&kRangeGET_TransactionOK);
   std::string headers;
 
@@ -1847,8 +1853,9 @@ TEST(HttpCache, DISABLED_UnknownRangeGET_1) {
 // Tests that we can cache range requests when the start or end is unknown.
 // We start with one request from a given point, followed by a suffix request.
 // We'll also verify that synchronous cache responses work as intended.
-TEST(HttpCache, DISABLED_UnknownRangeGET_2) {
+TEST(HttpCache, UnknownRangeGET_2) {
   MockHttpCache cache;
+  cache.http_cache()->set_enable_range_support(true);
   std::string headers;
 
   MockTransaction transaction(kRangeGET_TransactionOK);
@@ -1884,8 +1891,9 @@ TEST(HttpCache, DISABLED_UnknownRangeGET_2) {
 
 // Tests that receiving Not Modified when asking for an open range doesn't mess
 // up things.
-TEST(HttpCache, DISABLED_UnknownRangeGET_304) {
+TEST(HttpCache, UnknownRangeGET_304) {
   MockHttpCache cache;
+  cache.http_cache()->set_enable_range_support(true);
   std::string headers;
 
   MockTransaction transaction(kRangeGET_TransactionOK);
@@ -1912,8 +1920,9 @@ TEST(HttpCache, DISABLED_UnknownRangeGET_304) {
 }
 
 // Tests that we can handle non-range requests when we have cached a range.
-TEST(HttpCache, DISABLED_GET_Previous206) {
+TEST(HttpCache, GET_Previous206) {
   MockHttpCache cache;
+  cache.http_cache()->set_enable_range_support(true);
   AddMockTransaction(&kRangeGET_TransactionOK);
   std::string headers;
 
@@ -1942,8 +1951,9 @@ TEST(HttpCache, DISABLED_GET_Previous206) {
 }
 
 // Tests that we can handle cached 206 responses that are not sparse.
-TEST(HttpCache, DISABLED_GET_Previous206_NotSparse) {
+TEST(HttpCache, GET_Previous206_NotSparse) {
   MockHttpCache cache;
+  cache.http_cache()->set_enable_range_support(true);
 
   // Create a disk cache entry that stores 206 headers while not being sparse.
   disk_cache::Entry* entry;
@@ -1983,8 +1993,9 @@ TEST(HttpCache, DISABLED_GET_Previous206_NotSparse) {
 
 // Tests that we can handle cached 206 responses that are not sparse. This time
 // we issue a range request and expect to receive a range.
-TEST(HttpCache, DISABLED_RangeGET_Previous206_NotSparse_2) {
+TEST(HttpCache, RangeGET_Previous206_NotSparse_2) {
   MockHttpCache cache;
+  cache.http_cache()->set_enable_range_support(true);
   AddMockTransaction(&kRangeGET_TransactionOK);
 
   // Create a disk cache entry that stores 206 headers while not being sparse.
@@ -2023,8 +2034,9 @@ TEST(HttpCache, DISABLED_RangeGET_Previous206_NotSparse_2) {
 }
 
 // Tests that we can handle range requests with cached 200 responses.
-TEST(HttpCache, DISABLED_RangeGET_Previous200) {
+TEST(HttpCache, RangeGET_Previous200) {
   MockHttpCache cache;
+  cache.http_cache()->set_enable_range_support(true);
 
   // Store the whole thing with status 200.
   MockTransaction transaction(kTypicalGET_Transaction);
@@ -2069,8 +2081,9 @@ TEST(HttpCache, DISABLED_RangeGET_Previous200) {
 }
 
 // Tests that we can handle a 200 response when dealing with sparse entries.
-TEST(HttpCache, DISABLED_RangeRequestResultsIn200) {
+TEST(HttpCache, RangeRequestResultsIn200) {
   MockHttpCache cache;
+  cache.http_cache()->set_enable_range_support(true);
   AddMockTransaction(&kRangeGET_TransactionOK);
   std::string headers;
 
@@ -2109,8 +2122,9 @@ TEST(HttpCache, DISABLED_RangeRequestResultsIn200) {
 
 // Tests that a range request that falls outside of the size that we know about
 // only deletes the entry if the resource has indeed changed.
-TEST(HttpCache, DISABLED_RangeGET_MoreThanCurrentSize) {
+TEST(HttpCache, RangeGET_MoreThanCurrentSize) {
   MockHttpCache cache;
+  cache.http_cache()->set_enable_range_support(true);
   AddMockTransaction(&kRangeGET_TransactionOK);
   std::string headers;
 
@@ -2142,8 +2156,9 @@ TEST(HttpCache, DISABLED_RangeGET_MoreThanCurrentSize) {
 
 #ifdef NDEBUG
 // This test hits a NOTREACHED so it is a release mode only test.
-TEST(HttpCache, DISABLED_RangeGET_OK_LoadOnlyFromCache) {
+TEST(HttpCache, RangeGET_OK_LoadOnlyFromCache) {
   MockHttpCache cache;
+  cache.http_cache()->set_enable_range_support(true);
   AddMockTransaction(&kRangeGET_TransactionOK);
 
   // Write to the cache (40-49).