commit 3d14cae18da2bf58bd111533b8ce95d956b4735c
author Ben Chan <benchan@chromium.org> Mon Mar 03 19:23:45 2014
committer chrome-internal-fetch <chrome-internal-fetch@google.com> Tue Mar 04 04:41:32 2014
tree 96bf446d8c5e6f415c8fd472a7c1553d772d5d5a
parent e8d90d92eec47230de20697f07da16ceb585ea5a

Prevent ProcessTest::ResetPidByFile from killing any innocent process.

BUG=chromium:308196
TEST=Tested the following:
1. FEATURES=test emerge-$BOARD platform2
2. Run libchromeos-180609_unittests and libchromeos-242728_unittests
   under strace and verify that the ProcessTest::ResetPidByFile unit
   test doesn't send SIGKILL to a random process with pid 456.

Change-Id: If304c1ad1aa64eddf68304438d03325c28732c35
Reviewed-on: https://chromium-review.googlesource.com/188517
Tested-by: Ben Chan <benchan@chromium.org>
Reviewed-by: David James <davidjames@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>
Commit-Queue: Ben Chan <benchan@chromium.org>

chromeos/process.h

diff --git a/chromeos/process.h b/chromeos/process.h
index 7e294fc..36a1c85 100644
--- a/chromeos/process.h
+++ b/chromeos/process.h
@@ -20,6 +20,7 @@
 #include <base/string_util.h>
 #include <base/stringprintf.h>
 #endif
+#include <gtest/gtest_prod.h>
 
 #if BASE_VER >= 242728
 using base::StringPrintf;
@@ -167,6 +168,8 @@
   bool PopulatePipeMap();
 
  private:
+  FRIEND_TEST(ProcessTest, ResetPidByFile);
+
   // Pid of currently managed process or 0 if no currently managed
   // process.  pid must not be modified except by calling
   // UpdatePid(new_pid).

chromeos/process_test.cc

diff --git a/chromeos/process_test.cc b/chromeos/process_test.cc
index 483051b..5cc1628 100644
--- a/chromeos/process_test.cc
+++ b/chromeos/process_test.cc
@@ -35,16 +35,12 @@
 static const char kBinSleep[] = "/bin/sleep";
 static const char kBinTrue[] = "/bin/true";
 
-using chromeos::Process;
-using chromeos::ProcessImpl;
-using chromeos::ClearLog;
-using chromeos::FindLog;
-using chromeos::GetLog;
+namespace chromeos {
 
 // Test that the mock has all the functions of the interface by
 // instantiating it.  This variable is not used elsewhere.
 struct CompileMocks {
-  chromeos::ProcessMock process_mock;
+  ProcessMock process_mock;
 };
 
 TEST(SimpleProcess, Basic) {
@@ -83,7 +79,7 @@
 #endif
     CreateDirectory(test_path_);
     process_.RedirectOutput(output_file_);
-    chromeos::ClearLog();
+    ClearLog();
   }
 
   void TearDown() {
@@ -302,6 +298,11 @@
   EXPECT_TRUE(file_util::WriteFile(pid_path, "456\n", 4));
   EXPECT_TRUE(process_.ResetPidByFile(pid_path.value()));
   EXPECT_EQ(456, process_.pid());
+  // The purpose of this unit test is to check if Process::ResetPidByFile() can
+  // properly read a pid from a file. We don't really want to kill the process
+  // with pid 456, so update the pid to 0 to prevent the Process destructor from
+  // killing any innocent process.
+  process_.UpdatePid(0);
 }
 
 TEST_F(ProcessTest, KillSleeper) {
@@ -328,3 +329,5 @@
   process_.SetPreExecCallback(base::Bind(&ReturnFalse));
   ASSERT_NE(0, process_.Run());
 }
+
+}  // namespace chromeos