Relanding: Deduplicating code performing WebHTTPBody::Element conversions.

storage/common/data_element.h

 // Copyright 2013 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 STORAGE_COMMON_DATA_ELEMENT_H_
 #define STORAGE_COMMON_DATA_ELEMENT_H_
 
 #include <stddef.h>
 #include <stdint.h>
 
@@ skipping 24 matching lines @@
     TYPE_BLOB,
     TYPE_FILE_FILESYSTEM,
     TYPE_DISK_CACHE_ENTRY,
   };
 
   DataElement();
   DataElement(const DataElement& other);
   ~DataElement();
 
   Type type() const { return type_; }
-  const char* bytes() const { return bytes_ ? bytes_ : &buf_[0]; }
+  const char* bytes() const { return bytes_ ? bytes_ : buf_.data(); }

[Łukasz Anforowicz, 2016/05/25 00:26:18]

There are places in code where users of DataElement would check that type() ==
TYPE_BYTES and then assume that they can call something like
std::string(elem.bytes(), elem.length()).  This seems like a valid assumption -
similar to how it is okay to access STL's container's begin() or data() even if
the container is empty (it is okay as long as one doesn't dereference the
result).

The old code would crash when |buf_| was empty.  The crash would only happen in
debug builds - the STL in Goobuntu would say that dereferencing element 0 is
wrong when there are zero elements.

I assume that the old code was written before C++11 where std::vector::data()
accessor was introduced.  The new accessors doesn't crash for empty containers
(even on debug builds).

The above hopefully convinced you that the change above is really the right fix,
but you might still be asking yourself why we didn't hit this problem before
this CL.  The answer is that before this CL ExplodedHttpBodyElement would hold
bytes in a std::string, rather than in storage::DataElement (as it does after
the deduplication from this CL).

[danakj, 2016/05/25 22:13:46]

Returning null lgtm

   const base::FilePath& path() const { return path_; }
   const GURL& filesystem_url() const { return filesystem_url_; }
   const std::string& blob_uuid() const { return blob_uuid_; }
   uint64_t offset() const { return offset_; }
   uint64_t length() const { return length_; }
   const base::Time& expected_modification_time() const {
     return expected_modification_time_;
   }
 
   // For use with SetToAllocatedBytes. Should only be used after calling
@@ skipping 99 matching lines @@
 };
 
 STORAGE_COMMON_EXPORT bool operator==(const DataElement& a,
                                       const DataElement& b);
 STORAGE_COMMON_EXPORT bool operator!=(const DataElement& a,
                                       const DataElement& b);
 
 }  // namespace storage
 
 #endif  // STORAGE_COMMON_DATA_ELEMENT_H_