net: Fix a regression that broke file uploading with http authentication

net/base/upload_data.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_DATA_H_
 #define NET_BASE_UPLOAD_DATA_H_
 #pragma once
 
 #include <vector>
 
@@ skipping 111 matching lines @@
 
     // Reads up to |buf_len| bytes synchronously. Returns the number of bytes
     // read. This function never fails. If there's less data to read than we
     // initially observed, then pad with zero (this can happen with files).
     // |buf_len| must be greater than 0.
     int ReadSync(char* buf, int buf_len);
 
     // Returns the number of bytes remaining to read.
     uint64 BytesRemaining();
 
-    // Resets the offset to zero, so that the element can be reread.
-    void ResetOffset() { offset_ = 0; }
+    // Resets the offset to zero and closes the file stream if opened, so
+    // that the element can be reread.
+    void ResetOffset();
 
    private:
     // Returns a FileStream opened for reading for this element, positioned
     // at |file_range_offset_|. Returns NULL if the file is not openable.
     FileStream* OpenFileStream();
 
     // Reads up to |buf_len| bytes synchronously from memory (i.e. type_ is
     // TYPE_BYTES or TYPE_CHUNK).
     int ReadFromMemorySync(char* buf, int buf_len);
 
@@ skipping 17 matching lines @@
     bool is_last_chunk_;
     bool override_content_length_;
     bool content_length_computed_;
     uint64 content_length_;
 
     // The byte offset from the beginning of the element data. Used to track
     // the current position when reading data.
     size_t offset_;
 
     // The stream of the element data, if this element is of TYPE_FILE.
     FileStream* file_stream_;

[willchan no longer on Chromium, 2012/05/21 18:12:51]

Hm, why isn't this a scoped_ptr? It looks like we own this.

[satorux1, 2012/05/21 19:37:00]

IIRC, I once tried to make it a scoped_ptr but failed as the UploadData::Element
has to be copyable (the Element class is hold as std::vector<Element> in
UploadData) but making it a scoped_ptr makes it non-copyable. 

[willchan no longer on Chromium, 2012/05/21 20:27:40]

Ugh, so fragile :( This code sucks.

[wtc, 2012/05/21 23:13:11]

Should we use base/memory/linked_ptr.h to deal with this?
    base::link_ptr<FileStream> file_stream_;
should allow UploadData::Element to be copyable.  But I'm
not sure about the current opinion about base/memory/linked_ptr.h.

[willchan no longer on Chromium, 2012/05/21 23:38:55]

I don't think we should do this. The fundamental problem is that this isn't
truly copyable. The FileStream should *not* be shared across
UploadData::Elements. The way it works right now is fragile, I think we rely on
convention that we don't actually ever copy the vector. The Elements are only
copied when resizing the vector.

 
     FRIEND_TEST_ALL_PREFIXES(UploadDataStreamTest, FileSmallerThanLength);
     FRIEND_TEST_ALL_PREFIXES(HttpNetworkTransactionTest,
                              UploadFileSmallerThanLength);
     FRIEND_TEST_ALL_PREFIXES(HttpNetworkTransactionSpdy2Test,
                              UploadFileSmallerThanLength);
     FRIEND_TEST_ALL_PREFIXES(HttpNetworkTransactionSpdy3Test,
                              UploadFileSmallerThanLength);
   };
 
@@ skipping 94 matching lines @@
 
 inline bool operator!=(const UploadData::Element& a,
                        const UploadData::Element& b) {
   return !(a == b);
 }
 #endif  // defined(UNIT_TEST)
 
 }  // namespace net
 
 #endif  // NET_BASE_UPLOAD_DATA_H_