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

net/base/upload_bytes_element_reader.h

Index: net/base/upload_bytes_element_reader.h
diff --git a/net/base/upload_bytes_element_reader.h b/net/base/upload_bytes_element_reader.h
index afc9560e54aec448732fd3a54db87b77ccda8b42..451c3c5fcc661c968c66b8933e883d2aa56322bb 100644
--- a/net/base/upload_bytes_element_reader.h
+++ b/net/base/upload_bytes_element_reader.h
@@ -5,13 +5,15 @@
 #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();
@@ -39,6 +41,19 @@ class NET_EXPORT_PRIVATE UploadBytesElementReader : public UploadElementReader {
   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_