string_piece.h says:

  // data() may return a pointer to a buffer with embedded NULs, and the
  // returned buffer may or may not be null terminated.  Therefore it is
  // typically a mistake to pass data() to a routine that expects a NUL
  // terminated string.
  const value_type* data() const { return ptr_; }

This makes sense because a StringPiece may reference a substring of an existing string in which case NUL termination is impossible.

Meanwhile, in crashpad.cc we have:

void SetCrashKeyValue(const base::StringPiece& key,
                      const base::StringPiece& value) {
  // Convert from StringPiece to string to ensure null-termination.
  g_simple_string_dictionary->SetKeyValue(key.as_string().c_str(),
                                          value.as_string().c_str());
}

void ClearCrashKey(const base::StringPiece& key) {
  // Convert from StringPiece to string to ensure null-termination.
  g_simple_string_dictionary->RemoveKey(key.as_string().c_str());
}

Where the signatures of SetKeyValue and RemoveKey are:

SetKeyValue(const char* key, const char* value)
void RemoveKey(const char* key)


So, SetCrashKeyValue and ClearCrashKey value are violating the StringPiece contract. Prior to C++ 11 this could be a problem because std::string objects were not guaranteed to be NUL terminated, but with C++ 11 they are. So, probably the only risk is if SetCrashKeyValue or ClearCrashKey are called with sub-strings of some sort, and it appears that they are not.

On the other hand, converting from StringPiece to const char* is still generally a bad idea because the length information is lost and usually has to be reconstructed.

So, to avoid the risk of bugs and to allow for greater efficiency we should retain StringPiece all the way down. This should be kept in mind as the crash key system is redesigned, and I'll clean it up slightly for now.