Implement basic support for Windows module emission.

components/tracing/common/process_metrics_memory_dump_provider.cc

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..ecb44999588f7821d4f4c3a0d6c8abe83cd69586 100644
--- a/components/tracing/common/process_metrics_memory_dump_provider.cc
+++ b/components/tracing/common/process_metrics_memory_dump_provider.cc
@@ -38,6 +38,15 @@
 #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>
+#include <base/win/win_util.h>
+#endif

[Wez, 2017/02/23 23:35:03]

nit: // defined(OS_WIN) here, since the block is getting long.

[erikchen, 2017/02/24 17:32:48]

Done.

+
 namespace tracing {
 
 namespace {
@@ -233,6 +242,40 @@ 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) {
+  std::vector<HMODULE> modules;
+  bool result =
+      base::win::GetLoadedModulesSnapshot(::GetCurrentProcess(), &modules);
+  if (!result)

[Wez, 2017/02/23 23:35:03]

nit: Do you need |result|, or can you just if (!GetLoadedModulesSnapshot()) (and
similarly for the conditionals below)?

[erikchen, 2017/02/24 17:32:48]

Done.

+    return false;
+
+  // Query the base address for each module, and attach it to the dump.
+  for (size_t i = 0; i < modules.size(); ++i) {
+    wchar_t module_name[MAX_PATH];
+    BOOL result = ::GetModuleFileName(modules[i], module_name, MAX_PATH);

[etienneb, 2017/02/24 16:37:15]

nit: I agree with wez@ on the previous message.
You have a variable shadowed here (result) with a different type.
This is valid, but not a good practice.


Also,
  GetModuleFileName -> DWORD
    If the function succeeds, the return value is the length of the string that
is copied to the buffer, in characters,
  GetModuleInformation -> BOOL

[etienneb, 2017/02/24 17:06:43]

I remember a corner case with the function GetModuleFileName.

If you read the doc:
https://msdn.microsoft.com/en-us/library/windows/desktop/ms683197(v=vs.85).aspx

"""
Return value

[...] If the buffer is too small to hold the module name, the string is
truncated to nSize characters including the terminating null character [...]
"""

This is why it's implemented that way:
  https://cs.chromium.org/chromium/src/content/common/sandbox_win.cc?dr=C&l=175

Which mean the mapped_filename can be truncated.
  region.mapped_file = base::SysWideToNativeMB(module_name);

We should also note that MAX_PATH is not an hard limit, with unicode paths can
be longer.
  see:
https://msdn.microsoft.com/en-ca/library/windows/desktop/aa365247(v=vs.85).aspx
  read section: Maximum Path Length Limitation
    "The Windows API has many functions that also have Unicode versions to
permit an extended-length path for a maximum total path length of 32,767
characters."

These cases are rare and we can ignore them for now.
I remember seeing malwares evading our detection system using these tricks.

[erikchen, 2017/02/24 17:32:48]

Noted. It looks like we mostly ignore this problem in our code base.

+    if (!result)
+      continue;
+
+    MODULEINFO module_info;
+    result = ::GetModuleInformation(::GetCurrentProcess(), modules[i],
+                                    &module_info, sizeof(MODULEINFO));
+    if (!result)
+      continue;
+    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);
+  }
+  if (!pmd->process_mmaps()->vm_regions().empty())
+    pmd->set_has_process_mmaps();
+  return true;
+}
+#endif  // defined(OS_WIN)
+
 #if defined(OS_MACOSX)
 
 namespace {
@@ -547,11 +590,9 @@ bool ProcessMetricsMemoryDumpProvider::OnMemoryDump(
     base::trace_event::ProcessMemoryDump* pmd) {
   bool res = DumpProcessTotals(args, pmd);
 
-#if defined(OS_LINUX) || defined(OS_ANDROID) || defined(OS_MACOSX)
   if (args.level_of_detail ==
       base::trace_event::MemoryDumpLevelOfDetail::DETAILED)
     res &= DumpProcessMemoryMaps(args, pmd);
-#endif  // defined(OS_LINUX) || defined(OS_ANDROID) || defined(OS_MACOSX)
   return res;
 }