Commit 57213418 authored by Patrick Monette's avatar Patrick Monette Committed by Commit Bot

[3P blocking] Fix tests accessing the ModuleDatabase on the UI thread

The ModuleDatabase was moved to its own sequence but both the
IncompatibleApplicationsBrowserTest and ThirdPartyBlockingBrowserTest
test fixtures were not updated.

This CL moves the accesses to the ModuleDatabase instance to the right
sequence.

Also added an override for the OutOfProcessQuarantine feature to allow
local testing. This was not an issue on the bots because they respect
the fieldtrial_testing_config.json file.

Bug: 850517
Change-Id: I2e8955b33cd7ffc75f74dd56b01df9dec96fc604
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2031742
Commit-Queue: Chris Hamilton <chrisha@chromium.org>
Reviewed-by: default avatarChris Hamilton <chrisha@chromium.org>
Cr-Commit-Position: refs/heads/master@{#737448}
parent 41c0c590
...@@ -12,6 +12,7 @@ ...@@ -12,6 +12,7 @@
#include "base/scoped_native_library.h" #include "base/scoped_native_library.h"
#include "base/strings/stringprintf.h" #include "base/strings/stringprintf.h"
#include "base/strings/utf_string_conversions.h" #include "base/strings/utf_string_conversions.h"
#include "base/test/bind_test_util.h"
#include "base/test/scoped_feature_list.h" #include "base/test/scoped_feature_list.h"
#include "base/test/test_reg_util_win.h" #include "base/test/test_reg_util_win.h"
#include "base/threading/thread_restrictions.h" #include "base/threading/thread_restrictions.h"
...@@ -27,6 +28,7 @@ ...@@ -27,6 +28,7 @@
#include "chrome/test/base/in_process_browser_test.h" #include "chrome/test/base/in_process_browser_test.h"
#include "components/prefs/pref_change_registrar.h" #include "components/prefs/pref_change_registrar.h"
#include "components/prefs/pref_service.h" #include "components/prefs/pref_service.h"
#include "components/services/quarantine/public/cpp/quarantine_features_win.h"
// This class allows to wait until the kIncompatibleApplications preference is // This class allows to wait until the kIncompatibleApplications preference is
// modified. This can only happen if a new incompatible application is found, // modified. This can only happen if a new incompatible application is found,
...@@ -92,8 +94,10 @@ class IncompatibleApplicationsBrowserTest : public InProcessBrowserTest { ...@@ -92,8 +94,10 @@ class IncompatibleApplicationsBrowserTest : public InProcessBrowserTest {
ASSERT_NO_FATAL_FAILURE( ASSERT_NO_FATAL_FAILURE(
registry_override_manager_.OverrideRegistry(HKEY_CURRENT_USER)); registry_override_manager_.OverrideRegistry(HKEY_CURRENT_USER));
scoped_feature_list_.InitAndEnableFeature( scoped_feature_list_.InitWithFeatures(
features::kIncompatibleApplicationsWarning); {features::kIncompatibleApplicationsWarning,
quarantine::kOutOfProcessQuarantine},
{});
ASSERT_NO_FATAL_FAILURE(CreateModuleList()); ASSERT_NO_FATAL_FAILURE(CreateModuleList());
ASSERT_NO_FATAL_FAILURE(InstallThirdPartyApplication()); ASSERT_NO_FATAL_FAILURE(InstallThirdPartyApplication());
...@@ -198,18 +202,27 @@ IN_PROC_BROWSER_TEST_F(IncompatibleApplicationsBrowserTest, ...@@ -198,18 +202,27 @@ IN_PROC_BROWSER_TEST_F(IncompatibleApplicationsBrowserTest,
if (base::win::GetVersion() < base::win::Version::WIN10) if (base::win::GetVersion() < base::win::Version::WIN10)
return; return;
ModuleDatabase* module_database = ModuleDatabase::GetInstance();
// Speed up the test.
module_database->IncreaseInspectionPriority();
// Create the observer early so the change is guaranteed to be observed. // Create the observer early so the change is guaranteed to be observed.
auto incompatible_applications_observer = auto incompatible_applications_observer =
std::make_unique<IncompatibleApplicationsObserver>(); std::make_unique<IncompatibleApplicationsObserver>();
// Simulate the download of the module list component. base::RunLoop run_loop;
module_database->third_party_conflicts_manager()->LoadModuleList( ModuleDatabase::GetTaskRunner()->PostTask(
GetModuleListPath()); FROM_HERE,
base::BindLambdaForTesting([module_list_path = GetModuleListPath(),
quit_closure = run_loop.QuitClosure()]() {
ModuleDatabase* module_database = ModuleDatabase::GetInstance();
// Speed up the test.
module_database->IncreaseInspectionPriority();
// Simulate the download of the module list component.
module_database->third_party_conflicts_manager()->LoadModuleList(
module_list_path);
quit_closure.Run();
}));
run_loop.Run();
// Injects the DLL into the process. // Injects the DLL into the process.
base::ScopedAllowBlockingForTesting scoped_allow_blocking; base::ScopedAllowBlockingForTesting scoped_allow_blocking;
......
...@@ -118,6 +118,7 @@ scoped_refptr<base::SequencedTaskRunner> ModuleDatabase::GetTaskRunner() { ...@@ -118,6 +118,7 @@ scoped_refptr<base::SequencedTaskRunner> ModuleDatabase::GetTaskRunner() {
// static // static
ModuleDatabase* ModuleDatabase::GetInstance() { ModuleDatabase* ModuleDatabase::GetInstance() {
DCHECK(GetTaskRunner()->RunsTasksInCurrentSequence());
return g_module_database; return g_module_database;
} }
......
...@@ -10,6 +10,7 @@ ...@@ -10,6 +10,7 @@
#include "base/path_service.h" #include "base/path_service.h"
#include "base/run_loop.h" #include "base/run_loop.h"
#include "base/scoped_native_library.h" #include "base/scoped_native_library.h"
#include "base/test/bind_test_util.h"
#include "base/test/scoped_feature_list.h" #include "base/test/scoped_feature_list.h"
#include "base/test/test_reg_util_win.h" #include "base/test/test_reg_util_win.h"
#include "base/win/registry.h" #include "base/win/registry.h"
...@@ -23,6 +24,7 @@ ...@@ -23,6 +24,7 @@
#include "chrome/common/chrome_features.h" #include "chrome/common/chrome_features.h"
#include "chrome/install_static/install_util.h" #include "chrome/install_static/install_util.h"
#include "chrome/test/base/in_process_browser_test.h" #include "chrome/test/base/in_process_browser_test.h"
#include "components/services/quarantine/public/cpp/quarantine_features_win.h"
namespace { namespace {
...@@ -76,6 +78,29 @@ class ThirdPartyRegistryKeyObserver { ...@@ -76,6 +78,29 @@ class ThirdPartyRegistryKeyObserver {
DISALLOW_COPY_AND_ASSIGN(ThirdPartyRegistryKeyObserver); DISALLOW_COPY_AND_ASSIGN(ThirdPartyRegistryKeyObserver);
}; };
// Creates an empty serialized ModuleList proto in the module list component
// directory and returns its path.
void CreateModuleList(base::FilePath* module_list_path) {
chrome::conflicts::ModuleList module_list;
// Include an empty blacklist and whitelist.
module_list.mutable_blacklist();
module_list.mutable_whitelist();
std::string contents;
ASSERT_TRUE(module_list.SerializeToString(&contents));
// Put the module list beside the module blacklist cache.
*module_list_path = ModuleBlacklistCacheUpdater::GetModuleBlacklistCachePath()
.DirName()
.Append(FILE_PATH_LITERAL("ModuleList.bin"));
base::ScopedAllowBlockingForTesting scoped_allow_blocking;
ASSERT_TRUE(base::CreateDirectory(module_list_path->DirName()));
ASSERT_EQ(static_cast<int>(contents.size()),
base::WriteFile(*module_list_path, contents.data(),
static_cast<int>(contents.size())));
}
class ThirdPartyBlockingBrowserTest : public InProcessBrowserTest { class ThirdPartyBlockingBrowserTest : public InProcessBrowserTest {
protected: protected:
ThirdPartyBlockingBrowserTest() = default; ThirdPartyBlockingBrowserTest() = default;
...@@ -83,8 +108,9 @@ class ThirdPartyBlockingBrowserTest : public InProcessBrowserTest { ...@@ -83,8 +108,9 @@ class ThirdPartyBlockingBrowserTest : public InProcessBrowserTest {
// InProcessBrowserTest: // InProcessBrowserTest:
void SetUp() override { void SetUp() override {
scoped_feature_list_.InitAndEnableFeature( scoped_feature_list_.InitWithFeatures({features::kThirdPartyModulesBlocking,
features::kThirdPartyModulesBlocking); quarantine::kOutOfProcessQuarantine},
{});
ASSERT_TRUE(scoped_temp_dir_.CreateUniqueTempDir()); ASSERT_TRUE(scoped_temp_dir_.CreateUniqueTempDir());
ASSERT_NO_FATAL_FAILURE( ASSERT_NO_FATAL_FAILURE(
...@@ -108,30 +134,6 @@ class ThirdPartyBlockingBrowserTest : public InProcessBrowserTest { ...@@ -108,30 +134,6 @@ class ThirdPartyBlockingBrowserTest : public InProcessBrowserTest {
ASSERT_TRUE(base::CopyFile(test_dll_path, *third_party_module_path)); ASSERT_TRUE(base::CopyFile(test_dll_path, *third_party_module_path));
} }
// Creates an empty serialized ModuleList proto in the module list component
// directory and returns its path.
void CreateModuleList(base::FilePath* module_list_path) {
chrome::conflicts::ModuleList module_list;
// Include an empty blacklist and whitelist.
module_list.mutable_blacklist();
module_list.mutable_whitelist();
std::string contents;
ASSERT_TRUE(module_list.SerializeToString(&contents));
// Put the module list beside the module blacklist cache.
*module_list_path =
ModuleBlacklistCacheUpdater::GetModuleBlacklistCachePath()
.DirName()
.Append(FILE_PATH_LITERAL("ModuleList.bin"));
base::ScopedAllowBlockingForTesting scoped_allow_blocking;
ASSERT_TRUE(base::CreateDirectory(module_list_path->DirName()));
ASSERT_EQ(static_cast<int>(contents.size()),
base::WriteFile(*module_list_path, contents.data(),
static_cast<int>(contents.size())));
}
// Enables the ThirdPartyModulesBlocking feature. // Enables the ThirdPartyModulesBlocking feature.
base::test::ScopedFeatureList scoped_feature_list_; base::test::ScopedFeatureList scoped_feature_list_;
...@@ -158,22 +160,30 @@ IN_PROC_BROWSER_TEST_F(ThirdPartyBlockingBrowserTest, ...@@ -158,22 +160,30 @@ IN_PROC_BROWSER_TEST_F(ThirdPartyBlockingBrowserTest,
if (base::win::GetVersion() < base::win::Version::WIN8) if (base::win::GetVersion() < base::win::Version::WIN8)
return; return;
base::FilePath module_list_path;
ASSERT_NO_FATAL_FAILURE(CreateModuleList(&module_list_path));
ASSERT_FALSE(module_list_path.empty());
ModuleDatabase* module_database = ModuleDatabase::GetInstance();
// Speed up the test.
module_database->IncreaseInspectionPriority();
// Create the observer early so the change is guaranteed to be observed. // Create the observer early so the change is guaranteed to be observed.
ThirdPartyRegistryKeyObserver third_party_registry_key_observer; ThirdPartyRegistryKeyObserver third_party_registry_key_observer;
ASSERT_TRUE(third_party_registry_key_observer.StartWatching()); ASSERT_TRUE(third_party_registry_key_observer.StartWatching());
// Simulate the download of the module list component. base::RunLoop run_loop;
module_database->third_party_conflicts_manager()->LoadModuleList( ModuleDatabase::GetTaskRunner()->PostTask(
module_list_path); FROM_HERE,
base::BindLambdaForTesting([quit_closure = run_loop.QuitClosure()]() {
ModuleDatabase* module_database = ModuleDatabase::GetInstance();
// Speed up the test.
module_database->IncreaseInspectionPriority();
base::FilePath module_list_path;
ASSERT_NO_FATAL_FAILURE(CreateModuleList(&module_list_path));
ASSERT_FALSE(module_list_path.empty());
// Simulate the download of the module list component.
module_database->third_party_conflicts_manager()->LoadModuleList(
module_list_path);
quit_closure.Run();
}));
run_loop.Run();
// Injects the third-party DLL into the process. // Injects the third-party DLL into the process.
base::FilePath third_party_module_path; base::FilePath third_party_module_path;
......
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