Use a class instead of several separate globals in crash_logging.cc.

Use a class instead of several separate globals in crash_logging.cc.

There should be no behavior change; this is just refactoring.

Committed: https://crrev.com/5e57bcecf023ec05c9c2343b2b636f3d7e0cf1f1
Cr-Commit-Position: refs/heads/master@{#350701}

[Robert Sesek, 2015-09-24 17:12:57]

Patchset #1 (id:1) has been deleted

[Robert Sesek, 2015-09-24 20:44:50]

rsesek@chromium.org changed reviewers:
+ danakj@chromium.org

[Robert Sesek, 2015-09-24 20:44:50]

danakj: Please review some perfcrastination-induced refactoring :-)

[danakj, 2015-09-24 21:06:47]

https://codereview.chromium.org/1368703002/diff/40001/base/debug/crash_loggin...
File base/debug/crash_logging.cc (right):

https://codereview.chromium.org/1368703002/diff/40001/base/debug/crash_loggin...
base/debug/crash_logging.cc:76: return std::ceil(length /
static_cast<float>(chunk_max_length_));
This is kinda less good than before, I think. std::ceil will return a float,
which is being implicitly converted to a size_t (cuz warnings for this are
disabled in this directory cuz it happens a lot still).

To fix this we'd do a checked_cast back to a size_t. But floats and size_ts are
really different.

I'd prefer to just do the + - trick like before and everything stays size_ts.

[danakj, 2015-09-24 21:08:36]

LG otherwise

https://codereview.chromium.org/1368703002/diff/40001/base/debug/crash_loggin...
File base/debug/crash_logging_unittest.cc (right):

https://codereview.chromium.org/1368703002/diff/40001/base/debug/crash_loggin...
base/debug/crash_logging_unittest.cc:44: base::ShadowingAtExitManager
at_exit_manager_;
Can you leave a comment explaining why this is here?

https://codereview.chromium.org/1368703002/diff/40001/chrome/common/crash_key...
File chrome/common/crash_keys_unittest.cc (right):

https://codereview.chromium.org/1368703002/diff/40001/chrome/common/crash_key...
chrome/common/crash_keys_unittest.cc:56: base::ShadowingAtExitManager
at_exit_manager_;
ditto

[Robert Sesek, 2015-09-24 21:28:20]

https://codereview.chromium.org/1368703002/diff/40001/base/debug/crash_loggin...
File base/debug/crash_logging.cc (right):

https://codereview.chromium.org/1368703002/diff/40001/base/debug/crash_loggin...
base/debug/crash_logging.cc:76: return std::ceil(length /
static_cast<float>(chunk_max_length_));
On 2015/09/24 21:06:47, danakj wrote:
> This is kinda less good than before, I think. std::ceil will return a float,
> which is being implicitly converted to a size_t (cuz warnings for this are
> disabled in this directory cuz it happens a lot still).
> 
> To fix this we'd do a checked_cast back to a size_t. But floats and size_ts
are
> really different.
> 
> I'd prefer to just do the + - trick like before and everything stays size_ts.

Ah, you're right.
https://chromium.googlesource.com/chromium/src/+/9cf9b94a63dec7653bb48bac948d...
explains that.

https://codereview.chromium.org/1368703002/diff/40001/base/debug/crash_loggin...
File base/debug/crash_logging_unittest.cc (right):

https://codereview.chromium.org/1368703002/diff/40001/base/debug/crash_loggin...
base/debug/crash_logging_unittest.cc:44: base::ShadowingAtExitManager
at_exit_manager_;
On 2015/09/24 21:08:36, danakj wrote:
> Can you leave a comment explaining why this is here?

Done.

https://codereview.chromium.org/1368703002/diff/40001/chrome/common/crash_key...
File chrome/common/crash_keys_unittest.cc (right):

https://codereview.chromium.org/1368703002/diff/40001/chrome/common/crash_key...
chrome/common/crash_keys_unittest.cc:56: base::ShadowingAtExitManager
at_exit_manager_;
On 2015/09/24 21:08:36, danakj wrote:
> ditto

Done.

[danakj, 2015-09-24 21:49:31]

LGTM

[Robert Sesek, 2015-09-24 21:50:07]

The CQ bit was checked by rsesek@chromium.org

[commit-bot: I haz the power, 2015-09-24 21:51:37]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1368703002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1368703002/60001

[commit-bot: I haz the power, 2015-09-24 23:30:58]

Committed patchset #3 (id:60001)

[commit-bot: I haz the power, 2015-09-24 23:35:03]

Patchset 3 (id:??) landed as
https://crrev.com/5e57bcecf023ec05c9c2343b2b636f3d7e0cf1f1
Cr-Commit-Position: refs/heads/master@{#350701}

[zhaoqin1, 2015-09-25 15:14:28]

A revert of this CL (patchset #3 id:60001) has been created in
https://codereview.chromium.org/1360943004/ by zhaoqin@chromium.org.

The reason for reverting is: This CL is the culprit of an uninit read and
several test crash on Valgrind bots.

BUG=535990
.