Relanding: Deduplicating code performing WebHTTPBody::Element conversions.

storage/common/data_element.h

Index: storage/common/data_element.h
diff --git a/storage/common/data_element.h b/storage/common/data_element.h
index 08fea6e676796058a719be7f3587f952f1323204..276181b6ec97d9a92a5b1ce4c43c05c3a8831faa 100644
--- a/storage/common/data_element.h
+++ b/storage/common/data_element.h
@@ -42,7 +42,7 @@ class STORAGE_COMMON_EXPORT DataElement {
   ~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_; }