net: Change argument of URLRequest::set_upload from UploadData to UploadDataStream

net/base/upload_bytes_element_reader.h

 // 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.
 
 #ifndef NET_BASE_UPLOAD_BYTES_ELEMENT_READER_H_
 #define NET_BASE_UPLOAD_BYTES_ELEMENT_READER_H_
 
+#include <vector>
+
 #include "base/compiler_specific.h"
 #include "net/base/upload_element_reader.h"
 
 namespace net {
 
 // An UploadElementReader implementation for bytes.
-class NET_EXPORT_PRIVATE UploadBytesElementReader : public UploadElementReader {
+class NET_EXPORT UploadBytesElementReader : public UploadElementReader {

[kinuko, 2012/12/12 01:19:17]

Can we add a comment to note that this doesn't copy the given bytes or take the
ownership of the data? (The existence of ...Owned... version implies it, but
it's not obvious just by looking at this class's interface)

[hashimoto, 2012/12/12 10:29:07]

Done.

  public:
   UploadBytesElementReader(const char* bytes, int length);
   virtual ~UploadBytesElementReader();
 
   const char* bytes() const { return bytes_; }
   int length() const { return length_; }
 
   // UploadElementReader overrides:
   virtual const UploadBytesElementReader* AsBytesReader() const OVERRIDE;
   virtual int Init(const CompletionCallback& callback) OVERRIDE;
   virtual int InitSync() OVERRIDE;
   virtual uint64 GetContentLength() const OVERRIDE;
   virtual uint64 BytesRemaining() const OVERRIDE;
   virtual bool IsInMemory() const OVERRIDE;
   virtual int Read(IOBuffer* buf,
                    int buf_length,
                    const CompletionCallback& callback) OVERRIDE;
   virtual int ReadSync(IOBuffer* buf, int buf_length) OVERRIDE;
 
  private:
   const char* const bytes_;
   const int length_;
   int offset_;
 
   DISALLOW_COPY_AND_ASSIGN(UploadBytesElementReader);
 };
 
+// A subclass of UplodBytesElementReader which owns the data given as a vector.
+class NET_EXPORT UploadOwnedBytesElementReader
+    : public UploadBytesElementReader {
+ public:
+  UploadOwnedBytesElementReader(std::vector<char>* data);

[mmenke, 2012/12/11 18:53:21]

Think it's worth mentioning that we clear |data|.  It's implied by the fact we
take it as a pointer, but prefer to be explicit.

[mmenke, 2012/12/11 18:53:21]

Speaking of which...this function should also be explicit.

[hashimoto, 2012/12/12 10:29:07]

Oops, it seems presubmit check does not help for missing 'explicit' in src/net,
while it does in src/chrome.

[hashimoto, 2012/12/12 10:29:07]

Done.

+  virtual ~UploadOwnedBytesElementReader();
+
+ private:
+  std::vector<char> data_;

[mmenke, 2012/12/11 18:53:21]

May make life easier to make UploadOwnedBytesElementReader take a string rather
than a vector, as we seem to be doing a lot of converting between the two.

Or could just give it a constructor that takes a string (And optionally switch
to storing a string as well, but keeping the vector constructor).

[hashimoto, 2012/12/12 10:29:07]

Since yet there are only two places which uses UploadOwnedBytesReader with
string (chrome_fraudlent_certificate_reporter.cc and download_manager_impl.cc),
can we go with vector<char> now?
I think vector<char> is little bit purer than std::string as a way to own bytes.
Having a string ctor while keeping the vector does not work because:

UploadOwnedBytesElementReader::UploadOwnedBytesElementReader(
    const std::string& string)
  : UploadBytesElementReader(string.data(), string.length()),
    data_(string.begin(), string.end()) {
 // &data_[0] is different from this->bytes() now!
}

How about having UploadOwnedBytesElementReader::CreateFromString?

[mmenke, 2012/12/12 19:26:41]

I admit to a bit of a bias against vectors used in this way because I prefer
data.data() to "&(*data)[0]".  And passing strings around is a bit prettier,
too.
I think that's fine.

[kinuko, 2012/12/13 00:07:55]

As for the use of &(*data)[0] I think vector_as_array (defined in stl_util.h) is
more preferable? (By the way could we get an empty data here?)

[mmenke, 2012/12/13 00:43:47]

Whether we could get empty data or not depends on the callers.  Seems best to
support it.  I wasn't aware of vector_as_array - seems like a good idea, to me.

[hashimoto, 2012/12/13 03:58:47]

Good point, it seems cleaner to support empty data, thanks.
Replaced &(*data)[0] with vector_as_array(data).

+
+  DISALLOW_COPY_AND_ASSIGN(UploadOwnedBytesElementReader);
+};
+
 }  // namespace net
 
 #endif  // NET_BASE_UPLOAD_BYTES_ELEMENT_READER_H_