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..237509fee406f13210453d07a7c4082ec01c620c 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
 
@@ -170,106 +173,64 @@ 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.

[gab, 2015/06/29 13:04:00]

Should we file a bug for this and try to get someone on Android to take a look
at it? It's really only missing a Java hook similar to the one used to set
priority I assume.

[Takashi Toyoshima, 2015/06/29 16:21:10]

Done.

     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
-};
+    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 will be needed to check the capability.

[gab, 2015/06/29 13:04:00]

s/will/would/

[Takashi Toyoshima, 2015/06/29 16:21:11]

Done.

+  return geteuid() == 0;
+#else
+  return true;
+#endif
+}
 
 }  // 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);
-
-    // Alter and verify the test thread's priority.
-    PlatformThread::SetThreadPriority(handle, kThreadPriorityTestValues[i]);
-    EXPECT_EQ(kThreadPriorityTestValues[i],
-              PlatformThread::GetThreadPriority(handle));
-
-    // Make sure the current thread was otherwise unaffected.
-    EXPECT_EQ(ThreadPriority::NORMAL,
-              PlatformThread::GetThreadPriority(current_handle));
-  }
-
-  thread.MarkForTermination();
-  PlatformThread::Join(handle);
-}
+#if defined(OS_MACOSX)
+// PlatformThread::GetCurrentThreadPriority() is not implemented on OS X.
+#define MAYBE_ThreadPriorityCurrentThread DISABLED_ThreadPriorityCurrentThread
+#else
+#define MAYBE_ThreadPriorityCurrentThread ThreadPriorityCurrentThread
+#endif
 
 // 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());
-
+TEST(PlatformThreadTest, MAYBE_ThreadPriorityCurrentThread) {
   // Confirm that the current thread's priority is as expected.
-  EXPECT_EQ(ThreadPriority::NORMAL,
-            PlatformThread::GetThreadPriority(current_handle));
-
-  // Create a test thread for verification purposes only.
-  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());
-
-  // Confirm that the new thread's priority is as expected.
-  EXPECT_EQ(ThreadPriority::NORMAL, PlatformThread::GetThreadPriority(handle));
+  EXPECT_EQ(ThreadPriority::NORMAL, PlatformThread::GetCurrentThreadPriority());
 
-  // Toggle each supported priority on the current thread and confirm it only
-  // affects it (and not the test thread).
+  // 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;

[gab, 2015/06/29 13:04:00]

nit: {} since conditional is multiline

[Takashi Toyoshima, 2015/06/29 16:21:11]

Done.

 
     // Alter and verify the current thread's priority.
-    PlatformThread::SetThreadPriority(current_handle,
-                                      kThreadPriorityTestValues[i]);
+    PlatformThread::SetCurrentThreadPriority(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));
+              PlatformThread::GetCurrentThreadPriority());
   }
 
   // Restore current thread priority for follow-up tests.
-  PlatformThread::SetThreadPriority(current_handle, ThreadPriority::NORMAL);
-
-  thread.MarkForTermination();
-  PlatformThread::Join(handle);
+  PlatformThread::SetCurrentThreadPriority(ThreadPriority::NORMAL);
 }
 
 }  // namespace base