[base] Ensure that tests don't change the thread priority (reland #3).

This is a reland of https://crrev.com/c/chromium/src/+/1865405
The difference is that the check is not enabled on Android, due to
test failures. It is better to land this now to avoid regressions on
other platforms while Android failure is investigated.
Diff: https://crrev.com/c/chromium/src/+/1894158/1..2/base/test/test_suite.cc

This CL verifies that the thread priority is normal when a test process
is launched and before/after each test. The goal is to avoid having
tests that assume they run at normal priority be disabled because of
other misbehaving tests (e.g. https://crbug.com/931706).

TBR=gab@chromium.org,sky@chromium.org,michaelpg@chromium.org,thakis@chromium.org,eseckler@chromium.org

Bug: 931706
Change-Id: I2db19e011ad1c408bb112b35b453b081f6888739
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1894158
Commit-Queue: François Doray <fdoray@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: François Doray <fdoray@chromium.org>
Cr-Commit-Position: refs/heads/master@{#712259}

ash/shell/content/test/ash_content_perf_test_launcher.cc

@@ -34,9 +34,10 @@ class AshContentTestSuite : public content::ContentTestSuiteBase {
  protected:
   // content::ContentTestSuiteBase:
   void Initialize() override {
-    // Browser tests are expected not to tear-down various globals. (Must run
-    // before the base class is initialized.)
+    // Browser tests are expected not to tear-down various globals and may
+    // complete with the thread priority being above NORMAL.
     base::TestSuite::DisableCheckForLeakedGlobals();
+    base::TestSuite::DisableCheckForThreadPriorityAtTestEnd();
     ContentTestSuiteBase::Initialize();
     ui_controls::InstallUIControlsAura(ash::test::CreateAshUIControls());
   }

base/test/test_suite.cc

@@ -37,6 +37,7 @@
 #include "base/test/multiprocess_test.h"
 #include "base/test/test_switches.h"
 #include "base/test/test_timeouts.h"
+#include "base/threading/platform_thread.h"
 #include "base/time/time.h"
 #include "build/build_config.h"
 #include "testing/gmock/include/gmock/gmock.h"
@@ -196,6 +197,31 @@ class CheckProcessPriority : public testing::EmptyTestEventListener {
 };
 #endif  // !defined(OS_IOS)
 
+class CheckThreadPriority : public testing::EmptyTestEventListener {
+ public:
+  CheckThreadPriority(bool check_thread_priority_at_test_end)
+      : check_thread_priority_at_test_end_(check_thread_priority_at_test_end) {
+    CHECK_EQ(base::PlatformThread::GetCurrentThreadPriority(),
+             base::ThreadPriority::NORMAL);
+  }
+
+  void OnTestStart(const testing::TestInfo& test) override {
+    EXPECT_EQ(base::PlatformThread::GetCurrentThreadPriority(),
+              base::ThreadPriority::NORMAL);
+  }
+  void OnTestEnd(const testing::TestInfo& test) override {
+    if (check_thread_priority_at_test_end_) {
+      EXPECT_EQ(base::PlatformThread::GetCurrentThreadPriority(),
+                base::ThreadPriority::NORMAL);
+    }
+  }
+
+ private:
+  const bool check_thread_priority_at_test_end_;
+
+  DISALLOW_COPY_AND_ASSIGN(CheckThreadPriority);
+};
+
 const std::string& GetProfileName() {
   static const NoDestructor<std::string> profile_name([]() {
     const CommandLine& command_line = *CommandLine::ForCurrentProcess();
@@ -384,9 +410,14 @@ void TestSuite::DisableCheckForLeakedGlobals() {
   check_for_leaked_globals_ = false;
 }
 
-void TestSuite::DisableCheckForProcessPriority() {
+void TestSuite::DisableCheckForThreadAndProcessPriority() {
   DCHECK(!is_initialized_);
-  check_for_process_priority_ = false;
+  check_for_thread_and_process_priority_ = false;
+}
+
+void TestSuite::DisableCheckForThreadPriorityAtTestEnd() {
+  DCHECK(!is_initialized_);
+  check_for_thread_priority_at_test_end_ = false;
 }
 
 void TestSuite::UnitTestAssertHandler(const char* file,
@@ -575,10 +606,16 @@ void TestSuite::Initialize() {
   listeners.Append(new ResetCommandLineBetweenTests);
   if (check_for_leaked_globals_)
     listeners.Append(new CheckForLeakedGlobals);
+  if (check_for_thread_and_process_priority_) {
+#if !defined(OS_ANDROID)
+    // TODO(https://crbug.com/931706): Check thread priority on Android.
+    listeners.Append(
+        new CheckThreadPriority(check_for_thread_priority_at_test_end_));
+#endif
 #if !defined(OS_IOS)
-  if (check_for_process_priority_)
     listeners.Append(new CheckProcessPriority);
 #endif
+  }
 
   AddTestLauncherResultPrinter();
 

base/test/test_suite.h

@@ -43,8 +43,15 @@ class TestSuite {
 
   int Run();
 
-  // Disables checks for process priority. Most tests should not use this.
-  void DisableCheckForProcessPriority();
+  // Disables checks for thread and process priority at the beginning and end of
+  // each test. Most tests should not use this.
+  void DisableCheckForThreadAndProcessPriority();
+
+  // Disables checks for thread priority at the end of each test (still checks
+  // at the beginning of each test). This should be used for tests that run in
+  // their own process and should start with normal priorities but are allowed
+  // to end with different priorities.
+  void DisableCheckForThreadPriorityAtTestEnd();
 
   // Disables checks for certain global objects being leaked across tests.
   void DisableCheckForLeakedGlobals();
@@ -93,7 +100,8 @@ class TestSuite {
   std::unique_ptr<logging::ScopedLogAssertHandler> assert_handler_;
 
   bool check_for_leaked_globals_ = true;
-  bool check_for_process_priority_ = true;
+  bool check_for_thread_and_process_priority_ = true;
+  bool check_for_thread_priority_at_test_end_ = true;
 
   bool is_initialized_ = false;
 

chrome/test/base/browser_tests_main_chromeos.cc

@@ -30,8 +30,10 @@ class BrowserTestSuiteRunnerChromeOS : public ChromeTestSuiteRunner {
  public:
   int RunTestSuite(int argc, char** argv) override {
     BrowserTestSuiteChromeOS test_suite(argc, argv);
-    // Browser tests are expected not to tear-down various globals.
+    // Browser tests are expected not to tear-down various globals and may
+    // complete with the thread priority being above NORMAL.
     test_suite.DisableCheckForLeakedGlobals();
+    test_suite.DisableCheckForThreadPriorityAtTestEnd();
     return test_suite.Run();
   }
 };

chrome/test/base/chrome_test_launcher.cc

@@ -77,8 +77,10 @@ ChromeTestSuiteRunner::~ChromeTestSuiteRunner() {}
 
 int ChromeTestSuiteRunner::RunTestSuite(int argc, char** argv) {
   ChromeTestSuite test_suite(argc, argv);
-  // Browser tests are expected not to tear-down various globals.
+  // Browser tests are expected not to tear-down various globals and may
+  // complete with the thread priority being above NORMAL.
   test_suite.DisableCheckForLeakedGlobals();
+  test_suite.DisableCheckForThreadPriorityAtTestEnd();
 #if defined(OS_ANDROID)
   // Android browser tests run child processes as threads instead.
   content::ContentTestSuiteBase::RegisterInProcessThreads();

chrome/test/base/interactive_ui_tests_main.cc

@@ -41,8 +41,10 @@ class InteractiveUITestSuite : public ChromeTestSuite {
  protected:
   // ChromeTestSuite overrides:
   void Initialize() override {
-    // Browser tests are expected not to tear-down various globals.
+    // Browser tests are expected not to tear-down various globals and may
+    // complete with the thread priority being above NORMAL.
     base::TestSuite::DisableCheckForLeakedGlobals();
+    base::TestSuite::DisableCheckForThreadPriorityAtTestEnd();
 
     ChromeTestSuite::Initialize();
 

chromecast/app/cast_test_launcher.cc

@@ -24,8 +24,10 @@ class CastTestLauncherDelegate : public content::TestLauncherDelegate {
 
   int RunTestSuite(int argc, char** argv) override {
     base::TestSuite test_suite(argc, argv);
-    // Browser tests are expected not to tear-down various globals.
+    // Browser tests are expected not to tear-down various globals and may
+    // complete with the thread priority being above NORMAL.
     test_suite.DisableCheckForLeakedGlobals();
+    test_suite.DisableCheckForThreadPriorityAtTestEnd();
     return test_suite.Run();
   }
 

content/test/content_test_launcher.cc

@@ -39,9 +39,10 @@ class ContentBrowserTestSuite : public ContentTestSuiteBase {
 
  protected:
   void Initialize() override {
-    // Browser tests are expected not to tear-down various globals. (Must run
-    // before the base class is initialized.)
+    // Browser tests are expected not to tear-down various globals and may
+    // complete with the thread priority being above NORMAL.
     base::TestSuite::DisableCheckForLeakedGlobals();
+    base::TestSuite::DisableCheckForThreadPriorityAtTestEnd();
 
     ContentTestSuiteBase::Initialize();
 

extensions/shell/test/shell_test_launcher_delegate.cc

@@ -13,8 +13,10 @@ namespace extensions {
 
 int AppShellTestLauncherDelegate::RunTestSuite(int argc, char** argv) {
   base::TestSuite test_suite(argc, argv);
-  // Browser tests are expected not to tear-down various globals.
+  // Browser tests are expected not to tear-down various globals and may
+  // complete with the thread priority being above NORMAL.
   test_suite.DisableCheckForLeakedGlobals();
+  test_suite.DisableCheckForThreadPriorityAtTestEnd();
   return test_suite.Run();
 }
 

fuchsia/engine/test/web_engine_test_launcher.cc

@@ -23,8 +23,10 @@ class WebEngineTestLauncherDelegate : public content::TestLauncherDelegate {
   // content::TestLauncherDelegate implementation:
   int RunTestSuite(int argc, char** argv) override {
     base::TestSuite test_suite(argc, argv);
-    // Browser tests are expected not to tear-down various globals.
+    // Browser tests are expected not to tear-down various globals and may
+    // complete with the thread priority being above NORMAL.
     test_suite.DisableCheckForLeakedGlobals();
+    test_suite.DisableCheckForThreadPriorityAtTestEnd();
     return test_suite.Run();
   }
 

gpu/angle_deqp_tests_main.cc

@@ -35,8 +35,9 @@ int main(int argc, char** argv) {
   angle::InitTestHarness(&argc, argv);
   base::TestSuite test_suite(argc, argv);
 
-  // The process priority is lowered by the constructor of tcu::ANGLEPlatform().
-  test_suite.DisableCheckForProcessPriority();
+  // The process and thread priorities are modified by
+  // StabilizeCPUForBenchmarking()/SetLowPriorityProcess().
+  test_suite.DisableCheckForThreadAndProcessPriority();
 
   int rt = base::LaunchUnitTestsSerially(
       argc, argv, base::BindOnce(&RunHelper, base::Unretained(&test_suite)));

gpu/angle_perftests_main.cc

@@ -27,6 +27,10 @@ int main(int argc, char** argv) {
   ANGLEProcessPerfTestArgs(&argc, argv);
 
   base::TestSuite test_suite(argc, argv);
+
+  // The thread priority is modified by StabilizeCPUForBenchmarking().
+  test_suite.DisableCheckForThreadAndProcessPriority();
+
   int rt = base::LaunchUnitTestsSerially(
       argc, argv, base::BindOnce(&RunHelper, base::Unretained(&test_suite)));
   return rt;

gpu/angle_unittest_main.cc

@@ -24,7 +24,10 @@ int main(int argc, char** argv) {
   base::CommandLine::Init(argc, argv);
   testing::InitGoogleMock(&argc, argv);
   sh::Initialize();
+
   base::TestSuite test_suite(argc, argv);
+  test_suite.DisableCheckForThreadAndProcessPriority();
+
   int rt = base::LaunchUnitTestsSerially(
       argc, argv, base::BindOnce(&RunHelper, base::Unretained(&test_suite)));
   sh::Finalize();

headless/test/headless_test_launcher.cc

@@ -41,8 +41,10 @@ class HeadlessTestLauncherDelegate : public content::TestLauncherDelegate {
   // content::TestLauncherDelegate implementation:
   int RunTestSuite(int argc, char** argv) override {
     base::TestSuite test_suite(argc, argv);
-    // Browser tests are expected not to tear-down various globals.
+    // Browser tests are expected not to tear-down various globals and may
+    // complete with the thread priority being above NORMAL.
     test_suite.DisableCheckForLeakedGlobals();
+    test_suite.DisableCheckForThreadPriorityAtTestEnd();
     return test_suite.Run();
   }
 

weblayer/test/test_launcher_delegate_impl.cc

@@ -17,6 +17,7 @@ int TestLauncherDelegateImpl::RunTestSuite(int argc, char** argv) {
   // Browser tests are expected not to tear-down various globals and may
   // complete with the thread priority being above NORMAL.
   test_suite.DisableCheckForLeakedGlobals();
+  test_suite.DisableCheckForThreadPriorityAtTestEnd();
   return test_suite.Run();
 }