[net] Better StopCaching() handling for HttpCache::Transaction.

net/http/partial_data.cc

Index: net/http/partial_data.cc
diff --git a/net/http/partial_data.cc b/net/http/partial_data.cc
index 8e1eb0ab84658cc4f1e1f06c1abc289cfd19b7d7..5795ad03980336302915dbc6733d0d3dd9178f5f 100644
--- a/net/http/partial_data.cc
+++ b/net/http/partial_data.cc
@@ -4,6 +4,8 @@
 
 #include "net/http/partial_data.h"
 
+#include <limits>
+
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/format_macros.h"
@@ -27,6 +29,9 @@ const int kDataStream = 1;
 
 }  // namespace
 
+// static
+const int64 PartialData::kUnbounded = std::numeric_limits<int64>::max();
+
 PartialData::PartialData()
     : current_range_start_(0),
       current_range_end_(0),
@@ -94,7 +99,7 @@ int PartialData::ShouldValidateCache(disk_cache::Entry* entry,
   DCHECK_GE(current_range_start_, 0);
 
   // Scan the disk cache for the first cached portion within this range.
-  int len = GetNextRangeLen();
+  int64 len = GetNextRangeLen();
   if (!len)
     return 0;
 
@@ -103,13 +108,17 @@ int PartialData::ShouldValidateCache(disk_cache::Entry* entry,
   if (sparse_entry_) {
     DCHECK(callback_.is_null());
     int64* start = new int64;
+    // TODO(asanka): Use the full len when int64s are plumbed all the way
+    // through.
+    int cached_len = std::min<int64>(std::numeric_limits<int32>::max(), len);
+
     // This callback now owns "start". We make sure to keep it
     // in a local variable since we want to use it later.
     CompletionCallback cb =
         base::Bind(&PartialData::GetAvailableRangeCompleted,
                    weak_factory_.GetWeakPtr(), base::Owned(start));
     cached_min_len_ =
-        entry->GetAvailableRange(current_range_start_, len, start, cb);
+        entry->GetAvailableRange(current_range_start_, cached_len, start, cb);
 
     if (cached_min_len_ == ERR_IO_PENDING) {
       callback_ = callback;
@@ -140,17 +149,34 @@ void PartialData::PrepareCacheValidation(disk_cache::Entry* entry,
   DCHECK_GE(current_range_start_, 0);
   DCHECK_GE(cached_min_len_, 0);
 
-  int len = GetNextRangeLen();
+  int64 len = GetNextRangeLen();
+  // PrepareCacheValidation shouldn't have been called if ShouldValidateCache()
+  // returned 0.
   DCHECK_NE(0, len);
   range_present_ = false;
 
   headers->CopyFrom(extra_headers_);
 
+  // Do we need to request the remainder from the network? If so just issue an
+  // open ended range request.
+  if (len == kUnbounded) {
+    DCHECK_EQ(0, cached_min_len_);  // Nothing else should be stored. All cached
+                                    // ranges are finite, so we don't expect len
+                                    // to be kUnbounded.
+    cached_start_ = 0;
+    current_range_end_ = -1;
+    final_range_ = true;
+    headers->SetHeader(
+        HttpRequestHeaders::kRange,
+        HttpByteRange::RightUnbounded(current_range_start_).GetHeaderValue());
+    return;
+  }
+
   if (!cached_min_len_) {
     // We don't have anything else stored.
     final_range_ = true;
     cached_start_ =
-        byte_range_.HasLastBytePosition() ? current_range_start_  + len : 0;
+        byte_range_.HasLastBytePosition() ? current_range_start_ + len : 0;
   }
 
   if (current_range_start_ == cached_start_) {
@@ -365,7 +391,7 @@ int PartialData::CacheRead(disk_cache::Entry* entry,
                            IOBuffer* data,
                            int data_len,
                            const CompletionCallback& callback) {
-  int read_len = std::min(data_len, cached_min_len_);
+  int read_len = std::min<int64>(data_len, cached_min_len_);
   if (!read_len)
     return 0;
 
@@ -414,14 +440,10 @@ void PartialData::OnNetworkReadCompleted(int result) {
     current_range_start_ += result;
 }
 
-int PartialData::GetNextRangeLen() {
-  int64 range_len =
-      byte_range_.HasLastBytePosition() ?
-      byte_range_.last_byte_position() - current_range_start_ + 1 :
-      kint32max;
-  if (range_len > kint32max)
-    range_len = kint32max;
-  return static_cast<int32>(range_len);
+int64 PartialData::GetNextRangeLen() {
+  return byte_range_.HasLastBytePosition()

[rvargas (doing something else), 2015/07/27 22:25:12]

It seems like we should eliminate this method.

[asanka, 2015/08/18 22:46:59]

New patchset adds another call site. Let me know if you still this we should get
rid of it.

[rvargas (doing something else), 2015/08/19 23:46:37]

The reason was that the code at the call sites would be clearer if this is
inlined+simplified.

That may not be true for the new call site, but at the same time, what does it
mean to call SkipCacheForRemainder if !byte_range_.HasLastBytePos()?

I think I still lean to get rid of this method even if we add a little code
duplication.

[asanka, 2015/09/04 19:09:03]

SG. Removed. I think it also makes the handling of the kUnbounded case much more
explicit.

+             ? byte_range_.last_byte_position() - current_range_start_ + 1
+             : kUnbounded;
 }
 
 void PartialData::GetAvailableRangeCompleted(int64* start, int result) {