Refactor der::Input helper methods into new constructors

Refactor der::Input helper methods into new constructors

Committed: https://crrev.com/f5c6253ac16f742fadffe210795d50b9344383ae
Cr-Commit-Position: refs/heads/master@{#369311}

[nharper, 2016-01-13 22:34:24]

Add documentation

[nharper, 2016-01-13 22:35:26]

nharper@chromium.org changed reviewers:
+ eroman@chromium.org

[eroman, 2016-01-13 22:54:01]

lgtm

https://codereview.chromium.org/1573243011/diff/20001/net/der/input.cc
File net/der/input.cc (right):

https://codereview.chromium.org/1573243011/diff/20001/net/der/input.cc#newcode26
net/der/input.cc:26: : data_(reinterpret_cast<const uint8_t*>(s->data())),
len_(s->size()) {}
Could also delegate to stringpiece ctor
   : Input(StringPiece(*s))

https://codereview.chromium.org/1573243011/diff/20001/net/der/input.h
File net/der/input.h (right):

https://codereview.chromium.org/1573243011/diff/20001/net/der/input.h#newcode56
net/der/input.h:56: Input(const base::StringPiece& sp);
Mark as explicit

https://codereview.chromium.org/1573243011/diff/20001/net/der/input.h#newcode61
net/der/input.h:61: Input(const std::string* s);
explicit

https://codereview.chromium.org/1573243011/diff/20001/net/der/input.h#newcode79
net/der/input.h:79: // StringPiece cannot outlive the data that was used to
construct this Input.
nit: cannot --> must not

https://codereview.chromium.org/1573243011/diff/20001/net/der/input.h#newcode88
net/der/input.h:88: Input(std::string) = delete;
Does this need to be const std::string& ?

https://codereview.chromium.org/1573243011/diff/20001/net/socket/ssl_client_s...
File net/socket/ssl_client_socket_unittest.cc (right):

https://codereview.chromium.org/1573243011/diff/20001/net/socket/ssl_client_s...
net/socket/ssl_client_socket_unittest.cc:2386: der::Parser
parser((der::Input(input)));
yay c++ parsing!

[nharper, 2016-01-13 23:04:28]

fix nits

[nharper, 2016-01-13 23:04:48]

https://codereview.chromium.org/1573243011/diff/20001/net/der/input.cc
File net/der/input.cc (right):

https://codereview.chromium.org/1573243011/diff/20001/net/der/input.cc#newcode26
net/der/input.cc:26: : data_(reinterpret_cast<const uint8_t*>(s->data())),
len_(s->size()) {}
On 2016/01/13 22:54:00, eroman wrote:
> Could also delegate to stringpiece ctor
>    : Input(StringPiece(*s))

That looks a little cleaner. Done.

https://codereview.chromium.org/1573243011/diff/20001/net/der/input.h
File net/der/input.h (right):

https://codereview.chromium.org/1573243011/diff/20001/net/der/input.h#newcode56
net/der/input.h:56: Input(const base::StringPiece& sp);
On 2016/01/13 22:54:00, eroman wrote:
> Mark as explicit

Done.

https://codereview.chromium.org/1573243011/diff/20001/net/der/input.h#newcode61
net/der/input.h:61: Input(const std::string* s);
On 2016/01/13 22:54:00, eroman wrote:
> explicit

Done.

https://codereview.chromium.org/1573243011/diff/20001/net/der/input.h#newcode79
net/der/input.h:79: // StringPiece cannot outlive the data that was used to
construct this Input.
On 2016/01/13 22:54:00, eroman wrote:
> nit: cannot --> must not

Done.

https://codereview.chromium.org/1573243011/diff/20001/net/der/input.h#newcode88
net/der/input.h:88: Input(std::string) = delete;
On 2016/01/13 22:54:00, eroman wrote:
> Does this need to be const std::string& ?

I tested the following, and they all failed to compile:
Input test("test");
---
std::string t = "test";
Input test(t);
---
std::string TestString() { return "hello"; }
Input test(TestString())

What sort of things should I try that might slip past Input(std::string) but not
Input(const std::string&) ?

https://codereview.chromium.org/1573243011/diff/20001/net/socket/ssl_client_s...
File net/socket/ssl_client_socket_unittest.cc (right):

https://codereview.chromium.org/1573243011/diff/20001/net/socket/ssl_client_s...
net/socket/ssl_client_socket_unittest.cc:2386: der::Parser
parser((der::Input(input)));
On 2016/01/13 22:54:00, eroman wrote:
> yay c++ parsing!

Yay indeed :P
There were a few other vexing parses that needed extra parens in this cleanup.

[eroman, 2016-01-13 23:21:06]

lgtm

https://codereview.chromium.org/1573243011/diff/20001/net/der/input.h
File net/der/input.h (right):

https://codereview.chromium.org/1573243011/diff/20001/net/der/input.h#newcode88
net/der/input.h:88: Input(std::string) = delete;
On 2016/01/13 23:04:47, nharper wrote:
> On 2016/01/13 22:54:00, eroman wrote:
> > Does this need to be const std::string& ?
> 
> I tested the following, and they all failed to compile:
> Input test("test");
> ---
> std::string t = "test";
> Input test(t);
> ---
> std::string TestString() { return "hello"; }
> Input test(TestString())
> 
> What sort of things should I try that might slip past Input(std::string) but
not
> Input(const std::string&) ?

Sounds good. My only other test suggestions would be around the fact that it is
implicit ctor rather than explicit, but I don't anticipate problems there.

[nharper, 2016-01-13 23:26:23]

The CQ bit was checked by nharper@chromium.org

[commit-bot: I haz the power, 2016-01-13 23:29:13]

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

[commit-bot: I haz the power, 2016-01-14 01:40:52]

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

[commit-bot: I haz the power, 2016-01-14 01:40:53]

Try jobs failed on following builders:
  android_arm64_dbg_recipe on tryserver.chromium.android (JOB_TIMED_OUT, no
build URL)
  android_chromium_gn_compile_dbg on tryserver.chromium.android (JOB_TIMED_OUT,
no build URL)
  android_chromium_gn_compile_rel on tryserver.chromium.android (JOB_TIMED_OUT,
no build URL)
  android_clang_dbg_recipe on tryserver.chromium.android (JOB_TIMED_OUT, no
build URL)
  android_compile_dbg on tryserver.chromium.android (JOB_TIMED_OUT, no build
URL)
  cast_shell_android on tryserver.chromium.android (JOB_TIMED_OUT, no build URL)
  linux_android_rel_ng on tryserver.chromium.android (JOB_TIMED_OUT, no build
URL)
  cast_shell_linux on tryserver.chromium.linux (JOB_TIMED_OUT, no build URL)
  chromeos_amd64-generic_chromium_compile_only_ng on tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  chromeos_daisy_chromium_compile_only_ng on tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  chromeos_x86-generic_chromium_compile_only_ng on tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  chromium_presubmit on tryserver.chromium.linux (JOB_TIMED_OUT, no build URL)
  linux_chromium_asan_rel_ng on tryserver.chromium.linux (JOB_TIMED_OUT, no
build URL)
  linux_chromium_chromeos_compile_dbg_ng on tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  linux_chromium_chromeos_ozone_rel_ng on tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  linux_chromium_chromeos_rel_ng on tryserver.chromium.linux (JOB_TIMED_OUT, no
build URL)
  linux_chromium_clobber_rel_ng on tryserver.chromium.linux (JOB_TIMED_OUT, no
build URL)
  linux_chromium_compile_dbg_ng on tryserver.chromium.linux (JOB_TIMED_OUT, no
build URL)
  linux_chromium_gn_chromeos_rel on tryserver.chromium.linux (JOB_TIMED_OUT, no
build URL)
  linux_chromium_rel_ng on tryserver.chromium.linux (JOB_TIMED_OUT, no build
URL)
  mac_chromium_compile_dbg_ng on tryserver.chromium.mac (JOB_TIMED_OUT, no build
URL)
  mac_chromium_gn_rel on tryserver.chromium.mac (JOB_TIMED_OUT, no build URL)
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_TIMED_OUT, no build URL)

[nharper, 2016-01-14 01:46:33]

The CQ bit was checked by nharper@chromium.org

[commit-bot: I haz the power, 2016-01-14 01:47:56]

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

[commit-bot: I haz the power, 2016-01-14 03:42:31]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-01-14 03:43:24]

Description was changed from

==========
Refactor der::Input helper methods into new constructors
==========

to

==========
Refactor der::Input helper methods into new constructors

Committed: https://crrev.com/f5c6253ac16f742fadffe210795d50b9344383ae
Cr-Commit-Position: refs/heads/master@{#369311}
==========

[commit-bot: I haz the power, 2016-01-14 03:43:25]

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