Fix ProcessOutputWatcherTest.OutputWatcher under ASAN

Fix ProcessOutputWatcherTest.OutputWatcher under ASAN

String length was being set using arraysize of string that had an extra character.

BUG=122700
TEST=ProcessOutputWatcher.OutputWatcher under ASAN

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

[oshima, 2012-04-10 00:58:11]

http://codereview.chromium.org/10033015/diff/3001/chrome/browser/chromeos/pro...
File chrome/browser/chromeos/process_proxy/process_output_watcher_unittest.cc
(right):

http://codereview.chromium.org/10033015/diff/3001/chrome/browser/chromeos/pro...
chrome/browser/chromeos/process_proxy/process_output_watcher_unittest.cc:131:
arraysize(line_with_null_char) - 1,
Maybe I'm dumb, but sorry I didn't understand why you need -1. Can you
elaborate?

[tonibarzic, 2012-04-10 01:16:59]

http://codereview.chromium.org/10033015/diff/3001/chrome/browser/chromeos/pro...
File chrome/browser/chromeos/process_proxy/process_output_watcher_unittest.cc
(right):

http://codereview.chromium.org/10033015/diff/3001/chrome/browser/chromeos/pro...
chrome/browser/chromeos/process_proxy/process_output_watcher_unittest.cc:131:
arraysize(line_with_null_char) - 1,
On 2012/04/10 00:58:12, oshima wrote:
> Maybe I'm dumb, but sorry I didn't understand why you need -1. Can you
> elaborate?

I don't necessary need -1, but it's more in line with the comment at line 139
(ProcessOutputWatcher will not generally be sent NULL terminated strings in
wild)

[tbarzic, 2012-04-10 17:28:24]

Hey oshima, can you take another look at this.
I modified the test not to use c_str() on the string with embeded '\0'

Both tests work fine under asan.

https://chromiumcodereview.appspot.com/10033015/diff/6/chrome/browser/chromeo...
File chrome/browser/chromeos/process_proxy/process_output_watcher_unittest.cc
(right):

https://chromiumcodereview.appspot.com/10033015/diff/6/chrome/browser/chromeo...
chrome/browser/chromeos/process_proxy/process_output_watcher_unittest.cc:108:
base::Unretained(this),
this code is the same as the code removed form
ProcessOutputWatcher.OutputWatcher test (module setting up test_cases)

https://chromiumcodereview.appspot.com/10033015/diff/6/chrome/browser/chromeo...
chrome/browser/chromeos/process_proxy/process_output_watcher_unittest.cc:157: //
not terminate output watcher.
this test verifies the behaviour "line with \0 in it\n" test case used to, but
without using c_str on string with embeded \0

[tbarzic, 2012-04-10 17:30:05]

https://chromiumcodereview.appspot.com/10033015/diff/6/chrome/browser/chromeo...
File chrome/browser/chromeos/process_proxy/process_output_watcher_unittest.cc
(left):

https://chromiumcodereview.appspot.com/10033015/diff/6/chrome/browser/chromeo...
chrome/browser/chromeos/process_proxy/process_output_watcher_unittest.cc:118: //
TODO(tbarzic): We don't support stderr anymore, so this can be simplified.
I also addressed this todo by removing PROCESS_OUTPUT_TYPE from test cases (it
wasn't used anyway)

[oshima, 2012-04-10 18:23:03]

http://codereview.chromium.org/10033015/diff/6/chrome/browser/chromeos/proces...
File chrome/browser/chromeos/process_proxy/process_output_watcher_unittest.cc
(right):

http://codereview.chromium.org/10033015/diff/6/chrome/browser/chromeos/proces...
chrome/browser/chromeos/process_proxy/process_output_watcher_unittest.cc:161:
test_cases.push_back(TestCase("a", false));
true?

[tbarzic, 2012-04-10 18:46:40]

https://chromiumcodereview.appspot.com/10033015/diff/6/chrome/browser/chromeo...
File chrome/browser/chromeos/process_proxy/process_output_watcher_unittest.cc
(right):

https://chromiumcodereview.appspot.com/10033015/diff/6/chrome/browser/chromeo...
chrome/browser/chromeos/process_proxy/process_output_watcher_unittest.cc:161:
test_cases.push_back(TestCase("a", false));
On 2012/04/10 18:23:03, oshima wrote:
> true?

It's not crucial (as long as there is \0 in previous test case), but yeah, I can
put true :)

[oshima, 2012-04-10 18:55:00]

LGTM

https://chromiumcodereview.appspot.com/10033015/diff/7001/chrome/browser/chro...
File chrome/browser/chromeos/process_proxy/process_output_watcher_unittest.cc
(right):

https://chromiumcodereview.appspot.com/10033015/diff/7001/chrome/browser/chro...
chrome/browser/chromeos/process_proxy/process_output_watcher_unittest.cc:161:
test_cases.push_back(TestCase("a", true));
If you intention was to send null, and then make sure next one works (with
false), please mention that in the comment. It's not so obvious and whoever may
have to maintain this code in the future will get confused.