Simplify and optimize String::format.

Simplify and optimize String::format.

base::vsnprintf already handles the windows difference so we don't need the
platform ifdefs. We can also optimistically format into the buffer and if it
fits avoid formatting twice.

Review-Url: https://codereview.chromium.org/2620453002
Cr-Commit-Position: refs/heads/master@{#442084}
Committed: https://chromium.googlesource.com/chromium/src/+/8b39bd3c54837ff1bf5890fdf47ee21cf9b39398

[esprehn, 2017-01-06 02:55:43]

The CQ bit was checked by esprehn@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-06 02:56:03]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[esprehn, 2017-01-06 02:56:51]

Description was changed from

==========
Simplify String::format using base::vsnprintf.

base::vsnprintf already handles the windows difference so we don't need
the platform ifdefs. We can also optimistically format into the buffer
and if it fits avoid formatting twice.
==========

to

==========
Simplify and optimize String::format.

base::vsnprintf already handles the windows difference so we don't need the
platform ifdefs. We can also optimistically format into the buffer and if it
fits avoid formatting twice.
==========

[esprehn, 2017-01-06 03:56:29]

esprehn@chromium.org changed reviewers:
+ csharrison@chromium.org, yutak@chromium.org

[commit-bot: I haz the power, 2017-01-06 05:15:51]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-06 05:15:52]

Dry run: This issue passed the CQ dry run.

[Charlie Harrison, 2017-01-06 16:41:02]

Could we create an uninitialized StringImpl and format directly into the Impl's
buffer to avoid a copy / allocation?

[esprehn, 2017-01-06 21:11:13]

On 2017/01/06 at 16:41:02, csharrison wrote:
> Could we create an uninitialized StringImpl and format directly into the
Impl's buffer to avoid a copy / allocation?

No, because we don't know the size of the string to allocate. In practice this
code isn't used anywhere especially hot so one extra allocation isn't too bad,
it's the same as what's happening inside StringBuilder.

[Charlie Harrison, 2017-01-06 21:13:55]

Makes sense. LGTM with the caveat that I'm pretty unfamiliar with the
va_start/end/list macros.

[esprehn, 2017-01-06 22:47:06]

The CQ bit was checked by esprehn@chromium.org

[commit-bot: I haz the power, 2017-01-06 22:47:40]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-06 22:52:28]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1483742826863440,
"parent_rev": "f848b67b1486e0aba0ddcfea7e492062ba07064f", "commit_rev":
"8b39bd3c54837ff1bf5890fdf47ee21cf9b39398"}

[commit-bot: I haz the power, 2017-01-06 22:52:56]

Description was changed from

==========
Simplify and optimize String::format.

base::vsnprintf already handles the windows difference so we don't need the
platform ifdefs. We can also optimistically format into the buffer and if it
fits avoid formatting twice.
==========

to

==========
Simplify and optimize String::format.

base::vsnprintf already handles the windows difference so we don't need the
platform ifdefs. We can also optimistically format into the buffer and if it
fits avoid formatting twice.

Review-Url: https://codereview.chromium.org/2620453002
Cr-Commit-Position: refs/heads/master@{#442084}
Committed:
https://chromium.googlesource.com/chromium/src/+/8b39bd3c54837ff1bf5890fdf47e...
==========

[commit-bot: I haz the power, 2017-01-06 22:52:57]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/8b39bd3c54837ff1bf5890fdf47e...