disk_cache: Make GetAvailableRange reasonable to use safely.

Before this change, it took an out pointer parameter, which is really messy to manage destination lifetime for in a net-style method. (See the bug for a couple of different ways it could go wrong)

This change it to return (and pass to callbacks) the result by value instead, which avoids the complications entirely.


Bug: 1208738

Change-Id: I01f07ac693bb25266f91c3cd1ec4d69c023a1891
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2983259
Commit-Queue: Maksim Orlovich <morlovich@chromium.org>
Reviewed-by: Ben Kelly <wanderview@chromium.org>
Reviewed-by: Matthew Denton <mpdenton@chromium.org>
Reviewed-by: Shivani Sharma <shivanisha@chromium.org>
Cr-Commit-Position: refs/heads/master@{#898234}

diff --git a/net/http/partial_data.cc b/net/http/partial_data.cc
index 79f71ab..3f76d09 100644
--- a/net/http/partial_data.cc
+++ b/net/http/partial_data.cc

@@ -107,22 +107,18 @@
 
   if (sparse_entry_) {
     DCHECK(callback_.is_null());
-    // |start| will be deleted later in this method if GetAvailableRange()
-    // returns synchronously, or by GetAvailableRangeCompleted() if it returns
-    // asynchronously.
-    int64_t* start = new int64_t;
-    CompletionOnceCallback cb =
-        base::BindOnce(&PartialData::GetAvailableRangeCompleted,
-                       weak_factory_.GetWeakPtr(), start);
-    cached_min_len_ = entry->GetAvailableRange(current_range_start_, len, start,
-                                               std::move(cb));
+    disk_cache::RangeResultCallback cb = base::BindOnce(
+        &PartialData::GetAvailableRangeCompleted, weak_factory_.GetWeakPtr());
+    disk_cache::RangeResult range =
+        entry->GetAvailableRange(current_range_start_, len, std::move(cb));
 
+    cached_min_len_ =
+        range.net_error == OK ? range.available_len : range.net_error;
     if (cached_min_len_ == ERR_IO_PENDING) {
       callback_ = std::move(callback);
       return ERR_IO_PENDING;
     } else {
-      cached_start_ = *start;
-      delete start;
+      cached_start_ = range.start;
     }
   } else if (!truncated_) {
     if (byte_range_.HasFirstBytePosition() &&
@@ -456,17 +452,20 @@
   return static_cast<int32_t>(range_len);
 }
 
-void PartialData::GetAvailableRangeCompleted(int64_t* start, int result) {
+void PartialData::GetAvailableRangeCompleted(
+    const disk_cache::RangeResult& result) {
   DCHECK(!callback_.is_null());
-  DCHECK_NE(ERR_IO_PENDING, result);
+  DCHECK_NE(ERR_IO_PENDING, result.net_error);
 
-  cached_start_ = *start;
-  delete start;
-  cached_min_len_ = result;
-  if (result >= 0)
-    result = 1;  // Return success, go ahead and validate the entry.
+  int len_or_error =
+      result.net_error == OK ? result.available_len : result.net_error;
+  cached_start_ = result.start;
+  cached_min_len_ = len_or_error;
 
-  std::move(callback_).Run(result);
+  // ShouldValidateCache has an unusual convention where 0 denotes EOF,
+  // so convert end of range to success (since there may be things that need
+  // fetching from network or other ranges).
+  std::move(callback_).Run(len_or_error >= 0 ? 1 : len_or_error);
 }
 
 }  // namespace net