Fix chrome upload with content uri

net/base/upload_file_element_reader.cc

 // Copyright (c) 2012 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.
 
 #include "net/base/upload_file_element_reader.h"
 
 #include "base/bind.h"
 #include "base/file_util.h"
 #include "base/location.h"
 #include "base/task_runner_util.h"
 #include "net/base/file_stream.h"
 #include "net/base/io_buffer.h"
 #include "net/base/net_errors.h"
 
+#if defined(OS_ANDROID)
+#include "net/android/content_uri_utils.h"
+#endif
+
 namespace net {
 
 namespace {
 
 // In tests, this value is used to override the return value of
 // UploadFileElementReader::GetContentLength() when set to non-zero.
 uint64 overriding_content_length = 0;
 
 // This function is used to implement Init().
 template<typename FileStreamDeleter>
@@ skipping 13 matching lines @@
     file_stream.reset();
   } else if (range_offset) {
     rv = file_stream->SeekSync(FROM_BEGIN, range_offset);
     if (rv < 0) {
       DLOG(WARNING) << "Failed to seek \"" << path.value()
                     << "\" to offset: " << range_offset << " (" << rv << ")";
       file_stream.reset();
     }
   }
 
+#if defined(OS_ANDROID)
+  int64 length = std::max(0ll, GetContentUrlLengthSync(path));

[kinuko, 2013/10/31 06:10:51]

Can this part be factored out as a separate static method (e.g.
GetLengthFromPath(path, &length)) so that we don't need to have #if..else..endif
in the main code flow?

[joth, 2013/10/31 06:42:39]

+1

Maybe even, bundling this logic into file_util::GetFileSize() would mean no
special content:// logic here at all. 


Was a design goal of this patch explicitly not to add generic content:// support
in file_util? else I could easily imagine having a generic
file_util::OpenFileDescriptor(FilePath) method and the content_uri_utils call
would just happen inside that if appropriate.

[qinmin, 2013/11/05 01:41:31]

Done. GetFileInfo() should work for content url now. GetFileSize() calls
GetFileInfo(), so it will work too.
On 2013/10/31 06:42:39, joth wrote:

+  if (file_stream.get() && length > 0 &&
+      range_offset < static_cast<uint64>(length)) {

[mmenke, 2013/10/31 15:07:06]

This class is being modified to handle errors differently, see
https://codereview.chromium.org/39203004/diff/670001/net/base/upload_file_ele....
 If we keep the split logic here, whichever lands second will have to make this
return an error on the fail-to-get-length case.

I'm a fan of joth's idea to mover the logic into file_util, if we can, though.

[qinmin, 2013/11/05 01:41:31]

Done.

+    // Compensate for the offset.
+    length = std::min(length - range_offset, range_length);
+  }
+#else
   int64 length = 0;
   if (file_stream.get() &&
       file_util::GetFileSize(path, &length) &&
       range_offset < static_cast<uint64>(length)) {
     // Compensate for the offset.
     length = std::min(length - range_offset, range_length);
   }
+#endif
   *out_content_length = length;
   out_file_stream->reset(file_stream.release());
 
   // If the underlying file has been changed and the expected file modification
   // time is set, treat it as error. Note that the expected modification time
   // from WebKit is based on time_t precision. So we have to convert both to
   // time_t to compare. This check is used for sliced files.
   if (!expected_modification_time.is_null()) {
     base::PlatformFileInfo info;
     if (file_util::GetFileInfo(path, &info) &&
@@ skipping 216 matching lines @@
   const int result = ReadInternal(buf, buf_length, BytesRemaining(),
                                   file_stream_.get());
   if (result > 0) {
     DCHECK_GE(bytes_remaining_, static_cast<uint64>(result));
     bytes_remaining_ -= result;
   }
   return result;
 }
 
 }  // namespace net