Commit 7d566e60 authored by Patrick Monette's avatar Patrick Monette Committed by Commit Bot

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

This is a reland of 57213418

The GN dependency error was fixed

Original change's description:
> [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: Chris Hamilton <chrisha@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#737448}

Bug: 850517
Change-Id: Ibe99a6d359ec1afe7282b76fc3ad9491c7dee70c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2035719Reviewed-by: default avatarChris Hamilton <chrisha@chromium.org>
Commit-Queue: Chris Hamilton <chrisha@chromium.org>
Commit-Queue: Patrick Monette <pmonette@chromium.org>
Cr-Commit-Position: refs/heads/master@{#737840}
parent a9285343
......@@ -99,6 +99,7 @@ if (is_chrome_branded) {
"//chrome/browser",
"//chrome/chrome_elf:third_party_shared_defines",
"//chrome/test:test_support_ui",
"//components/services/quarantine/public/cpp:features",
"//testing/gtest",
]
}
......
......@@ -12,6 +12,7 @@
#include "base/scoped_native_library.h"
#include "base/strings/stringprintf.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/test_reg_util_win.h"
#include "base/threading/thread_restrictions.h"
......@@ -27,6 +28,7 @@
#include "chrome/test/base/in_process_browser_test.h"
#include "components/prefs/pref_change_registrar.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
// modified. This can only happen if a new incompatible application is found,
......@@ -92,8 +94,10 @@ class IncompatibleApplicationsBrowserTest : public InProcessBrowserTest {
ASSERT_NO_FATAL_FAILURE(
registry_override_manager_.OverrideRegistry(HKEY_CURRENT_USER));
scoped_feature_list_.InitAndEnableFeature(
features::kIncompatibleApplicationsWarning);
scoped_feature_list_.InitWithFeatures(
{features::kIncompatibleApplicationsWarning,
quarantine::kOutOfProcessQuarantine},
{});
ASSERT_NO_FATAL_FAILURE(CreateModuleList());
ASSERT_NO_FATAL_FAILURE(InstallThirdPartyApplication());
......@@ -198,18 +202,27 @@ IN_PROC_BROWSER_TEST_F(IncompatibleApplicationsBrowserTest,
if (base::win::GetVersion() < base::win::Version::WIN10)
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.
auto incompatible_applications_observer =
std::make_unique<IncompatibleApplicationsObserver>();
// Simulate the download of the module list component.
module_database->third_party_conflicts_manager()->LoadModuleList(
GetModuleListPath());
base::RunLoop run_loop;
ModuleDatabase::GetTaskRunner()->PostTask(
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.
base::ScopedAllowBlockingForTesting scoped_allow_blocking;
......
......@@ -118,6 +118,7 @@ scoped_refptr<base::SequencedTaskRunner> ModuleDatabase::GetTaskRunner() {
// static
ModuleDatabase* ModuleDatabase::GetInstance() {
DCHECK(GetTaskRunner()->RunsTasksInCurrentSequence());
return g_module_database;
}
......
......@@ -10,6 +10,7 @@
#include "base/path_service.h"
#include "base/run_loop.h"
#include "base/scoped_native_library.h"
#include "base/test/bind_test_util.h"
#include "base/test/scoped_feature_list.h"
#include "base/test/test_reg_util_win.h"
#include "base/win/registry.h"
......@@ -23,6 +24,7 @@
#include "chrome/common/chrome_features.h"
#include "chrome/install_static/install_util.h"
#include "chrome/test/base/in_process_browser_test.h"
#include "components/services/quarantine/public/cpp/quarantine_features_win.h"
namespace {
......@@ -76,6 +78,29 @@ class 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 {
protected:
ThirdPartyBlockingBrowserTest() = default;
......@@ -83,8 +108,9 @@ class ThirdPartyBlockingBrowserTest : public InProcessBrowserTest {
// InProcessBrowserTest:
void SetUp() override {
scoped_feature_list_.InitAndEnableFeature(
features::kThirdPartyModulesBlocking);
scoped_feature_list_.InitWithFeatures({features::kThirdPartyModulesBlocking,
quarantine::kOutOfProcessQuarantine},
{});
ASSERT_TRUE(scoped_temp_dir_.CreateUniqueTempDir());
ASSERT_NO_FATAL_FAILURE(
......@@ -108,30 +134,6 @@ class ThirdPartyBlockingBrowserTest : public InProcessBrowserTest {
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.
base::test::ScopedFeatureList scoped_feature_list_;
......@@ -158,22 +160,30 @@ IN_PROC_BROWSER_TEST_F(ThirdPartyBlockingBrowserTest,
if (base::win::GetVersion() < base::win::Version::WIN8)
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.
ThirdPartyRegistryKeyObserver third_party_registry_key_observer;
ASSERT_TRUE(third_party_registry_key_observer.StartWatching());
// Simulate the download of the module list component.
module_database->third_party_conflicts_manager()->LoadModuleList(
module_list_path);
base::RunLoop run_loop;
ModuleDatabase::GetTaskRunner()->PostTask(
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.
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