Index: components/tracing/common/process_metrics_memory_dump_provider.cc |
diff --git a/components/tracing/common/process_metrics_memory_dump_provider.cc b/components/tracing/common/process_metrics_memory_dump_provider.cc |
index 9bfe0a43acb0d9aee1a80b62c7a69da2a90ce532..cbf89c5dbdbc69259924fd191f137db5a0a1a6ed 100644 |
--- a/components/tracing/common/process_metrics_memory_dump_provider.cc |
+++ b/components/tracing/common/process_metrics_memory_dump_provider.cc |
@@ -38,6 +38,14 @@ |
#include "base/numerics/safe_math.h" |
#endif // defined(OS_MACOSX) |
+#if defined(OS_WIN) |
+#include <psapi.h> |
+#include <tchar.h> |
+#include <windows.h> |
+ |
+#include <base/strings/sys_string_conversions.h> |
+#endif |
+ |
namespace tracing { |
namespace { |
@@ -233,6 +241,50 @@ bool ProcessMetricsMemoryDumpProvider::DumpProcessMemoryMaps( |
} |
#endif // defined(OS_LINUX) || defined(OS_ANDROID) |
+#if defined(OS_WIN) |
+bool ProcessMetricsMemoryDumpProvider::DumpProcessMemoryMaps( |
+ const base::trace_event::MemoryDumpArgs& args, |
+ base::trace_event::ProcessMemoryDump* pmd) { |
+ DWORD module_count = 1024; |
Wez
2017/02/23 04:15:11
I would suggest including a brief comment to intro
erikchen
2017/02/23 21:00:14
Done.
awong
2017/02/23 22:05:44
Also, please comment on why 1024 makes sense as a
|
+ std::vector<HMODULE> modules; |
+ while (true) { |
+ modules.resize(module_count); |
+ DWORD output_size; |
Wez
2017/02/23 04:15:11
nit: Even though you're about to overwrite it, ple
erikchen
2017/02/23 21:00:14
Done.
|
+ BOOL result = ::EnumProcessModules(::GetCurrentProcess(), modules.data(), |
+ module_count, &output_size); |
Wez
2017/02/23 04:15:11
You're passing |module_count| here, where the API
erikchen
2017/02/23 21:00:14
fascinating, fixed. Also added a comment + buffer.
|
+ if (!result) |
+ return false; |
+ DWORD actual_module_count = output_size / sizeof(HMODULE); |
+ if (actual_module_count > module_count) { |
+ module_count = actual_module_count; |
+ continue; |
+ } |
+ module_count = actual_module_count; |
+ break; |
Wez
2017/02/23 04:15:11
Can we re-arrange this to read as:
while (true) {
erikchen
2017/02/23 21:00:14
Done.
|
+ } |
+ for (unsigned int i = 0; i < module_count; ++i) { |
Wez
2017/02/23 04:15:11
nit: Since |module_count|'s range is defined by DW
erikchen
2017/02/23 21:00:14
Done.
|
+ TCHAR module_name[MAX_PATH]; |
Wez
2017/02/23 04:15:10
wchar_t
erikchen
2017/02/23 21:00:14
Done.
|
+ BOOL result = GetModuleFileName(modules[i], module_name, MAX_PATH); |
Wez
2017/02/23 04:15:10
nit: You used :: prefix for EnumProcessModules(),
erikchen
2017/02/23 21:00:14
Switched to :: prefixes. The filename is necessary
Wez
2017/02/23 23:35:03
Ah OK; I wondered if it might be important to get
|
+ if (!result) |
+ return false; |
Wez
2017/02/23 04:15:11
This means failing to resolve any module's filenam
erikchen
2017/02/23 21:00:14
Done.
|
+ |
+ MODULEINFO module_info; |
+ result = GetModuleInformation(::GetCurrentProcess(), modules[i], |
+ &module_info, sizeof(MODULEINFO)); |
+ if (!result) |
+ return false; |
Wez
2017/02/23 04:15:11
As above, consider continue'ing here rather than f
erikchen
2017/02/23 21:00:14
Done.
|
+ base::trace_event::ProcessMemoryMaps::VMRegion region; |
+ region.size_in_bytes = module_info.SizeOfImage; |
+ region.mapped_file = base::SysWideToNativeMB(module_name); |
+ region.start_address = reinterpret_cast<uint64_t>(module_info.lpBaseOfDll); |
+ pmd->process_mmaps()->AddVMRegion(region); |
+ } |
+ |
+ pmd->set_has_process_mmaps(); |
Wez
2017/02/23 04:15:11
If you follow my suggestion to use continue's then
erikchen
2017/02/23 21:00:14
Done
|
+ return true; |
+} |
+#endif // defined(OS_WIN) |
+ |
#if defined(OS_MACOSX) |
namespace { |
@@ -547,11 +599,13 @@ bool ProcessMetricsMemoryDumpProvider::OnMemoryDump( |
base::trace_event::ProcessMemoryDump* pmd) { |
bool res = DumpProcessTotals(args, pmd); |
-#if defined(OS_LINUX) || defined(OS_ANDROID) || defined(OS_MACOSX) |
+#if defined(OS_LINUX) || defined(OS_ANDROID) || defined(OS_MACOSX) || \ |
+ defined(OS_WIN) |
Wez
2017/02/23 04:15:11
nit: Are there any other platforms? Or is this no
erikchen
2017/02/23 21:00:14
Nope, fixed.
|
if (args.level_of_detail == |
base::trace_event::MemoryDumpLevelOfDetail::DETAILED) |
res &= DumpProcessMemoryMaps(args, pmd); |
-#endif // defined(OS_LINUX) || defined(OS_ANDROID) || defined(OS_MACOSX) |
+#endif // defined(OS_LINUX) || defined(OS_ANDROID) || defined(OS_MACOSX) || |
+ // defined(OS_WIN) |
return res; |
} |