net: Change type of UploadData::elements from std::vector to ScopedVe…

…ctor

Using std::vector to hold UploadElement is bad for two reasons:
 1. It results in a lot of unnecessary copy of uploaded data.
 2. Appending new chunks may result in invalidating the pointer held by UploadBytesElementReader.

BUG=160028
TEST=git try
TBR=ananta@chromium.org, tony@chromium.org

Review URL: https://chromiumcodereview.appspot.com/11275223

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@167611 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/extensions/api/web_request/web_request_api.cc

@@ -196,13 +196,13 @@ void ExtractRequestInfoBody(const net::URLRequest* request,
     keys::kRequestBodyRawKey
   };
 
-  const std::vector<net::UploadElement>* elements =
+  const ScopedVector<net::UploadElement>& elements =
       request->get_upload()->elements();
   bool some_succeeded = false;
   for (size_t i = 0; !some_succeeded && i < arraysize(presenters); ++i) {
-    std::vector<net::UploadElement>::const_iterator element;
-    for (element = elements->begin(); element != elements->end(); ++element)
-      presenters[i]->FeedNext(*element);
+    ScopedVector<net::UploadElement>::const_iterator element;
+    for (element = elements.begin(); element != elements.end(); ++element)
+      presenters[i]->FeedNext(**element);
     if (presenters[i]->Succeeded()) {
       requestBody->Set(kKeys[i], presenters[i]->Result().release());
       some_succeeded = true;

chrome/browser/policy/device_management_service_browsertest.cc

@@ -49,10 +49,10 @@ class CannedResponseInterceptor : public net::URLRequest::Interceptor {
     if (request->url().GetOrigin() == service_url_.GetOrigin() &&
         request->url().path() == service_url_.path() &&
         upload != NULL &&
-        upload->elements()->size() == 1) {
+        upload->elements().size() == 1) {
       std::string response_data;
-      ConstructResponse(upload->elements()->at(0).bytes(),
-                        upload->elements()->at(0).bytes_length(),
+      ConstructResponse(upload->elements()[0]->bytes(),
+                        upload->elements()[0]->bytes_length(),
                         &response_data);
       return new net::URLRequestTestJob(request,
                                         network_delegate,

chrome/browser/sessions/better_session_restore_browsertest.cc

@@ -55,11 +55,11 @@ net::URLRequestJob* URLRequestFakerForPostRequests(
   const net::UploadData* upload_data = request->get_upload();
   g_last_upload_bytes.Get().clear();
   if (upload_data) {
-    const std::vector<net::UploadElement>* elements = upload_data->elements();
-    for (size_t i = 0; elements && i < elements->size(); ++i) {
-      if ((*elements)[i].type() == net::UploadElement::TYPE_BYTES) {
+    const ScopedVector<net::UploadElement>& elements = upload_data->elements();
+    for (size_t i = 0; i < elements.size(); ++i) {
+      if (elements[i]->type() == net::UploadElement::TYPE_BYTES) {
         g_last_upload_bytes.Get() +=
-            std::string((*elements)[i].bytes(), (*elements)[i].bytes_length());
+            std::string(elements[i]->bytes(), elements[i]->bytes_length());
       }
     }
   }

chrome/common/automation_messages.cc

@@ -225,7 +225,7 @@ void ParamTraits<scoped_refptr<net::UploadData> >::Write(Message* m,
                                                          const param_type& p) {
   WriteParam(m, p.get() != NULL);
   if (p) {
-    WriteParam(m, *p->elements());
+    WriteParam(m, p->elements());
     WriteParam(m, p->identifier());
     WriteParam(m, p->is_chunked());
     WriteParam(m, p->last_chunk_appended());
@@ -240,7 +240,7 @@ bool ParamTraits<scoped_refptr<net::UploadData> >::Read(const Message* m,
     return false;
   if (!has_object)
     return true;
-  std::vector<net::UploadElement> elements;
+  ScopedVector<net::UploadElement> elements;
   if (!ReadParam(m, iter, &elements))
     return false;
   int64 identifier;

chrome_frame/urlmon_url_request.cc

@@ -1078,10 +1078,10 @@ void UrlmonUrlRequestManager::StartRequestHelper(
 
   // Format upload data if it's chunked.
   if (request_info.upload_data && request_info.upload_data->is_chunked()) {
-    std::vector<net::UploadElement>* elements =
+    ScopedVector<net::UploadElement>* elements =
         request_info.upload_data->elements_mutable();
     for (size_t i = 0; i < elements->size(); ++i) {
-      net::UploadElement* element = &(*elements)[i];
+      net::UploadElement* element = (*elements)[i];
       DCHECK(element->type() == net::UploadElement::TYPE_BYTES);
       std::string chunk_length = StringPrintf(
           "%X\r\n", static_cast<unsigned int>(element->bytes_length()));

ipc/ipc_message_utils.h

@@ -12,6 +12,7 @@
 #include <vector>
 
 #include "base/format_macros.h"
+#include "base/memory/scoped_vector.h"
 #include "base/platform_file.h"
 #include "base/string16.h"
 #include "base/stringprintf.h"
@@ -633,6 +634,37 @@ struct ParamTraits< Tuple5<A, B, C, D, E> > {
   }
 };
 
+template<class P>
+struct ParamTraits<ScopedVector<P> > {
+  typedef ScopedVector<P> param_type;
+  static void Write(Message* m, const param_type& p) {
+    WriteParam(m, static_cast<int>(p.size()));
+    for (size_t i = 0; i < p.size(); i++)
+      WriteParam(m, *p[i]);
+  }
+  static bool Read(const Message* m, PickleIterator* iter, param_type* r) {
+    int size = 0;
+    if (!m->ReadLength(iter, &size))
+      return false;
+    if (INT_MAX/sizeof(P) <= static_cast<size_t>(size))
+      return false;
+    r->resize(size);
+    for (int i = 0; i < size; i++) {
+      (*r)[i] = new P();
+      if (!ReadParam(m, iter, (*r)[i]))
+        return false;
+    }
+    return true;
+  }
+  static void Log(const param_type& p, std::string* l) {
+    for (size_t i = 0; i < p.size(); ++i) {
+      if (i != 0)
+        l->append(" ");
+      LogParam(*p[i], l);
+    }
+  }
+};
+
 // IPC types ParamTraits -------------------------------------------------------
 
 // A ChannelHandle is basically a platform-inspecific wrapper around the

net/base/upload_data.cc

@@ -20,27 +20,27 @@ UploadData::UploadData()
 void UploadData::AppendBytes(const char* bytes, int bytes_len) {
   DCHECK(!is_chunked_);
   if (bytes_len > 0) {
-    elements_.push_back(UploadElement());
-    elements_.back().SetToBytes(bytes, bytes_len);
+    elements_.push_back(new UploadElement());
+    elements_.back()->SetToBytes(bytes, bytes_len);
   }
 }
 
 void UploadData::AppendFileRange(const FilePath& file_path,
                                  uint64 offset, uint64 length,
                                  const base::Time& expected_modification_time) {
   DCHECK(!is_chunked_);
-  elements_.push_back(UploadElement());
-  elements_.back().SetToFilePathRange(file_path, offset, length,
-                                      expected_modification_time);
+  elements_.push_back(new UploadElement());
+  elements_.back()->SetToFilePathRange(file_path, offset, length,
+                                       expected_modification_time);
 }
 
 void UploadData::AppendChunk(const char* bytes,
                              int bytes_len,
                              bool is_last_chunk) {
   DCHECK(is_chunked_);
   DCHECK(!last_chunk_appended_);
-  elements_.push_back(UploadElement());
-  elements_.back().SetToBytes(bytes, bytes_len);
+  elements_.push_back(new UploadElement());
+  elements_.back()->SetToBytes(bytes, bytes_len);
   last_chunk_appended_ = is_last_chunk;
   if (!chunk_callback_.is_null())
     chunk_callback_.Run();
@@ -50,10 +50,6 @@ void UploadData::set_chunk_callback(const base::Closure& callback) {
   chunk_callback_ = callback;
 }
 
-void UploadData::SetElements(const std::vector<UploadElement>& elements) {
-  elements_ = elements;
-}
-
 UploadData::~UploadData() {
 }
 

net/base/upload_data.h

@@ -5,11 +5,10 @@
 #ifndef NET_BASE_UPLOAD_DATA_H_
 #define NET_BASE_UPLOAD_DATA_H_
 
-#include <vector>
-
 #include "base/basictypes.h"
 #include "base/callback.h"
 #include "base/memory/ref_counted.h"
+#include "base/memory/scoped_vector.h"
 #include "base/supports_user_data.h"
 #include "net/base/net_export.h"
 #include "net/base/upload_element.h"
@@ -61,17 +60,15 @@ class NET_EXPORT UploadData
   void set_last_chunk_appended(bool set) { last_chunk_appended_ = set; }
   bool last_chunk_appended() const { return last_chunk_appended_; }
 
-  const std::vector<UploadElement>* elements() const {
-    return &elements_;
+  const ScopedVector<UploadElement>& elements() const {
+    return elements_;
   }
 
-  std::vector<UploadElement>* elements_mutable() {
+  ScopedVector<UploadElement>* elements_mutable() {
     return &elements_;
   }
 
-  void SetElements(const std::vector<UploadElement>& elements);
-
-  void swap_elements(std::vector<UploadElement>* elements) {
+  void swap_elements(ScopedVector<UploadElement>* elements) {
     elements_.swap(*elements);
   }
 
@@ -86,7 +83,7 @@ class NET_EXPORT UploadData
 
   virtual ~UploadData();
 
-  std::vector<UploadElement> elements_;
+  ScopedVector<UploadElement> elements_;
   int64 identifier_;
   base::Closure chunk_callback_;
   bool is_chunked_;

net/base/upload_data_stream.cc

@@ -27,9 +27,9 @@ UploadDataStream::UploadDataStream(UploadData* upload_data)
       initialized_successfully_(false),
       weak_ptr_factory_(ALLOW_THIS_IN_INITIALIZER_LIST(this)),
       weak_ptr_factory_for_chunks_(ALLOW_THIS_IN_INITIALIZER_LIST(this)) {
-  const std::vector<UploadElement>& elements = *upload_data_->elements();
+  const ScopedVector<UploadElement>& elements = upload_data_->elements();
   for (size_t i = 0; i < elements.size(); ++i)
-    element_readers_.push_back(UploadElementReader::Create(elements[i]));
+    element_readers_.push_back(UploadElementReader::Create(*elements[i]));
 
   upload_data_->set_chunk_callback(
       base::Bind(&UploadDataStream::OnChunkAvailable,
@@ -241,13 +241,13 @@ void UploadDataStream::OnChunkAvailable() {
   DCHECK(is_chunked());
 
   // Initialize a reader for the newly appended chunk.
-  const std::vector<UploadElement>& elements = *upload_data_->elements();
+  const ScopedVector<UploadElement>& elements = upload_data_->elements();
   DCHECK_EQ(elements.size(), element_readers_.size() + 1);
 
   // We can initialize the reader synchronously here because only bytes can be
   // appended for chunked data. We leave |total_size_| at zero, since for
   // chunked uploads, we may not know the total size.
-  const UploadElement& element = elements.back();
+  const UploadElement& element = *elements.back();
   DCHECK_EQ(UploadElement::TYPE_BYTES, element.type());
   UploadElementReader* reader = UploadElementReader::Create(element);
   const int rv = reader->InitSync();

net/base/upload_data_stream_unittest.cc

@@ -177,11 +177,7 @@ TEST_F(UploadDataStreamTest, File) {
   ASSERT_EQ(static_cast<int>(kTestDataSize),
             file_util::WriteFile(temp_file_path, kTestData, kTestDataSize));
 
-  std::vector<UploadElement> elements;
-  UploadElement element;
-  element.SetToFilePath(temp_file_path);
-  elements.push_back(element);
-  upload_data_->SetElements(elements);
+  upload_data_->AppendFileRange(temp_file_path, 0, kuint64max, base::Time());
 
   UploadDataStream stream(upload_data_);
   ASSERT_EQ(OK, stream.InitSync());
@@ -209,11 +205,7 @@ TEST_F(UploadDataStreamTest, FileSmallerThanLength) {
   UploadFileElementReader::ScopedOverridingContentLengthForTests
       overriding_content_length(kFakeSize);
 
-  std::vector<UploadElement> elements;
-  UploadElement element;
-  element.SetToFilePath(temp_file_path);
-  elements.push_back(element);
-  upload_data_->SetElements(elements);
+  upload_data_->AppendFileRange(temp_file_path, 0, kuint64max, base::Time());
 
   UploadDataStream stream(upload_data_);
   ASSERT_EQ(OK, stream.InitSync());
@@ -442,14 +434,10 @@ TEST_F(UploadDataStreamTest, ReadAsync) {
 void UploadDataStreamTest::FileChangedHelper(const FilePath& file_path,
                                              const base::Time& time,
                                              bool error_expected) {
-  std::vector<UploadElement> elements;
-  UploadElement element;
-  element.SetToFilePathRange(file_path, 1, 2, time);
-  elements.push_back(element);
   // Don't use upload_data_ here, as this function is called twice, and
   // reusing upload_data_ is wrong.
   scoped_refptr<UploadData> upload_data(new UploadData);
-  upload_data->SetElements(elements);
+  upload_data->AppendFileRange(file_path, 1, 2, time);
 
   UploadDataStream stream(upload_data);
   int error_code = stream.InitSync();
@@ -486,11 +474,7 @@ TEST_F(UploadDataStreamTest, UploadDataReused) {
             file_util::WriteFile(temp_file_path, kTestData, kTestDataSize));
 
   // Prepare |upload_data_| that contains a file.
-  std::vector<UploadElement> elements;
-  UploadElement element;
-  element.SetToFilePath(temp_file_path);
-  elements.push_back(element);
-  upload_data_->SetElements(elements);
+  upload_data_->AppendFileRange(temp_file_path, 0, kuint64max, base::Time());
 
   // Confirm that the file is read properly.
   {

net/base/upload_element.h

@@ -77,6 +77,8 @@ class NET_EXPORT UploadElement {
   uint64 file_range_offset_;
   uint64 file_range_length_;
   base::Time expected_file_modification_time_;
+
+  DISALLOW_COPY_AND_ASSIGN(UploadElement);
 };
 
 #if defined(UNIT_TEST)

net/http/http_network_transaction_spdy2_unittest.cc

@@ -6583,11 +6583,8 @@ TEST_F(HttpNetworkTransactionSpdy2Test, UploadFileSmallerThanLength) {
   UploadFileElementReader::ScopedOverridingContentLengthForTests
       overriding_content_length(kFakeSize);
 
-  std::vector<UploadElement> elements;
-  UploadElement element;
-  element.SetToFilePath(temp_file_path);
-  elements.push_back(element);
-  request.upload_data->SetElements(elements);
+  request.upload_data->AppendFileRange(
+      temp_file_path, 0, kuint64max, base::Time());
 
   MockRead data_reads[] = {
     MockRead("HTTP/1.0 200 OK\r\n\r\n"),
@@ -6639,11 +6636,7 @@ TEST_F(HttpNetworkTransactionSpdy2Test, UploadUnreadableFile) {
                                    temp_file_content.length()));
   ASSERT_TRUE(file_util::MakeFileUnreadable(temp_file));
 
-  std::vector<UploadElement> elements;
-  UploadElement element;
-  element.SetToFilePath(temp_file);
-  elements.push_back(element);
-  request.upload_data->SetElements(elements);
+  request.upload_data->AppendFileRange(temp_file, 0, kuint64max, base::Time());
 
   MockRead data_reads[] = {
     MockRead("HTTP/1.0 200 OK\r\n\r\n"),
@@ -6694,11 +6687,7 @@ TEST_F(HttpNetworkTransactionSpdy2Test, UnreadableUploadFileAfterAuthRestart) {
   ASSERT_TRUE(file_util::WriteFile(temp_file, temp_file_contents.c_str(),
                                    temp_file_contents.length()));
 
-  std::vector<UploadElement> elements;
-  UploadElement element;
-  element.SetToFilePath(temp_file);
-  elements.push_back(element);
-  request.upload_data->SetElements(elements);
+  request.upload_data->AppendFileRange(temp_file, 0, kuint64max, base::Time());
 
   MockRead data_reads[] = {
     MockRead("HTTP/1.1 401 Unauthorized\r\n"),

net/http/http_network_transaction_spdy3_unittest.cc

@@ -6583,11 +6583,8 @@ TEST_F(HttpNetworkTransactionSpdy3Test, UploadFileSmallerThanLength) {
   UploadFileElementReader::ScopedOverridingContentLengthForTests
       overriding_content_length(kFakeSize);
 
-  std::vector<UploadElement> elements;
-  UploadElement element;
-  element.SetToFilePath(temp_file_path);
-  elements.push_back(element);
-  request.upload_data->SetElements(elements);
+  request.upload_data->AppendFileRange(
+      temp_file_path, 0, kuint64max, base::Time());
 
   MockRead data_reads[] = {
     MockRead("HTTP/1.0 200 OK\r\n\r\n"),
@@ -6639,11 +6636,7 @@ TEST_F(HttpNetworkTransactionSpdy3Test, UploadUnreadableFile) {
                                    temp_file_content.length()));
   ASSERT_TRUE(file_util::MakeFileUnreadable(temp_file));
 
-  std::vector<UploadElement> elements;
-  UploadElement element;
-  element.SetToFilePath(temp_file);
-  elements.push_back(element);
-  request.upload_data->SetElements(elements);
+  request.upload_data->AppendFileRange(temp_file, 0, kuint64max, base::Time());
 
   MockRead data_reads[] = {
     MockRead("HTTP/1.0 200 OK\r\n\r\n"),
@@ -6694,11 +6687,7 @@ TEST_F(HttpNetworkTransactionSpdy3Test, UnreadableUploadFileAfterAuthRestart) {
   ASSERT_TRUE(file_util::WriteFile(temp_file, temp_file_contents.c_str(),
                                    temp_file_contents.length()));
 
-  std::vector<UploadElement> elements;
-  UploadElement element;
-  element.SetToFilePath(temp_file);
-  elements.push_back(element);
-  request.upload_data->SetElements(elements);
+  request.upload_data->AppendFileRange(temp_file, 0, kuint64max, base::Time());
 
   MockRead data_reads[] = {
     MockRead("HTTP/1.1 401 Unauthorized\r\n"),