Skip to content

Commit

Permalink
win7dep: Remove Win7+8 code in base/threading+task
Browse files Browse the repository at this point in the history
Removes Win7 and Win8 specific code in base/task, getting rid
of Win7-specifc call to ScopedCOMInitializer.

Removes win7-specific thread priorities in base/threading,
and reworks description of SetThreadPriority kernel bug to describe
Win10+-only. Also removes test code for Win7.

Also fixes lint warning.

Bug: 1385856
Change-Id: I6d61ef5933cb61a4f0e17793fa06ef81ac0311b1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4113990
Reviewed-by: Francois Pierre Doray <fdoray@chromium.org>
Commit-Queue: David Bienvenu <davidbienvenu@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1084727}
  • Loading branch information
David Bienvenu authored and Chromium LUCI CQ committed Dec 17, 2022
1 parent 7a2928b commit fd07a8a
Show file tree
Hide file tree
Showing 3 changed files with 11 additions and 48 deletions.
16 changes: 2 additions & 14 deletions base/task/thread_pool/thread_group.cc
Expand Up @@ -17,9 +17,7 @@

#if BUILDFLAG(IS_WIN)
#include "base/win/com_init_check_hook.h"
#include "base/win/scoped_com_initializer.h"
#include "base/win/scoped_winrt_initializer.h"
#include "base/win/windows_version.h"
#endif

namespace base {
Expand Down Expand Up @@ -327,18 +325,8 @@ bool ThreadGroup::ShouldYield(TaskSourceSortKey sort_key) {
std::unique_ptr<win::ScopedWindowsThreadEnvironment>
ThreadGroup::GetScopedWindowsThreadEnvironment(WorkerEnvironment environment) {
std::unique_ptr<win::ScopedWindowsThreadEnvironment> scoped_environment;
switch (environment) {
case WorkerEnvironment::COM_MTA: {
if (win::GetVersion() >= win::Version::WIN8) {
scoped_environment = std::make_unique<win::ScopedWinrtInitializer>();
} else {
scoped_environment = std::make_unique<win::ScopedCOMInitializer>(
win::ScopedCOMInitializer::kMTA);
}
break;
}
default:
break;
if (environment == WorkerEnvironment::COM_MTA) {
scoped_environment = std::make_unique<win::ScopedWinrtInitializer>();
}

DCHECK(!scoped_environment || scoped_environment->Succeeded());
Expand Down
21 changes: 3 additions & 18 deletions base/threading/platform_thread_win.cc
Expand Up @@ -6,6 +6,8 @@

#include <stddef.h>

#include <string>

#include "base/allocator/buildflags.h"
#include "base/allocator/partition_allocator/partition_alloc_buildflags.h"
#include "base/debug/activity_tracker.h"
Expand Down Expand Up @@ -47,14 +49,6 @@ namespace {
// |kUseThreadPriorityLowest| Feature.
std::atomic<bool> g_use_thread_priority_lowest{false};

// The most common value returned by ::GetThreadPriority() after background
// thread mode is enabled on Windows 7.
constexpr int kWin7BackgroundThreadModePriority = 4;

// Value sometimes returned by ::GetThreadPriority() after thread priority is
// set to normal on Windows 7.
constexpr int kWin7NormalPriority = 3;

// These values are sometimes returned by ::GetThreadPriority().
constexpr int kWinNormalPriority1 = 5;
constexpr int kWinNormalPriority2 = 6;
Expand Down Expand Up @@ -237,10 +231,7 @@ void AssertMemoryPriority(HANDLE thread, int memory_priority) {
reinterpret_cast<decltype(&::GetThreadInformation)>(::GetProcAddress(
::GetModuleHandle(L"Kernel32.dll"), "GetThreadInformation"));

if (!get_thread_information_fn) {
DCHECK_EQ(win::GetVersion(), win::Version::WIN7);
return;
}
DCHECK(get_thread_information_fn);

MEMORY_PRIORITY_INFORMATION memory_priority_information = {};
DCHECK(get_thread_information_fn(thread, ::ThreadMemoryPriority,
Expand Down Expand Up @@ -542,14 +533,8 @@ ThreadPriorityForTest PlatformThread::GetCurrentThreadPriorityForTest() {
return ThreadPriorityForTest::kBackground;

switch (priority) {
case kWin7BackgroundThreadModePriority:
DCHECK_EQ(win::GetVersion(), win::Version::WIN7);
return ThreadPriorityForTest::kBackground;
case THREAD_PRIORITY_BELOW_NORMAL:
return ThreadPriorityForTest::kUtility;
case kWin7NormalPriority:
DCHECK_EQ(win::GetVersion(), win::Version::WIN7);
[[fallthrough]];
case THREAD_PRIORITY_NORMAL:
return ThreadPriorityForTest::kNormal;
case kWinNormalPriority1:
Expand Down
22 changes: 6 additions & 16 deletions base/threading/platform_thread_win_unittest.cc
Expand Up @@ -17,9 +17,8 @@ namespace base {

// It has been observed that calling
// :SetThreadPriority(THREAD_MODE_BACKGROUND_BEGIN) in an IDLE_PRIORITY_CLASS
// process doesn't always affect the return value of ::GetThreadPriority() or
// the base priority reported in Process Explorer (on Win7, the values are
// sometimes affected while on Win8+ they are never affected). It does however
// process never affects the return value of ::GetThreadPriority() or
// the base priority reported in Process Explorer. It does however
// set the memory and I/O priorities to very low. This test confirms that
// behavior which we suspect is a Windows kernel bug. If this test starts
// failing, the mitigation for https://crbug.com/901483 in
Expand Down Expand Up @@ -50,24 +49,15 @@ TEST(PlatformThreadWinTest, SetBackgroundThreadModeFailsInIdlePriorityProcess) {
// Begin thread mode background.
EXPECT_TRUE(::SetThreadPriority(thread_handle, THREAD_MODE_BACKGROUND_BEGIN));

// On Win8, GetThreadPriority() stays NORMAL. On Win7, it can stay NORMAL or
// switch to one of the various priorities that are observed after entering
// thread mode background in a NORMAL_PRIORITY_CLASS process. On all Windows
// versions, memory priority becomes VERY_LOW.
// On Win10+, GetThreadPriority() stays NORMAL and memory priority becomes
// VERY_LOW.
//
// Note: this documents the aforementioned kernel bug. Ideally this would
// *not* be the case.
const int priority_after_thread_mode_background_begin =
::GetThreadPriority(thread_handle);
if (win::GetVersion() == win::Version::WIN7) {
const ThreadPriorityForTest priority =
PlatformThread::GetCurrentThreadPriorityForTest();
EXPECT_TRUE(priority == ThreadPriorityForTest::kNormal ||
priority == ThreadPriorityForTest::kBackground);
} else {
EXPECT_EQ(priority_after_thread_mode_background_begin,
THREAD_PRIORITY_NORMAL);
}
EXPECT_EQ(priority_after_thread_mode_background_begin,
THREAD_PRIORITY_NORMAL);
internal::AssertMemoryPriority(thread_handle, MEMORY_PRIORITY_VERY_LOW);

PlatformThread::Sleep(base::Seconds(1));
Expand Down

0 comments on commit fd07a8a

Please sign in to comment.