Change FindNextSliceToDownload() into FindSlicesToDownload()

content/browser/download/parallel_download_utils.h

 // Copyright 2017 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef CONTENT_BROWSER_DOWNLOAD_PARALLEL_DOWNLOAD_UTILS_H_
 #define CONTENT_BROWSER_DOWNLOAD_PARALLEL_DOWNLOAD_UTILS_H_
 
 #include <vector>
 
 #include "content/common/content_export.h"
 #include "content/public/browser/download_item.h"
 
 namespace content {
 
 // Find the first gap in an array of received slices and return it as the next

[xingliu, 2017/03/03 18:44:15]

Comment for the function is out-dated.

This function looks for new slices to download. Do we directly write these to
db?

If not, then we still need to maintain some kind of mapping from the  slices for
the database and slices that are downloading, or have a function to transform
the downloading slices into slices for db.

This is kind of weird though since we can just use the slice info for the
database, update them in memory and at some point write them back to db.

[qinmin, 2017/03/03 19:32:35]

Fixed the comment. 
The return value is only used for the caller to make url request, and should not
be written to db. History db will finally get the new slice information when
DownloadFile starts receiving data from those url requests, this happens when
DownloadFileImpl::SendUpdate() is called. 

The return type of ReceivedSlice is bad. We should use a type FileSlice or
something. The ReceivedSlice is only for slices that are written to the disk.
But here the return type doesn't mean that those slices are already received.
However, introducing a almost identical class is very strange, so I just use
ReceivedSlice instead.

DownloadSliceInfo is history/, cannot be used in content/.

On 2017/03/03 18:44:15, xingliu wrote:

[xingliu, 2017/03/03 21:37:44]

sgtm on new download use scenario.

Question for resumption from browser restart:

If this function is also called on resumption(in BuildParallelRequests), we
build some requests that offset may be different from the slice info read from
the db. Then we download some bytes, so do we still need to keep a map of offset
of new request and the offset of the slice info from db here?

[qinmin, 2017/03/03 22:20:10]

Once all the parallel requests are kicked off, this class shouldn't really care.
This is because the new request can fail, so those new offsets are not final.
The DownloadFileImpl, on the otherhand,  will be responsible for keeping track
of the new slices, and also the old one.

DownloadFileImpl can call this class once all the stream finishes, and this
class can then decide whether it wants to issue additional requests by checking
the received_slices_ from DownloadItem(assuming downloadFileImpl already
reported all the slices to DownloadItem).

 // slice to download. If not found, return a slice that is to the end of all
 // slices. |received_slices| must be ordered by offsets.
-CONTENT_EXPORT DownloadItem::ReceivedSlice FindNextSliceToDownload(
+CONTENT_EXPORT std::vector<DownloadItem::ReceivedSlice> FindSlicesToDownload(
     const std::vector<DownloadItem::ReceivedSlice>& received_slices);
 
 }  //  namespace content
 
 #endif  // CONTENT_BROWSER_DOWNLOAD_PARALLEL_DOWNLOAD_UTILS_H_