Commit 60edb79c authored by Patrick Monette's avatar Patrick Monette Committed by Commit Bot

Minor cleanup of OnModuleLoad()

Remove the module load address parameter unused by the ModuleDatabase

Change-Id: I80a41dba4c0f33c9acb0c4d4a03934ad3b19ca54
Reviewed-on: https://chromium-review.googlesource.com/1237215Reviewed-by: default avatarChris Hamilton <chrisha@chromium.org>
Reviewed-by: default avatarScott Violet <sky@chromium.org>
Commit-Queue: Patrick Monette <pmonette@chromium.org>
Cr-Commit-Position: refs/heads/master@{#594776}
parent aac501b9
...@@ -230,8 +230,7 @@ void DetectFaultTolerantHeap() { ...@@ -230,8 +230,7 @@ void DetectFaultTolerantHeap() {
// load event to the ModuleDatabase. // load event to the ModuleDatabase.
void HandleModuleLoadEventWithoutTimeDateStamp( void HandleModuleLoadEventWithoutTimeDateStamp(
const base::FilePath& module_path, const base::FilePath& module_path,
size_t module_size, size_t module_size) {
uintptr_t load_address) {
uint32_t size_of_image = 0; uint32_t size_of_image = 0;
uint32_t time_date_stamp = 0; uint32_t time_date_stamp = 0;
bool got_time_date_stamp = GetModuleImageSizeAndTimeDateStamp( bool got_time_date_stamp = GetModuleImageSizeAndTimeDateStamp(
...@@ -246,9 +245,8 @@ void HandleModuleLoadEventWithoutTimeDateStamp( ...@@ -246,9 +245,8 @@ void HandleModuleLoadEventWithoutTimeDateStamp(
if (!got_time_date_stamp) if (!got_time_date_stamp)
return; return;
ModuleDatabase::GetInstance()->OnModuleLoad(content::PROCESS_TYPE_BROWSER, ModuleDatabase::GetInstance()->OnModuleLoad(
module_path, module_size, content::PROCESS_TYPE_BROWSER, module_path, module_size, time_date_stamp);
time_date_stamp, load_address);
} }
// Helper function for getting the module size associated with a module in this // Helper function for getting the module size associated with a module in this
...@@ -329,8 +327,6 @@ bool TryGetModuleTimeDateStamp(void* module_load_address, ...@@ -329,8 +327,6 @@ bool TryGetModuleTimeDateStamp(void* module_load_address,
// them to the ModuleDatabase. // them to the ModuleDatabase.
void OnModuleEvent(const ModuleWatcher::ModuleEvent& event) { void OnModuleEvent(const ModuleWatcher::ModuleEvent& event) {
auto* module_database = ModuleDatabase::GetInstance(); auto* module_database = ModuleDatabase::GetInstance();
uintptr_t load_address =
reinterpret_cast<uintptr_t>(event.module_load_address);
switch (event.event_type) { switch (event.event_type) {
case mojom::ModuleEventType::MODULE_ALREADY_LOADED: { case mojom::ModuleEventType::MODULE_ALREADY_LOADED: {
...@@ -342,7 +338,7 @@ void OnModuleEvent(const ModuleWatcher::ModuleEvent& event) { ...@@ -342,7 +338,7 @@ void OnModuleEvent(const ModuleWatcher::ModuleEvent& event) {
&time_date_stamp)) { &time_date_stamp)) {
module_database->OnModuleLoad(content::PROCESS_TYPE_BROWSER, module_database->OnModuleLoad(content::PROCESS_TYPE_BROWSER,
event.module_path, event.module_size, event.module_path, event.module_size,
time_date_stamp, load_address); time_date_stamp);
} else { } else {
// Failed to get the TimeDateStamp directly from memory. The next step // Failed to get the TimeDateStamp directly from memory. The next step
// to try is to read the file on disk. This must be done in a blocking // to try is to read the file on disk. This must be done in a blocking
...@@ -352,14 +348,14 @@ void OnModuleEvent(const ModuleWatcher::ModuleEvent& event) { ...@@ -352,14 +348,14 @@ void OnModuleEvent(const ModuleWatcher::ModuleEvent& event) {
{base::MayBlock(), base::TaskPriority::BEST_EFFORT, {base::MayBlock(), base::TaskPriority::BEST_EFFORT,
base::TaskShutdownBehavior::CONTINUE_ON_SHUTDOWN}, base::TaskShutdownBehavior::CONTINUE_ON_SHUTDOWN},
base::Bind(&HandleModuleLoadEventWithoutTimeDateStamp, base::Bind(&HandleModuleLoadEventWithoutTimeDateStamp,
event.module_path, event.module_size, load_address)); event.module_path, event.module_size));
} }
return; return;
} }
case mojom::ModuleEventType::MODULE_LOADED: { case mojom::ModuleEventType::MODULE_LOADED: {
module_database->OnModuleLoad( module_database->OnModuleLoad(
content::PROCESS_TYPE_BROWSER, event.module_path, event.module_size, content::PROCESS_TYPE_BROWSER, event.module_path, event.module_size,
GetModuleTimeDateStamp(event.module_load_address), load_address); GetModuleTimeDateStamp(event.module_load_address));
return; return;
} }
} }
......
...@@ -147,18 +147,16 @@ void ModuleDatabase::OnImeEnumerationFinished() { ...@@ -147,18 +147,16 @@ void ModuleDatabase::OnImeEnumerationFinished() {
void ModuleDatabase::OnModuleLoad(content::ProcessType process_type, void ModuleDatabase::OnModuleLoad(content::ProcessType process_type,
const base::FilePath& module_path, const base::FilePath& module_path,
uint32_t module_size, uint32_t module_size,
uint32_t module_time_date_stamp, uint32_t module_time_date_stamp) {
uintptr_t module_load_address) {
// Messages can arrive from any thread (UI thread for calls over IPC, and // Messages can arrive from any thread (UI thread for calls over IPC, and
// anywhere at all for calls from ModuleWatcher), so bounce if necessary. // anywhere at all for calls from ModuleWatcher), so bounce if necessary.
// It is safe to use base::Unretained() because this class is a singleton that // It is safe to use base::Unretained() because this class is a singleton that
// is never freed. // is never freed.
if (!task_runner_->RunsTasksInCurrentSequence()) { if (!task_runner_->RunsTasksInCurrentSequence()) {
task_runner_->PostTask( task_runner_->PostTask(
FROM_HERE, FROM_HERE, base::Bind(&ModuleDatabase::OnModuleLoad,
base::Bind(&ModuleDatabase::OnModuleLoad, base::Unretained(this), base::Unretained(this), process_type, module_path,
process_type, module_path, module_size, module_size, module_time_date_stamp));
module_time_date_stamp, module_load_address));
return; return;
} }
......
...@@ -102,8 +102,7 @@ class ModuleDatabase : public ModuleDatabaseEventSource { ...@@ -102,8 +102,7 @@ class ModuleDatabase : public ModuleDatabaseEventSource {
void OnModuleLoad(content::ProcessType process_type, void OnModuleLoad(content::ProcessType process_type,
const base::FilePath& module_path, const base::FilePath& module_path,
uint32_t module_size, uint32_t module_size,
uint32_t module_time_date_stamp, uint32_t module_time_date_stamp);
uintptr_t module_load_address);
void OnModuleBlocked(const base::FilePath& module_path, void OnModuleBlocked(const base::FilePath& module_path,
uint32_t module_size, uint32_t module_size,
......
...@@ -34,9 +34,6 @@ constexpr size_t kSize2 = 20 * 4096; ...@@ -34,9 +34,6 @@ constexpr size_t kSize2 = 20 * 4096;
constexpr uint32_t kTime1 = 0xDEADBEEF; constexpr uint32_t kTime1 = 0xDEADBEEF;
constexpr uint32_t kTime2 = 0xBAADF00D; constexpr uint32_t kTime2 = 0xBAADF00D;
constexpr uintptr_t kGoodAddress1 = 0x04000000u;
constexpr uintptr_t kGoodAddress2 = 0x05000000u;
} // namespace } // namespace
class ModuleDatabaseTest : public testing::Test { class ModuleDatabaseTest : public testing::Test {
...@@ -93,16 +90,14 @@ class ModuleDatabaseTest : public testing::Test { ...@@ -93,16 +90,14 @@ class ModuleDatabaseTest : public testing::Test {
TEST_F(ModuleDatabaseTest, TasksAreBounced) { TEST_F(ModuleDatabaseTest, TasksAreBounced) {
// Run a task on the current thread. This should not be bounced, so their // Run a task on the current thread. This should not be bounced, so their
// results should be immediately available. // results should be immediately available.
module_database()->OnModuleLoad(kProcessType1, dll1_, kSize1, kTime1, module_database()->OnModuleLoad(kProcessType1, dll1_, kSize1, kTime1);
kGoodAddress1);
EXPECT_EQ(1u, modules().size()); EXPECT_EQ(1u, modules().size());
// Run similar tasks on another thread with another module. These should be // Run similar tasks on another thread with another module. These should be
// bounced. // bounced.
base::PostTask(FROM_HERE, base::PostTask(FROM_HERE, base::Bind(&ModuleDatabase::OnModuleLoad,
base::Bind(&ModuleDatabase::OnModuleLoad, base::Unretained(module_database()),
base::Unretained(module_database()), kProcessType2, kProcessType2, dll2_, kSize1, kTime1));
dll2_, kSize1, kTime1, kGoodAddress1));
EXPECT_EQ(1u, modules().size()); EXPECT_EQ(1u, modules().size());
RunSchedulerUntilIdle(); RunSchedulerUntilIdle();
EXPECT_EQ(2u, modules().size()); EXPECT_EQ(2u, modules().size());
...@@ -112,8 +107,7 @@ TEST_F(ModuleDatabaseTest, DatabaseIsConsistent) { ...@@ -112,8 +107,7 @@ TEST_F(ModuleDatabaseTest, DatabaseIsConsistent) {
EXPECT_EQ(0u, modules().size()); EXPECT_EQ(0u, modules().size());
// Load a module. // Load a module.
module_database()->OnModuleLoad(kProcessType1, dll1_, kSize1, kTime1, module_database()->OnModuleLoad(kProcessType1, dll1_, kSize1, kTime1);
kGoodAddress1);
EXPECT_EQ(1u, modules().size()); EXPECT_EQ(1u, modules().size());
// Ensure that the process and module sets are up to date. // Ensure that the process and module sets are up to date.
...@@ -123,8 +117,7 @@ TEST_F(ModuleDatabaseTest, DatabaseIsConsistent) { ...@@ -123,8 +117,7 @@ TEST_F(ModuleDatabaseTest, DatabaseIsConsistent) {
m1->second.process_types); m1->second.process_types);
// Provide a redundant load message for that module. // Provide a redundant load message for that module.
module_database()->OnModuleLoad(kProcessType1, dll1_, kSize1, kTime1, module_database()->OnModuleLoad(kProcessType1, dll1_, kSize1, kTime1);
kGoodAddress1);
EXPECT_EQ(1u, modules().size()); EXPECT_EQ(1u, modules().size());
// Ensure that the process and module sets haven't changed. // Ensure that the process and module sets haven't changed.
...@@ -133,8 +126,7 @@ TEST_F(ModuleDatabaseTest, DatabaseIsConsistent) { ...@@ -133,8 +126,7 @@ TEST_F(ModuleDatabaseTest, DatabaseIsConsistent) {
m1->second.process_types); m1->second.process_types);
// Load a second module into the process. // Load a second module into the process.
module_database()->OnModuleLoad(kProcessType1, dll2_, kSize2, kTime2, module_database()->OnModuleLoad(kProcessType1, dll2_, kSize2, kTime2);
kGoodAddress2);
EXPECT_EQ(2u, modules().size()); EXPECT_EQ(2u, modules().size());
// Ensure that the process and module sets are up to date. // Ensure that the process and module sets are up to date.
...@@ -144,8 +136,7 @@ TEST_F(ModuleDatabaseTest, DatabaseIsConsistent) { ...@@ -144,8 +136,7 @@ TEST_F(ModuleDatabaseTest, DatabaseIsConsistent) {
m2->second.process_types); m2->second.process_types);
// Load the dummy.dll in the second process as well. // Load the dummy.dll in the second process as well.
module_database()->OnModuleLoad(kProcessType2, dll1_, kSize1, kTime1, module_database()->OnModuleLoad(kProcessType2, dll1_, kSize1, kTime1);
kGoodAddress1);
EXPECT_EQ(ProcessTypeToBit(content::PROCESS_TYPE_BROWSER) | EXPECT_EQ(ProcessTypeToBit(content::PROCESS_TYPE_BROWSER) |
ProcessTypeToBit(content::PROCESS_TYPE_RENDERER), ProcessTypeToBit(content::PROCESS_TYPE_RENDERER),
m1->second.process_types); m1->second.process_types);
...@@ -196,8 +187,7 @@ TEST_F(ModuleDatabaseTest, Observers) { ...@@ -196,8 +187,7 @@ TEST_F(ModuleDatabaseTest, Observers) {
module_database()->AddObserver(&before_load_observer); module_database()->AddObserver(&before_load_observer);
EXPECT_EQ(0, before_load_observer.new_module_count()); EXPECT_EQ(0, before_load_observer.new_module_count());
module_database()->OnModuleLoad(kProcessType1, dll1_, kSize1, kTime1, module_database()->OnModuleLoad(kProcessType1, dll1_, kSize1, kTime1);
kGoodAddress1);
RunSchedulerUntilIdle(); RunSchedulerUntilIdle();
EXPECT_EQ(1, before_load_observer.new_module_count()); EXPECT_EQ(1, before_load_observer.new_module_count());
...@@ -231,8 +221,7 @@ TEST_F(ModuleDatabaseTest, OnKnownModuleLoaded) { ...@@ -231,8 +221,7 @@ TEST_F(ModuleDatabaseTest, OnKnownModuleLoaded) {
EXPECT_EQ(0, dummy_observer.known_module_loaded_count()); EXPECT_EQ(0, dummy_observer.known_module_loaded_count());
// Pretend the shell extension loads. // Pretend the shell extension loads.
module_database()->OnModuleLoad(kProcessType1, dll1_, kSize1, kTime1, module_database()->OnModuleLoad(kProcessType1, dll1_, kSize1, kTime1);
kGoodAddress1);
RunSchedulerUntilIdle(); RunSchedulerUntilIdle();
EXPECT_EQ(1, dummy_observer.new_module_count()); EXPECT_EQ(1, dummy_observer.new_module_count());
...@@ -255,8 +244,7 @@ TEST_F(ModuleDatabaseTest, IsIdle) { ...@@ -255,8 +244,7 @@ TEST_F(ModuleDatabaseTest, IsIdle) {
EXPECT_FALSE(module_database()->IsIdle()); EXPECT_FALSE(module_database()->IsIdle());
// A load module event starts the timer. // A load module event starts the timer.
module_database()->OnModuleLoad(kProcessType1, dll1_, kSize1, kTime1, module_database()->OnModuleLoad(kProcessType1, dll1_, kSize1, kTime1);
kGoodAddress1);
EXPECT_FALSE(module_database()->IsIdle()); EXPECT_FALSE(module_database()->IsIdle());
FastForwardToIdleTimer(); FastForwardToIdleTimer();
...@@ -277,8 +265,7 @@ TEST_F(ModuleDatabaseTest, IsIdle) { ...@@ -277,8 +265,7 @@ TEST_F(ModuleDatabaseTest, IsIdle) {
module_database()->RemoveObserver(&is_idle_observer); module_database()->RemoveObserver(&is_idle_observer);
// Make the ModuleDabatase busy. // Make the ModuleDabatase busy.
module_database()->OnModuleLoad(kProcessType2, dll2_, kSize2, kTime2, module_database()->OnModuleLoad(kProcessType2, dll2_, kSize2, kTime2);
kGoodAddress2);
EXPECT_FALSE(module_database()->IsIdle()); EXPECT_FALSE(module_database()->IsIdle());
// Adding an observer while busy doesn't. // Adding an observer while busy doesn't.
...@@ -302,8 +289,7 @@ TEST_F(ModuleDatabaseTest, WaitUntilRegisteredModulesEnumerated) { ...@@ -302,8 +289,7 @@ TEST_F(ModuleDatabaseTest, WaitUntilRegisteredModulesEnumerated) {
module_database()->AddObserver(&before_load_observer); module_database()->AddObserver(&before_load_observer);
EXPECT_EQ(0, before_load_observer.new_module_count()); EXPECT_EQ(0, before_load_observer.new_module_count());
module_database()->OnModuleLoad(kProcessType1, dll1_, kSize1, kTime1, module_database()->OnModuleLoad(kProcessType1, dll1_, kSize1, kTime1);
kGoodAddress1);
FastForwardToIdleTimer(); FastForwardToIdleTimer();
// Idle state is prevented. // Idle state is prevented.
......
...@@ -134,7 +134,7 @@ void HandleModuleEvent(ModuleDatabase* module_database, ...@@ -134,7 +134,7 @@ void HandleModuleEvent(ModuleDatabase* module_database,
// Forward this to the module database. // Forward this to the module database.
module_database->OnModuleLoad(process_type, module_path, module_size, module_database->OnModuleLoad(process_type, module_path, module_size,
module_time_date_stamp, load_address); module_time_date_stamp);
} }
} // namespace } // namespace
......
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