Create LongStringDictionary and replace SimpleStringDictionary on iOS

This relands fd0a0d2b7ae9dd3d8a02b6a12e7941f7189fbb6c which was reverted
in 5dad29423e62292c6ff468cabfee4422ba55b18b, with a fix for guarding
kMaxSuffixLength which only used in assert()s with macros which breaks
chromium.mac/ios-device.

Change-Id: I5ee21b7f290517d6e7a0ef90b693b97f92392549
Reviewed-on: https://chromium-review.googlesource.com/751922
Reviewed-by: Mark Mentovai <mark@chromium.org>

5 files changed, 573 insertions, 2 deletions

diff --git a/src/common/common.gyp b/src/common/common.gyp
index 9cc90f84..e2ea4007 100644
--- a/src/common/common.gyp
+++ b/src/common/common.gyp
@@ -121,6 +121,8 @@
         'linux/safe_readlink.h',
         'linux/synth_elf.cc',
         'linux/synth_elf.h',
+        'long_string_dictionary.cc',
+        'long_string_dictionary.h',
         'mac/arch_utilities.cc',
         'mac/arch_utilities.h',
         'mac/bootstrap_compat.cc',
@@ -220,6 +222,7 @@
         'linux/tests/auto_testfile.h',
         'linux/tests/crash_generator.cc',
         'linux/tests/crash_generator.h',
+        'long_string_dictionary_unittest.cc',
         'mac/macho_reader_unittest.cc',
         'memory_allocator_unittest.cc',
         'memory_range_unittest.cc',
diff --git a/src/common/long_string_dictionary.cc b/src/common/long_string_dictionary.cc
new file mode 100644
index 00000000..46bbf613
--- /dev/null
+++ b/src/common/long_string_dictionary.cc
@@ -0,0 +1,178 @@
+// Copyright (c) 2017, Google Inc.
+// All rights reserved.
+//
+// Redistribution and use in source and binary forms, with or without
+// modification, are permitted provided that the following conditions are
+// met:
+//
+//     * Redistributions of source code must retain the above copyright
+// notice, this list of conditions and the following disclaimer.
+//     * Redistributions in binary form must reproduce the above
+// copyright notice, this list of conditions and the following disclaimer
+// in the documentation and/or other materials provided with the
+// distribution.
+//     * Neither the name of Google Inc. nor the names of its
+// contributors may be used to endorse or promote products derived from
+// this software without specific prior written permission.
+//
+// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+#include "common/long_string_dictionary.h"
+
+#include <assert.h>
+#include <string.h>
+
+#include <algorithm>
+#include <string>
+
+#include "common/simple_string_dictionary.h"
+
+#define arraysize(f) (sizeof(f) / sizeof(*f))
+
+namespace {
+// Suffixes for segment keys.
+const char* const kSuffixes[] = {"__1", "__2", "__3", "__4", "__5", "__6",
+    "__7", "__8", "__9", "__10"};
+#if !defined(NDEBUG)
+// The maximum suffix string length.
+const size_t kMaxSuffixLength = 4;
+#endif
+} // namespace
+
+namespace google_breakpad {
+
+using std::string;
+
+void LongStringDictionary::SetKeyValue(const char* key, const char* value) {
+  assert(key);
+  if (!key)
+    return;
+
+  RemoveKey(key);
+
+  if (!value) {
+    return;
+  }
+
+  // Key must not be an empty string.
+  assert(key[0] != '\0');
+  if (key[0] == '\0')
+    return;
+
+  // If the value is not valid for segmentation, forwards the key and the value
+  // to SetKeyValue of SimpleStringDictionary and returns.
+  size_t value_length = strlen(value);
+  if (value_length <= (value_size - 1)) {
+    SimpleStringDictionary::SetKeyValue(key, value);
+    return;
+  }
+
+  size_t key_length = strlen(key);
+  assert(key_length + kMaxSuffixLength <= (key_size - 1));
+
+  char segment_key[key_size];
+  char segment_value[value_size];
+
+  strcpy(segment_key, key);
+
+  const char* remain_value = value;
+  size_t remain_value_length = strlen(value);
+
+  for (unsigned long i = 0; i < arraysize(kSuffixes); i++) {
+    if (remain_value_length == 0) {
+      return;
+    }
+
+    strcpy(segment_key + key_length, kSuffixes[i]);
+
+    size_t segment_value_length =
+        std::min(remain_value_length, value_size - 1);
+
+    strncpy(segment_value, remain_value, segment_value_length);
+    segment_value[segment_value_length] = '\0';
+
+    remain_value += segment_value_length;
+    remain_value_length -= segment_value_length;
+
+    SimpleStringDictionary::SetKeyValue(segment_key, segment_value);
+  }
+}
+
+bool LongStringDictionary::RemoveKey(const char* key) {
+  assert(key);
+  if (!key)
+    return false;
+
+  if (SimpleStringDictionary::RemoveKey(key)) {
+    return true;
+  }
+
+  size_t key_length = strlen(key);
+  assert(key_length + kMaxSuffixLength <= (key_size - 1));
+
+  char segment_key[key_size];
+  strcpy(segment_key, key);
+
+  unsigned long i = 0;
+  for (; i < arraysize(kSuffixes); i++) {
+    strcpy(segment_key + key_length, kSuffixes[i]);
+    if (!SimpleStringDictionary::RemoveKey(segment_key)) {
+      break;
+    }
+  }
+  return i != 0;
+}
+
+const string LongStringDictionary::GetValueForKey(const char* key) const {
+  assert(key);
+  if (!key)
+    return "";
+
+  // Key must not be an empty string.
+  assert(key[0] != '\0');
+  if (key[0] == '\0')
+    return "";
+
+  const char* value = SimpleStringDictionary::GetValueForKey(key);
+  if (value)
+    return string(value);
+
+  size_t key_length = strlen(key);
+  assert(key_length + kMaxSuffixLength <= (key_size - 1));
+
+  bool found_segment = false;
+  char segment_key[key_size];
+  string return_value;
+
+  strcpy(segment_key, key);
+  for (unsigned long i = 0; i < arraysize(kSuffixes); i++) {
+    strcpy(segment_key + key_length, kSuffixes[i]);
+
+    const char* segment_value =
+        SimpleStringDictionary::GetValueForKey(segment_key);
+
+    if (segment_value != NULL) {
+      found_segment = true;
+      return_value.append(segment_value);
+    } else {
+      break;
+    }
+  }
+
+  if (found_segment) {
+    return return_value;
+  }
+  return "";
+}
+
+}  // namespace google_breakpad
diff --git a/src/common/long_string_dictionary.h b/src/common/long_string_dictionary.h
new file mode 100644
index 00000000..68bf03de
--- /dev/null
+++ b/src/common/long_string_dictionary.h
@@ -0,0 +1,87 @@
+// Copyright (c) 2017, Google Inc.
+// All rights reserved.
+//
+// Redistribution and use in source and binary forms, with or without
+// modification, are permitted provided that the following conditions are
+// met:
+//
+//     * Redistributions of source code must retain the above copyright
+// notice, this list of conditions and the following disclaimer.
+//     * Redistributions in binary form must reproduce the above
+// copyright notice, this list of conditions and the following disclaimer
+// in the documentation and/or other materials provided with the
+// distribution.
+//     * Neither the name of Google Inc. nor the names of its
+// contributors may be used to endorse or promote products derived from
+// this software without specific prior written permission.
+//
+// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+#ifndef COMMON_LONG_STRING_DICTIONARY_H_
+#define COMMON_LONG_STRING_DICTIONARY_H_
+
+#include <string>
+
+#include "common/simple_string_dictionary.h"
+
+namespace google_breakpad {
+// key_size is the maxium size that |key| can take in
+// SimpleStringDictionary which is defined in simple_string_dictionary.h.
+//
+// value_size is the maxium size that |value| can take in
+// SimpleStringDictionary which is defined in simple_string_dictionary.h.
+//
+// LongStringDictionary is a subclass of SimpleStringDictionary which supports
+// longer values to be stored in the dictionary. The maximum length supported is
+// (value_size - 1) * 10.
+//
+// For example, LongStringDictionary will store long value with key 'abc' into
+// segment values with segment keys 'abc__1', 'abc__2', 'abc__3', ...
+//
+// Clients must avoid using the same suffixes as their key's suffix when
+// LongStringDictionary is used.
+class LongStringDictionary : public SimpleStringDictionary {
+ public:
+  // Stores |value| into |key|, or segment values into segment keys. The maxium
+  // number of segments is 10. If |value| can not be stored in 10 segments, it
+  // will be truncated. Replacing the existing value if |key| is already present
+  // and replacing the existing segment values if segment keys are already
+  // present.
+  //
+  // |key| must not be NULL. If the |value| need to be divided into segments,
+  // the lengh of |key| must be smaller enough so that lengths of segment keys
+  // which are key with suffixes are all samller than (key_size - 1). Currently,
+  // the max length of suffixes are 4.
+  //
+  // If |value| is NULL, the key and its corresponding segment keys are removed
+  // from the map. If there is no more space in the map, then the operation
+  // silently fails.
+  void SetKeyValue(const char* key, const char* value);
+
+  // Given |key|, removes any associated value or associated segment values.
+  // |key| must not be NULL. If the key is not found, searchs its segment keys
+  // and removes corresponding segment values if found.
+  bool RemoveKey(const char* key);
+
+  // Given |key|, returns its corresponding |value|. |key| must not be NULL. If
+  // the key is found, its corresponding |value| is returned.
+  //
+  // If no corresponding |value| is found, segment keys of the given |key| will
+  // be used to search for corresponding segment values. If segment values
+  // exist, assembled value from them is returned. If no segment value exists,
+  // NULL is returned.
+  const std::string GetValueForKey(const char* key) const;
+};
+} // namespace google_breakpad
+
+#endif // COMMON_LONG_STRING_DICTIONARY_H_
diff --git a/src/common/long_string_dictionary_unittest.cc b/src/common/long_string_dictionary_unittest.cc
new file mode 100644
index 00000000..f9b645ba
--- /dev/null
+++ b/src/common/long_string_dictionary_unittest.cc
@@ -0,0 +1,301 @@
+// Copyright (c) 2017, Google Inc.
+// All rights reserved.
+//
+// Redistribution and use in source and binary forms, with or without
+// modification, are permitted provided that the following conditions are
+// met:
+//
+//     * Redistributions of source code must retain the above copyright
+// notice, this list of conditions and the following disclaimer.
+//     * Redistributions in binary form must reproduce the above
+// copyright notice, this list of conditions and the following disclaimer
+// in the documentation and/or other materials provided with the
+// distribution.
+//     * Neither the name of Google Inc. nor the names of its
+// contributors may be used to endorse or promote products derived from
+// this software without specific prior written permission.
+//
+// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+#include <algorithm>
+#include <string>
+
+#include "breakpad_googletest_includes.h"
+#include "common/long_string_dictionary.h"
+
+namespace google_breakpad {
+
+using std::string;
+
+TEST(LongStringDictionary, LongStringDictionary) {
+  // Make a new dictionary
+  LongStringDictionary dict;
+
+  // Set three distinct values on three keys
+  dict.SetKeyValue("key1", "value1");
+  dict.SetKeyValue("key2", "value2");
+  dict.SetKeyValue("key3", "value3");
+
+  EXPECT_EQ("value1", dict.GetValueForKey("key1"));
+  EXPECT_EQ("value2", dict.GetValueForKey("key2"));
+  EXPECT_EQ("value3", dict.GetValueForKey("key3"));
+  EXPECT_EQ(3u, dict.GetCount());
+  // try an unknown key
+  EXPECT_EQ("", dict.GetValueForKey("key4"));
+
+  // Remove a key
+  dict.RemoveKey("key3");
+
+  // Now make sure it's not there anymore
+  EXPECT_EQ("", dict.GetValueForKey("key3"));
+
+  // Remove by setting value to NULL
+  dict.SetKeyValue("key2", NULL);
+
+  // Now make sure it's not there anymore
+  EXPECT_EQ("", dict.GetValueForKey("key2"));
+}
+
+// Add a bunch of values to the dictionary, remove some entries in the middle,
+// and then add more.
+TEST(LongStringDictionary, Iterator) {
+  LongStringDictionary* dict = new LongStringDictionary();
+  ASSERT_TRUE(dict);
+
+  char key[LongStringDictionary::key_size];
+  char value[LongStringDictionary::value_size];
+
+  const int kDictionaryCapacity = LongStringDictionary::num_entries;
+  const int kPartitionIndex = kDictionaryCapacity - 5;
+
+  // We assume at least this size in the tests below
+  ASSERT_GE(kDictionaryCapacity, 64);
+
+  // We'll keep track of the number of key/value pairs we think should
+  // be in the dictionary
+  int expectedDictionarySize = 0;
+
+  // Set a bunch of key/value pairs like key0/value0, key1/value1, ...
+  for (int i = 0; i < kPartitionIndex; ++i) {
+    sprintf(key, "key%d", i);
+    sprintf(value, "value%d", i);
+    dict->SetKeyValue(key, value);
+  }
+  expectedDictionarySize = kPartitionIndex;
+
+  // set a couple of the keys twice (with the same value) - should be nop
+  dict->SetKeyValue("key2", "value2");
+  dict->SetKeyValue("key4", "value4");
+  dict->SetKeyValue("key15", "value15");
+
+  // Remove some random elements in the middle
+  dict->RemoveKey("key7");
+  dict->RemoveKey("key18");
+  dict->RemoveKey("key23");
+  dict->RemoveKey("key31");
+  expectedDictionarySize -= 4; // we just removed four key/value pairs
+
+  // Set some more key/value pairs like key59/value59, key60/value60, ...
+  for (int i = kPartitionIndex; i < kDictionaryCapacity; ++i) {
+    sprintf(key, "key%d", i);
+    sprintf(value, "value%d", i);
+    dict->SetKeyValue(key, value);
+  }
+  expectedDictionarySize += kDictionaryCapacity - kPartitionIndex;
+
+  // Now create an iterator on the dictionary
+  SimpleStringDictionary::Iterator iter(*dict);
+
+  // We then verify that it iterates through exactly the number of
+  // key/value pairs we expect, and that they match one-for-one with what we
+  // would expect.  The ordering of the iteration does not matter...
+
+  // used to keep track of number of occurrences found for key/value pairs
+  int count[kDictionaryCapacity];
+  memset(count, 0, sizeof(count));
+
+  int totalCount = 0;
+
+  const SimpleStringDictionary::Entry* entry;
+  while ((entry = iter.Next())) {
+    totalCount++;
+
+    // Extract keyNumber from a string of the form key<keyNumber>
+    int keyNumber;
+    sscanf(entry->key, "key%d", &keyNumber);
+
+    // Extract valueNumber from a string of the form value<valueNumber>
+    int valueNumber;
+    sscanf(entry->value, "value%d", &valueNumber);
+
+    // The value number should equal the key number since that's how we set them
+    EXPECT_EQ(keyNumber, valueNumber);
+
+    // Key and value numbers should be in proper range:
+    // 0 <= keyNumber < kDictionaryCapacity
+    bool isKeyInGoodRange = (keyNumber >= 0 && keyNumber < kDictionaryCapacity);
+    bool isValueInGoodRange =
+        (valueNumber >= 0 && valueNumber < kDictionaryCapacity);
+    EXPECT_TRUE(isKeyInGoodRange);
+    EXPECT_TRUE(isValueInGoodRange);
+
+    if (isKeyInGoodRange && isValueInGoodRange) {
+      ++count[keyNumber];
+    }
+  }
+
+  // Make sure each of the key/value pairs showed up exactly one time, except
+  // for the ones which we removed.
+  for (size_t i = 0; i < kDictionaryCapacity; ++i) {
+    // Skip over key7, key18, key23, and key31, since we removed them
+    if (!(i == 7 || i == 18 || i == 23 || i == 31)) {
+      EXPECT_EQ(count[i], 1);
+    }
+  }
+
+  // Make sure the number of iterations matches the expected dictionary size.
+  EXPECT_EQ(totalCount, expectedDictionarySize);
+}
+
+TEST(LongStringDictionary, AddRemove) {
+  LongStringDictionary dict;
+  dict.SetKeyValue("rob", "ert");
+  dict.SetKeyValue("mike", "pink");
+  dict.SetKeyValue("mark", "allays");
+
+  EXPECT_EQ(3u, dict.GetCount());
+  EXPECT_EQ("ert", dict.GetValueForKey("rob"));
+  EXPECT_EQ("pink", dict.GetValueForKey("mike"));
+  EXPECT_EQ("allays", dict.GetValueForKey("mark"));
+
+  dict.RemoveKey("mike");
+
+  EXPECT_EQ(2u, dict.GetCount());
+  EXPECT_EQ("", dict.GetValueForKey("mike"));
+
+  dict.SetKeyValue("mark", "mal");
+  EXPECT_EQ(2u, dict.GetCount());
+  EXPECT_EQ("mal", dict.GetValueForKey("mark"));
+
+  dict.RemoveKey("mark");
+  EXPECT_EQ(1u, dict.GetCount());
+  EXPECT_EQ("", dict.GetValueForKey("mark"));
+}
+
+TEST(LongStringDictionary, AddRemoveLongValue) {
+  LongStringDictionary dict;
+
+  string long_value = string(256, 'x');
+  dict.SetKeyValue("rob", long_value.c_str());
+
+  EXPECT_EQ(2u, dict.GetCount());
+
+  string long_value_part_1 = string(255, 'x');
+
+  EXPECT_EQ(long_value_part_1, dict.GetValueForKey("rob__1"));
+  EXPECT_EQ("x", dict.GetValueForKey("rob__2"));
+
+  EXPECT_EQ(long_value, dict.GetValueForKey("rob"));
+
+  dict.RemoveKey("rob");
+  EXPECT_EQ(0u, dict.GetCount());
+}
+
+TEST(LongStringDictionary, AddRemoveSuperLongValue) {
+  LongStringDictionary dict;
+
+  string long_value = string(255 * 10, 'x');
+  dict.SetKeyValue("rob", long_value.c_str());
+
+  EXPECT_EQ(10u, dict.GetCount());
+
+  string long_value_part = string(255, 'x');
+
+  EXPECT_EQ(long_value_part, dict.GetValueForKey("rob__1"));
+  EXPECT_EQ(long_value_part, dict.GetValueForKey("rob__2"));
+  EXPECT_EQ(long_value_part, dict.GetValueForKey("rob__3"));
+  EXPECT_EQ(long_value_part, dict.GetValueForKey("rob__4"));
+  EXPECT_EQ(long_value_part, dict.GetValueForKey("rob__5"));
+  EXPECT_EQ(long_value_part, dict.GetValueForKey("rob__6"));
+  EXPECT_EQ(long_value_part, dict.GetValueForKey("rob__7"));
+  EXPECT_EQ(long_value_part, dict.GetValueForKey("rob__8"));
+  EXPECT_EQ(long_value_part, dict.GetValueForKey("rob__9"));
+  EXPECT_EQ(long_value_part, dict.GetValueForKey("rob__10"));
+  EXPECT_EQ(10u, dict.GetCount());
+
+  EXPECT_EQ(long_value, dict.GetValueForKey("rob"));
+
+  dict.RemoveKey("rob");
+  EXPECT_EQ(0u, dict.GetCount());
+}
+
+TEST(LongStringDictionary, TruncateSuperLongValue) {
+  LongStringDictionary dict;
+
+  string long_value = string(255 * 11, 'x');
+  dict.SetKeyValue("rob", long_value.c_str());
+
+  EXPECT_EQ(10u, dict.GetCount());
+
+  string long_value_part = string(255, 'x');
+
+  EXPECT_EQ(long_value_part, dict.GetValueForKey("rob__1"));
+  EXPECT_EQ(long_value_part, dict.GetValueForKey("rob__2"));
+  EXPECT_EQ(long_value_part, dict.GetValueForKey("rob__3"));
+  EXPECT_EQ(long_value_part, dict.GetValueForKey("rob__4"));
+  EXPECT_EQ(long_value_part, dict.GetValueForKey("rob__5"));
+  EXPECT_EQ(long_value_part, dict.GetValueForKey("rob__6"));
+  EXPECT_EQ(long_value_part, dict.GetValueForKey("rob__7"));
+  EXPECT_EQ(long_value_part, dict.GetValueForKey("rob__8"));
+  EXPECT_EQ(long_value_part, dict.GetValueForKey("rob__9"));
+  EXPECT_EQ(long_value_part, dict.GetValueForKey("rob__10"));
+  EXPECT_EQ(10u, dict.GetCount());
+
+  string expected_long_value = string(255 * 10, 'x');
+  EXPECT_EQ(expected_long_value, dict.GetValueForKey("rob"));
+
+  dict.RemoveKey("rob");
+  EXPECT_EQ(0u, dict.GetCount());
+}
+
+TEST(LongStringDictionary, OverrideLongValue) {
+  LongStringDictionary dict;
+
+  string long_value = string(255 * 10, 'x');
+  dict.SetKeyValue("rob", long_value.c_str());
+
+  EXPECT_EQ(10u, dict.GetCount());
+  EXPECT_EQ(long_value, dict.GetValueForKey("rob"));
+
+  dict.SetKeyValue("rob", "short_value");
+
+  EXPECT_EQ(1u, dict.GetCount());
+  EXPECT_EQ("short_value", dict.GetValueForKey("rob"));
+}
+
+TEST(LongStringDictionary, OverrideShortValue) {
+  LongStringDictionary dict;
+
+  dict.SetKeyValue("rob", "short_value");
+
+  EXPECT_EQ(1u, dict.GetCount());
+  EXPECT_EQ("short_value", dict.GetValueForKey("rob"));
+
+  string long_value = string(255 * 10, 'x');
+  dict.SetKeyValue("rob", long_value.c_str());
+
+  EXPECT_EQ(10u, dict.GetCount());
+  EXPECT_EQ(long_value, dict.GetValueForKey("rob"));
+}
+
+} // namespace google_breakpad
diff --git a/src/common/simple_string_dictionary.h b/src/common/simple_string_dictionary.h
index d2ab17fd..28c4bf1c 100644
--- a/src/common/simple_string_dictionary.h
+++ b/src/common/simple_string_dictionary.h
@@ -209,20 +209,22 @@ class NonAllocatingMap {
 
   // Given |key|, removes any associated value. |key| must not be NULL. If
   // the key is not found, this is a noop.
-  void RemoveKey(const char* key) {
+  bool RemoveKey(const char* key) {
     assert(key);
     if (!key)
-      return;
+      return false;
 
     Entry* entry = GetEntryForKey(key);
     if (entry) {
       entry->key[0] = '\0';
       entry->value[0] = '\0';
+      return true;
     }
 
 #ifndef NDEBUG
     assert(GetEntryForKey(key) == NULL);
 #endif
+    return false;
   }
 
   // Places a serialized version of the map into |map| and returns the size.