Refactor the breakpad Windows key/value stash, step one.

Refactor the breakpad Windows key/value stash, step one.
This is a minimal diff that refactors the key/value stash into a class.
The next step is to pull the class implementation out to a separate
implementation file and add a unittest.
Also normalizes the use of the anonymous namespace to exclude functions
that are DLL exports.

BUG=378916
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=273953

[Sigurður Ásgeirsson, 2014-05-29 20:26:28]

Yo, PTAL!

[Robert Sesek, 2014-05-29 20:34:41]

https://codereview.chromium.org/302033002/diff/1/components/breakpad/app/brea...
File components/breakpad/app/breakpad_win.cc (right):

https://codereview.chromium.org/302033002/diff/1/components/breakpad/app/brea...
components/breakpad/app/breakpad_win.cc:52: class KeyValuePairKeeper {
Not quite a fan of this name. Perhaps CustomInfoMap (since it contains
CustomInfoEntry's)? Or CrashKeysWin?

https://codereview.chromium.org/302033002/diff/1/components/breakpad/app/brea...
components/breakpad/app/breakpad_win.cc:67: // Sets private
Incomplete comment?

https://codereview.chromium.org/302033002/diff/1/components/breakpad/app/brea...
components/breakpad/app/breakpad_win.cc:91: 
nit: remove blank line

https://codereview.chromium.org/302033002/diff/1/components/breakpad/app/brea...
components/breakpad/app/breakpad_win.cc:501: //    here, and an implicit
std::wstring conversion. Fixme.
Where's the NULL deref potential?

[Sigurður Ásgeirsson, 2014-05-30 15:14:49]

Thanks! Please take another look?

https://codereview.chromium.org/302033002/diff/1/components/breakpad/app/brea...
File components/breakpad/app/breakpad_win.cc (right):

https://codereview.chromium.org/302033002/diff/1/components/breakpad/app/brea...
components/breakpad/app/breakpad_win.cc:52: class KeyValuePairKeeper {
On 2014/05/29 20:34:41, rsesek wrote:
> Not quite a fan of this name. 
Yeah, me neither :/. You know of course that there are only two hard problems in
software development:
1. Cache eviction.
2. Naming.
3. One-offs.

> Perhaps CustomInfoMap (since it contains
> CustomInfoEntry's)? Or CrashKeysWin?

I like CrashKeysWin, so done, thanks.

https://codereview.chromium.org/302033002/diff/1/components/breakpad/app/brea...
components/breakpad/app/breakpad_win.cc:67: // Sets private
On 2014/05/29 20:34:41, rsesek wrote:
> Incomplete comment?

Done.

https://codereview.chromium.org/302033002/diff/1/components/breakpad/app/brea...
components/breakpad/app/breakpad_win.cc:91: 
On 2014/05/29 20:34:41, rsesek wrote:
> nit: remove blank line

Done.

https://codereview.chromium.org/302033002/diff/1/components/breakpad/app/brea...
components/breakpad/app/breakpad_win.cc:501: //    here, and an implicit
std::wstring conversion. Fixme.
On 2014/05/29 20:34:41, rsesek wrote:
> Where's the NULL deref potential?

If there's NULL on the input, std::string will try to deref it, I believe. As
this is an exported function, it might be better to be defensive here, but as-is
this behaves much as before.

[Robert Sesek, 2014-05-30 15:28:56]

lgtm

[Sigurður Ásgeirsson, 2014-05-30 18:21:41]

The CQ bit was checked by siggi@chromium.org

[commit-bot: I haz the power, 2014-05-30 18:24:33]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/siggi@chromium.org/302033002/50001

[commit-bot: I haz the power, 2014-05-30 23:20:48]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  android_aosp on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_aosp/builds/7...)

[commit-bot: I haz the power, 2014-05-31 00:12:16]

Change committed as 273953