Commit ba4879d0 authored by Cliff Smolinsky's avatar Cliff Smolinsky Committed by Commit Bot

Remove FreeLibrary calls for chrome and chrome_child load sanity tests.

Address sanitizer runs were flaky for the chrome and chrome_child dll
load sanity tests. While it's not 100% clearly exactly what is causing
the issues because they can't be repro'd, it does appear that the
failures relate to unloading the dlls (with FreeLibrary). These dlls
have never been intended to be freed like this, however. During normal
execution they are never unloaded but instead just go away when the
process is terminated. This change updates the tests to match that
behavior and simply let the process exit.

Bug: 964861
Change-Id: I46384f42b87e99d9615fa754097d0d589bd4a62d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1627755Reviewed-by: default avatarBruce Dawson <brucedawson@chromium.org>
Reviewed-by: default avatarScott Violet <sky@chromium.org>
Commit-Queue: Cliff Smolinsky <cliffsmo@microsoft.com>
Cr-Commit-Position: refs/heads/master@{#663776}
parent 13326563
...@@ -114,7 +114,7 @@ TEST_F(DelayloadsTest, ChromeDllDelayloadsCheck) { ...@@ -114,7 +114,7 @@ TEST_F(DelayloadsTest, ChromeDllDelayloadsCheck) {
} }
} }
TEST_F(DelayloadsTest, DISABLED_ChromeDllLoadSanityTest) { TEST_F(DelayloadsTest, ChromeDllLoadSanityTest) {
// As a precaution to avoid affecting other tests, we need to ensure this is // As a precaution to avoid affecting other tests, we need to ensure this is
// executed in its own test process. This "test" will re-launch with custom // executed in its own test process. This "test" will re-launch with custom
// parameters to accomplish that. // parameters to accomplish that.
...@@ -156,10 +156,9 @@ TEST_F(DelayloadsTest, DISABLED_ChromeDllLoadSanityTestImpl) { ...@@ -156,10 +156,9 @@ TEST_F(DelayloadsTest, DISABLED_ChromeDllLoadSanityTestImpl) {
ASSERT_TRUE(chrome_module_handle != nullptr); ASSERT_TRUE(chrome_module_handle != nullptr);
// Loading chrome.dll should not load user32.dll. // Loading chrome.dll should not load user32.dll.
EXPECT_EQ(nullptr, ::GetModuleHandle(L"user32.dll")); EXPECT_EQ(nullptr, ::GetModuleHandle(L"user32.dll"));
EXPECT_TRUE(!!::FreeLibrary(chrome_module_handle));
} }
TEST_F(DelayloadsTest, DISABLED_ChromeChildDllDelayloadsCheck) { TEST_F(DelayloadsTest, ChromeChildDllDelayloadsCheck) {
base::FilePath dll; base::FilePath dll;
ASSERT_TRUE(base::PathService::Get(base::DIR_EXE, &dll)); ASSERT_TRUE(base::PathService::Get(base::DIR_EXE, &dll));
dll = dll.Append(L"chrome_child.dll"); dll = dll.Append(L"chrome_child.dll");
...@@ -250,14 +249,13 @@ TEST_F(DelayloadsTest, DISABLED_ChromeChildDllLoadSanityTestImpl) { ...@@ -250,14 +249,13 @@ TEST_F(DelayloadsTest, DISABLED_ChromeChildDllLoadSanityTestImpl) {
HMODULE chrome_child_module_handle = ::LoadLibrary(dll.value().c_str()); HMODULE chrome_child_module_handle = ::LoadLibrary(dll.value().c_str());
ASSERT_TRUE(chrome_child_module_handle != nullptr); ASSERT_TRUE(chrome_child_module_handle != nullptr);
// Loading chrome.dll should not load user32.dll on Win10. // Loading chrome_child.dll should not load user32.dll on Win10.
// On Win7, chains of system dlls and lack of apisets result in it loading. // On Win7, chains of system dlls and lack of apisets result in it loading.
if (base::win::GetVersion() >= base::win::Version::WIN10) { if (base::win::GetVersion() >= base::win::Version::WIN10) {
EXPECT_EQ(nullptr, ::GetModuleHandle(L"user32.dll")); EXPECT_EQ(nullptr, ::GetModuleHandle(L"user32.dll"));
} else { } else {
EXPECT_NE(nullptr, ::GetModuleHandle(L"user32.dll")); EXPECT_NE(nullptr, ::GetModuleHandle(L"user32.dll"));
} }
EXPECT_TRUE(!!::FreeLibrary(chrome_child_module_handle));
} }
TEST_F(DelayloadsTest, ChromeElfDllDelayloadsCheck) { TEST_F(DelayloadsTest, ChromeElfDllDelayloadsCheck) {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment