base/threading: restrict to set only current thread priority

base/threading/platform_thread_unittest.cc

Index: base/threading/platform_thread_unittest.cc
diff --git a/base/threading/platform_thread_unittest.cc b/base/threading/platform_thread_unittest.cc
index c4b3d5d7ecc6092a89ca519800b55f92d3fd77da..be057ba7596f7c4e87e638ac1426e1c7d172973a 100644
--- a/base/threading/platform_thread_unittest.cc
+++ b/base/threading/platform_thread_unittest.cc
@@ -8,7 +8,10 @@
 #include "base/threading/platform_thread.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
-#if defined(OS_WIN)
+#if defined(OS_POSIX)
+#include <sys/types.h>
+#include <unistd.h>
+#elif defined(OS_WIN)
 #include <windows.h>
 #endif
 
@@ -16,6 +19,8 @@ namespace base {
 
 // Trivial tests that thread runs and doesn't crash on create and join ---------
 
+namespace {
+
 class TrivialThread : public PlatformThread::Delegate {
  public:
   TrivialThread() : did_run_(false) {}
@@ -30,6 +35,8 @@ class TrivialThread : public PlatformThread::Delegate {
   DISALLOW_COPY_AND_ASSIGN(TrivialThread);
 };
 
+}  // namespace
+
 TEST(PlatformThreadTest, Trivial) {
   TrivialThread thread;
   PlatformThreadHandle handle;
@@ -56,11 +63,13 @@ TEST(PlatformThreadTest, TrivialTimesTen) {
 
 // Tests of basic thread functions ---------------------------------------------
 
+namespace {
+
 class FunctionTestThread : public PlatformThread::Delegate {
  public:
   FunctionTestThread()
       : thread_id_(kInvalidThreadId),
-        thread_started_(true, false),
+        termination_ready_(true, false),
         terminate_thread_(true, false),
         done_(false) {}
   ~FunctionTestThread() override {
@@ -70,8 +79,8 @@ class FunctionTestThread : public PlatformThread::Delegate {
         << "WaitableEvent blocking the underlying thread's main.";
   }
 
-  // Grabs |thread_id_|, signals |thread_started_|, and then waits for
-  // |terminate_thread_| to be signaled before exiting.
+  // Grabs |thread_id_|, runs tests, signals |termination_ready_|, and then

[Lei Zhang, 2015/07/14 07:54:20]

s/runs tests/runs an optional test on that thread/ ?

[Takashi Toyoshima, 2015/07/14 08:03:23]

Done.

+  // waits for |terminate_thread_| to be signaled before exiting.
   void ThreadMain() override {
     thread_id_ = PlatformThread::CurrentId();
     EXPECT_NE(thread_id_, kInvalidThreadId);
@@ -79,39 +88,45 @@ class FunctionTestThread : public PlatformThread::Delegate {
     // Make sure that the thread ID is the same across calls.
     EXPECT_EQ(thread_id_, PlatformThread::CurrentId());
 
-    thread_started_.Signal();
+    // Run extra tests.
+    RunTest();
 
+    termination_ready_.Signal();
     terminate_thread_.Wait();
 
     done_ = true;
   }
 
   PlatformThreadId thread_id() const {
-    EXPECT_TRUE(thread_started_.IsSignaled()) << "Thread ID still unknown";
+    EXPECT_TRUE(termination_ready_.IsSignaled()) << "Thread ID still unknown";
     return thread_id_;
   }
 
   bool IsRunning() const {
-    return thread_started_.IsSignaled() && !done_;
+    return termination_ready_.IsSignaled() && !done_;
   }
 
-  // Blocks until this thread is started.
-  void WaitForThreadStart() { thread_started_.Wait(); }
+  // Blocks until this thread is started and ready to be terminated.
+  void WaitForTerminationReady() { termination_ready_.Wait(); }
 
   // Mark this thread for termination (callers must then join this thread to be
   // guaranteed of termination).
   void MarkForTermination() { terminate_thread_.Signal(); }
 
+  virtual void RunTest() {}

[Lei Zhang, 2015/07/14 07:54:20]

Add a brief comment about running an optional test on the newly created thread.

[Lei Zhang, 2015/07/14 07:54:20]

Make this private since derived classes should only be able to implement it but
not call it?

[Takashi Toyoshima, 2015/07/14 08:03:23]

Done.

[Takashi Toyoshima, 2015/07/14 08:03:23]

Done.

+
  private:
   PlatformThreadId thread_id_;
 
-  mutable WaitableEvent thread_started_;
+  mutable WaitableEvent termination_ready_;
   WaitableEvent terminate_thread_;
   bool done_;
 
   DISALLOW_COPY_AND_ASSIGN(FunctionTestThread);
 };
 
+}  // namespace
+
 TEST(PlatformThreadTest, Function) {
   PlatformThreadId main_thread_id = PlatformThread::CurrentId();
 
@@ -120,7 +135,7 @@ TEST(PlatformThreadTest, Function) {
 
   ASSERT_FALSE(thread.IsRunning());
   ASSERT_TRUE(PlatformThread::Create(0, &thread, &handle));
-  thread.WaitForThreadStart();
+  thread.WaitForTerminationReady();
   ASSERT_TRUE(thread.IsRunning());
   EXPECT_NE(thread.thread_id(), main_thread_id);
 
@@ -144,7 +159,7 @@ TEST(PlatformThreadTest, FunctionTimesTen) {
   for (size_t n = 0; n < arraysize(thread); n++)
     ASSERT_TRUE(PlatformThread::Create(0, &thread[n], &handle[n]));
   for (size_t n = 0; n < arraysize(thread); n++)
-    thread[n].WaitForThreadStart();
+    thread[n].WaitForTerminationReady();
 
   for (size_t n = 0; n < arraysize(thread); n++) {
     ASSERT_TRUE(thread[n].IsRunning());
@@ -170,106 +185,86 @@ TEST(PlatformThreadTest, FunctionTimesTen) {
 namespace {
 
 const ThreadPriority kThreadPriorityTestValues[] = {
-// Disable non-normal priority toggling on POSIX as it appears to be broken
-// (http://crbug.com/468793). This is prefered to disabling the tests altogether
-// on POSIX as it at least provides coverage for running this code under
-// "normal" priority.
-#if !defined(OS_POSIX)
-    ThreadPriority::DISPLAY,
+// The order should be higher to lower to cover as much cases as possible on
+// Linux trybots running without CAP_SYS_NICE permission.
+#if !defined(OS_ANDROID)
+    // PlatformThread::GetCurrentThreadPriority() on Android does not support
+    // REALTIME_AUDIO case. See http://crbug.com/505474.
     ThreadPriority::REALTIME_AUDIO,
-    // Keep BACKGROUND second to last to test backgrounding from other
-    // priorities.
+#endif
+    ThreadPriority::DISPLAY,
+    // This redundant BACKGROUND priority is to test backgrounding from other
+    // priorities, and unbackgrounding.
     ThreadPriority::BACKGROUND,
-#endif  // !defined(OS_POSIX)
-    // Keep NORMAL last to test unbackgrounding.
-    ThreadPriority::NORMAL
-};
-
-}  // namespace
-
-// Test changing another thread's priority.
-// NOTE: This test is partially disabled on POSIX, see note above and
-// http://crbug.com/468793.
-TEST(PlatformThreadTest, ThreadPriorityOtherThread) {
-  PlatformThreadHandle current_handle(PlatformThread::CurrentHandle());
-
-  // Confirm that the current thread's priority is as expected.
-  EXPECT_EQ(ThreadPriority::NORMAL,
-            PlatformThread::GetThreadPriority(current_handle));
-
-  // Create a test thread.
-  FunctionTestThread thread;
-  PlatformThreadHandle handle;
-  ASSERT_TRUE(PlatformThread::Create(0, &thread, &handle));
-  thread.WaitForThreadStart();
-  EXPECT_NE(thread.thread_id(), kInvalidThreadId);
-  EXPECT_NE(thread.thread_id(), PlatformThread::CurrentId());
-
-  // New threads should get normal priority by default.
-  EXPECT_EQ(ThreadPriority::NORMAL, PlatformThread::GetThreadPriority(handle));
-
-  // Toggle each supported priority on the test thread and confirm it only
-  // affects it (and not the current thread).
-  for (size_t i = 0; i < arraysize(kThreadPriorityTestValues); ++i) {
-    SCOPED_TRACE(i);
+    ThreadPriority::NORMAL,
+    ThreadPriority::BACKGROUND};
+
+bool IsBumpingPriorityAllowed() {
+#if defined(OS_POSIX)
+  // Only root can raise thread priority on POSIX environment. On Linux, users
+  // who have CAP_SYS_NICE permission also can raise the thread priority, but
+  // libcap.so would be needed to check the capability.
+  return geteuid() == 0;
+#else
+  return true;
+#endif
+}
 
-    // Alter and verify the test thread's priority.
-    PlatformThread::SetThreadPriority(handle, kThreadPriorityTestValues[i]);
-    EXPECT_EQ(kThreadPriorityTestValues[i],
-              PlatformThread::GetThreadPriority(handle));
+class ThreadPriorityTestThread : public FunctionTestThread {
+ public:
+  ThreadPriorityTestThread() = default;
+  ~ThreadPriorityTestThread() override = default;
 
-    // Make sure the current thread was otherwise unaffected.
+  void RunTest() override {

[Lei Zhang, 2015/07/14 07:54:20]

Also private.

[Takashi Toyoshima, 2015/07/14 08:03:23]

Done.

+    // Confirm that the current thread's priority is as expected.
     EXPECT_EQ(ThreadPriority::NORMAL,
-              PlatformThread::GetThreadPriority(current_handle));
+              PlatformThread::GetCurrentThreadPriority());
+
+    // Toggle each supported priority on the current thread and confirm it
+    // affects it.
+    const bool bumping_priority_allowed = IsBumpingPriorityAllowed();
+    for (size_t i = 0; i < arraysize(kThreadPriorityTestValues); ++i) {
+      SCOPED_TRACE(i);
+      if (!bumping_priority_allowed &&
+          kThreadPriorityTestValues[i] >
+              PlatformThread::GetCurrentThreadPriority()) {
+        continue;
+      }
+
+      // Alter and verify the current thread's priority.
+      PlatformThread::SetCurrentThreadPriority(kThreadPriorityTestValues[i]);
+      EXPECT_EQ(kThreadPriorityTestValues[i],
+                PlatformThread::GetCurrentThreadPriority());
+    }
   }
 
-  thread.MarkForTermination();
-  PlatformThread::Join(handle);
-}
+ private:
+  DISALLOW_COPY_AND_ASSIGN(ThreadPriorityTestThread);
+};
 
-// Test changing the current thread's priority (which has different semantics on
-// some platforms).
-// NOTE: This test is partially disabled on POSIX, see note above and
-// http://crbug.com/468793.
-TEST(PlatformThreadTest, ThreadPriorityCurrentThread) {
-  PlatformThreadHandle current_handle(PlatformThread::CurrentHandle());
+}  // namespace
 
-  // Confirm that the current thread's priority is as expected.
-  EXPECT_EQ(ThreadPriority::NORMAL,
-            PlatformThread::GetThreadPriority(current_handle));
+#if defined(OS_MACOSX)
+// PlatformThread::GetCurrentThreadPriority() is not implemented on OS X.
+#define MAYBE_ThreadPriorityCurrentThread DISABLED_ThreadPriorityCurrentThread
+#else
+#define MAYBE_ThreadPriorityCurrentThread ThreadPriorityCurrentThread
+#endif
 
-  // Create a test thread for verification purposes only.
-  FunctionTestThread thread;
+// Test changing a created thread's priority (which has different semantics on
+// some platforms).
+TEST(PlatformThreadTest, MAYBE_ThreadPriorityCurrentThread) {
+  ThreadPriorityTestThread thread;
   PlatformThreadHandle handle;
-  ASSERT_TRUE(PlatformThread::Create(0, &thread, &handle));
-  thread.WaitForThreadStart();
-  EXPECT_NE(thread.thread_id(), kInvalidThreadId);
-  EXPECT_NE(thread.thread_id(), PlatformThread::CurrentId());
-
-  // Confirm that the new thread's priority is as expected.
-  EXPECT_EQ(ThreadPriority::NORMAL, PlatformThread::GetThreadPriority(handle));
-
-  // Toggle each supported priority on the current thread and confirm it only
-  // affects it (and not the test thread).
-  for (size_t i = 0; i < arraysize(kThreadPriorityTestValues); ++i) {
-    SCOPED_TRACE(i);
-
-    // Alter and verify the current thread's priority.
-    PlatformThread::SetThreadPriority(current_handle,
-                                      kThreadPriorityTestValues[i]);
-    EXPECT_EQ(kThreadPriorityTestValues[i],
-              PlatformThread::GetThreadPriority(current_handle));
-
-    // Make sure the test thread was otherwise unaffected.
-    EXPECT_EQ(ThreadPriority::NORMAL,
-              PlatformThread::GetThreadPriority(handle));
-  }
 
-  // Restore current thread priority for follow-up tests.
-  PlatformThread::SetThreadPriority(current_handle, ThreadPriority::NORMAL);
+  ASSERT_FALSE(thread.IsRunning());
+  ASSERT_TRUE(PlatformThread::Create(0, &thread, &handle));
+  thread.WaitForTerminationReady();
+  ASSERT_TRUE(thread.IsRunning());
 
   thread.MarkForTermination();
   PlatformThread::Join(handle);
+  ASSERT_FALSE(thread.IsRunning());
 }
 
 }  // namespace base