Commit ea0fc396 authored by Ovidio de Jesús Ruiz-Henríquez's avatar Ovidio de Jesús Ruiz-Henríquez Committed by Commit Bot

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

This reverts commit 57213418.

Reason for revert: Causing compile error in win-chrome builder: https://ci.chromium.org/p/chrome/builders/ci/win-chrome

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}

TBR=chrisha@chromium.org,pmonette@chromium.org

Change-Id: I6067a3b50bb835025807ff1b81e7787cda35625d
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 850517
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2033634Reviewed-by: default avatarOvidio de Jesús Ruiz-Henríquez <odejesush@chromium.org>
Commit-Queue: Ovidio de Jesús Ruiz-Henríquez <odejesush@chromium.org>
Cr-Commit-Position: refs/heads/master@{#737480}
parent e49f21f3
...@@ -12,7 +12,6 @@ ...@@ -12,7 +12,6 @@
#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"
...@@ -28,7 +27,6 @@ ...@@ -28,7 +27,6 @@
#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,
...@@ -94,10 +92,8 @@ class IncompatibleApplicationsBrowserTest : public InProcessBrowserTest { ...@@ -94,10 +92,8 @@ 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_.InitWithFeatures( scoped_feature_list_.InitAndEnableFeature(
{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());
...@@ -202,27 +198,18 @@ IN_PROC_BROWSER_TEST_F(IncompatibleApplicationsBrowserTest, ...@@ -202,27 +198,18 @@ 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>();
base::RunLoop run_loop; // Simulate the download of the module list component.
ModuleDatabase::GetTaskRunner()->PostTask( module_database->third_party_conflicts_manager()->LoadModuleList(
FROM_HERE, GetModuleListPath());
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,7 +118,6 @@ scoped_refptr<base::SequencedTaskRunner> ModuleDatabase::GetTaskRunner() { ...@@ -118,7 +118,6 @@ 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,7 +10,6 @@ ...@@ -10,7 +10,6 @@
#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"
...@@ -24,7 +23,6 @@ ...@@ -24,7 +23,6 @@
#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 {
...@@ -78,29 +76,6 @@ class ThirdPartyRegistryKeyObserver { ...@@ -78,29 +76,6 @@ 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;
...@@ -108,9 +83,8 @@ class ThirdPartyBlockingBrowserTest : public InProcessBrowserTest { ...@@ -108,9 +83,8 @@ class ThirdPartyBlockingBrowserTest : public InProcessBrowserTest {
// InProcessBrowserTest: // InProcessBrowserTest:
void SetUp() override { void SetUp() override {
scoped_feature_list_.InitWithFeatures({features::kThirdPartyModulesBlocking, scoped_feature_list_.InitAndEnableFeature(
quarantine::kOutOfProcessQuarantine}, features::kThirdPartyModulesBlocking);
{});
ASSERT_TRUE(scoped_temp_dir_.CreateUniqueTempDir()); ASSERT_TRUE(scoped_temp_dir_.CreateUniqueTempDir());
ASSERT_NO_FATAL_FAILURE( ASSERT_NO_FATAL_FAILURE(
...@@ -134,6 +108,30 @@ class ThirdPartyBlockingBrowserTest : public InProcessBrowserTest { ...@@ -134,6 +108,30 @@ 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_;
...@@ -160,30 +158,22 @@ IN_PROC_BROWSER_TEST_F(ThirdPartyBlockingBrowserTest, ...@@ -160,30 +158,22 @@ IN_PROC_BROWSER_TEST_F(ThirdPartyBlockingBrowserTest,
if (base::win::GetVersion() < base::win::Version::WIN8) if (base::win::GetVersion() < base::win::Version::WIN8)
return; return;
// Create the observer early so the change is guaranteed to be observed. base::FilePath module_list_path;
ThirdPartyRegistryKeyObserver third_party_registry_key_observer; ASSERT_NO_FATAL_FAILURE(CreateModuleList(&module_list_path));
ASSERT_TRUE(third_party_registry_key_observer.StartWatching()); ASSERT_FALSE(module_list_path.empty());
base::RunLoop run_loop; ModuleDatabase* module_database = ModuleDatabase::GetInstance();
ModuleDatabase::GetTaskRunner()->PostTask(
FROM_HERE,
base::BindLambdaForTesting([quit_closure = run_loop.QuitClosure()]() {
ModuleDatabase* module_database = ModuleDatabase::GetInstance();
// Speed up the test. // Speed up the test.
module_database->IncreaseInspectionPriority(); module_database->IncreaseInspectionPriority();
base::FilePath module_list_path; // Create the observer early so the change is guaranteed to be observed.
ASSERT_NO_FATAL_FAILURE(CreateModuleList(&module_list_path)); ThirdPartyRegistryKeyObserver third_party_registry_key_observer;
ASSERT_FALSE(module_list_path.empty()); 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);
quit_closure.Run(); // Simulate the download of the module list component.
})); module_database->third_party_conflicts_manager()->LoadModuleList(
run_loop.Run(); module_list_path);
// 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