Fix ProcessOutputWatcherTest.OutputWatcher under ASAN

chrome/browser/chromeos/process_proxy/process_output_watcher_unittest.cc

Index: chrome/browser/chromeos/process_proxy/process_output_watcher_unittest.cc
diff --git a/chrome/browser/chromeos/process_proxy/process_output_watcher_unittest.cc b/chrome/browser/chromeos/process_proxy/process_output_watcher_unittest.cc
index 778e905bdc2c50bb0e25e4b83b3ed2ce455a4455..b81da9065e7f5d341d657fe7afee73aa3b3814f6 100644
--- a/chrome/browser/chromeos/process_proxy/process_output_watcher_unittest.cc
+++ b/chrome/browser/chromeos/process_proxy/process_output_watcher_unittest.cc
@@ -19,17 +19,12 @@
 
 struct TestCase {
   std::string str;
-  ProcessOutputType type;
+  bool should_send_terminating_null;
 
-  TestCase(const char* expected_string,
-           size_t expected_string_length,
-           ProcessOutputType expected_type)
-      : str(expected_string, expected_string_length),
-        type(expected_type) {
-  }
-  TestCase(const std::string& expected_string, ProcessOutputType expected_type)
+  TestCase(const std::string& expected_string,
+           bool send_terminating_null)
       : str(expected_string),
-        type(expected_type) {
+        should_send_terminating_null(send_terminating_null) {
   }
 };
 
@@ -41,8 +36,9 @@ class ProcessWatcherExpectations {
     received_from_out_ = 0;
 
     for (size_t i = 0; i < expectations.size(); i++) {
-      out_expectations_.append(expectations[i].str.c_str(),
-                               expectations[i].str.length());
+      out_expectations_.append(expectations[i].str);
+      if (expectations[i].should_send_terminating_null)
+        out_expectations_.append(std::string("", 1));
     }
   }
 
@@ -97,6 +93,44 @@ public:
     return result;
   }
 
+  void RunTest(const std::vector<TestCase>& test_cases) {
+    all_data_received_.reset(new base::WaitableEvent(true, false));
+
+    base::Thread output_watch_thread("ProcessOutpuWatchThread");
+    ASSERT_TRUE(output_watch_thread.Start());
+
+    int pt_pipe[2], stop_pipe[2];
+    ASSERT_FALSE(HANDLE_EINTR(pipe(pt_pipe)));
+    ASSERT_FALSE(HANDLE_EINTR(pipe(stop_pipe)));
+
+    output_watch_thread.message_loop()->PostTask(FROM_HERE,
+        base::Bind(&ProcessOutputWatcherTest::StartWatch,
+                   base::Unretained(this),
+                   pt_pipe[0], stop_pipe[0], test_cases));
+
+    for (size_t i = 0; i < test_cases.size(); i++) {
+      const std::string& test_str = test_cases[i].str;
+      // Let's make inputs not NULL terminated, unless other is specified in
+      // the test case.
+      ssize_t test_size = test_str.length() * sizeof(*test_str.c_str());
+      if (test_cases[i].should_send_terminating_null)
+        test_size += sizeof(*test_str.c_str());
+      EXPECT_EQ(test_size,
+                file_util::WriteFileDescriptor(pt_pipe[1], test_str.c_str(),
+                                               test_size));
+    }
+
+    all_data_received_->Wait();
+
+    // Send stop signal. It is not important which string we send.
+    EXPECT_EQ(1, file_util::WriteFileDescriptor(stop_pipe[1], "q", 1));
+
+    EXPECT_NE(-1, HANDLE_EINTR(close(stop_pipe[1])));
+    EXPECT_NE(-1, HANDLE_EINTR(close(pt_pipe[1])));
+
+    output_watch_thread.Stop();
+  }
+
   scoped_ptr<base::WaitableEvent> all_data_received_;
 
  private:
@@ -106,50 +140,25 @@ public:
 
 
 TEST_F(ProcessOutputWatcherTest, OutputWatcher) {
-  all_data_received_.reset(new base::WaitableEvent(true, false));
-
-  base::Thread output_watch_thread("ProcessOutpuWatchThread");
-  ASSERT_TRUE(output_watch_thread.Start());
-
-  int pt_pipe[2], stop_pipe[2];
-  ASSERT_FALSE(HANDLE_EINTR(pipe(pt_pipe)));
-  ASSERT_FALSE(HANDLE_EINTR(pipe(stop_pipe)));
-
-  // TODO(tbarzic): We don't support stderr anymore, so this can be simplified.
   std::vector<TestCase> test_cases;
-  test_cases.push_back(TestCase("testing output\n", PROCESS_OUTPUT_TYPE_OUT));
-  test_cases.push_back(TestCase("testing error\n", PROCESS_OUTPUT_TYPE_OUT));
-  test_cases.push_back(TestCase("testing error1\n", PROCESS_OUTPUT_TYPE_OUT));
-  test_cases.push_back(TestCase("testing output1\n", PROCESS_OUTPUT_TYPE_OUT));
-  test_cases.push_back(TestCase("testing output2\n", PROCESS_OUTPUT_TYPE_OUT));
-  test_cases.push_back(TestCase("testing output3\n", PROCESS_OUTPUT_TYPE_OUT));
-  test_cases.push_back(TestCase(VeryLongString(), PROCESS_OUTPUT_TYPE_OUT));
-  test_cases.push_back(TestCase("testing error2\n", PROCESS_OUTPUT_TYPE_OUT));
-  test_cases.push_back(TestCase("line with \0 in it\n",
-                                arraysize("line with \0 in it \n"),
-                                PROCESS_OUTPUT_TYPE_OUT));
-
-  output_watch_thread.message_loop()->PostTask(FROM_HERE,
-      base::Bind(&ProcessOutputWatcherTest::StartWatch, base::Unretained(this),
-                 pt_pipe[0], stop_pipe[0], test_cases));
-
-  for (size_t i = 0; i < test_cases.size(); i++) {
-    // Let's make inputs not NULL terminated.
-    const std::string& test_str = test_cases[i].str;
-    ssize_t test_size = test_str.length() * sizeof(*test_str.c_str());
-    EXPECT_EQ(test_size,
-              file_util::WriteFileDescriptor(pt_pipe[1], test_str.c_str(),
-                                             test_size));
-  }
-
-  all_data_received_->Wait();
-
-  // Send stop signal. It is not important which string we send.
-  EXPECT_EQ(1, file_util::WriteFileDescriptor(stop_pipe[1], "q", 1));
+  test_cases.push_back(TestCase("testing output\n", false));
+  test_cases.push_back(TestCase("testing error\n", false));
+  test_cases.push_back(TestCase("testing error1\n", false));
+  test_cases.push_back(TestCase("testing output1\n", false));
+  test_cases.push_back(TestCase("testing output2\n", false));
+  test_cases.push_back(TestCase("testing output3\n", false));
+  test_cases.push_back(TestCase(VeryLongString(), false));
+  test_cases.push_back(TestCase("testing error2\n", false));
+
+  RunTest(test_cases);
+};
 
-  EXPECT_NE(-1, HANDLE_EINTR(close(stop_pipe[1])));
-  EXPECT_NE(-1, HANDLE_EINTR(close(pt_pipe[1])));
+// Verifies that sending '\0' generates PROCESS_OUTPUT_TYPE_OUT event and does
+// not terminate output watcher.
+TEST_F(ProcessOutputWatcherTest, SendNull) {
+  std::vector<TestCase> test_cases;
+  test_cases.push_back(TestCase("", true));
+  test_cases.push_back(TestCase("a", true));

[oshima, 2012/04/10 18:55:01]

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.

 
-  output_watch_thread.Stop();
+  RunTest(test_cases);
 };
-