Add unit-test for SharedSampler handling of zero-thread processes.

chrome/browser/task_manager/sampling/shared_sampler_win_defines.h

Index: chrome/browser/task_manager/sampling/shared_sampler_win_defines.h
diff --git a/chrome/browser/task_manager/sampling/shared_sampler_win_defines.h b/chrome/browser/task_manager/sampling/shared_sampler_win_defines.h
new file mode 100644
index 0000000000000000000000000000000000000000..b3990a136186c897cc2b8a2b1448c3018ef10bbf
--- /dev/null
+++ b/chrome/browser/task_manager/sampling/shared_sampler_win_defines.h
@@ -0,0 +1,122 @@
+// Copyright 2016 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROME_BROWSER_TASK_MANAGER_SAMPLING_SHARED_SAMPLER_WIN_DEFINES_H_

[ncarter (slow), 2017/01/31 19:35:03]

win_defines.h or defines_win.h ?

[Wez, 2017/02/01 01:24:45]

I chose win_defines.h because these are the define internal to
shared_sampler_win, rather than generic shared_sampler defines specialized for
Windows; I realize the distinction is subtle but this made more sense to me.

+#define CHROME_BROWSER_TASK_MANAGER_SAMPLING_SHARED_SAMPLER_WIN_DEFINES_H_

[ncarter (slow), 2017/01/31 19:35:03]

This file ought to be listed in chrome/browser/BUILD.gn.

[Wez, 2017/02/01 01:24:45]

Oh, yes - good catch.

+
+#include <windows.h>
+#include <winternl.h>
+
+namespace task_manager {
+
+namespace {

[ncarter (slow), 2017/01/31 19:35:03]

Anonymous namespace in a header file is strange to me. Why not move these to the
task_manager namespace?

[Wez, 2017/02/01 01:24:45]

The unit-test just needs to know the layout of the SYSTEM_PROCESS_INFORMATION
structure; I could duplicate the structure definitions in the unit-test cc file,
in which case they'd be anonymous to shared_sampler_win.cc and to the
unittest.cc, so pulling them out into this header seemed to achieve the same
result without the duplication.

I'm fine with trimming down this header and getting rid of the anonymous
namespace; avoiding exporting symbols unnecessarily just seemed slightly
cleaner.

WDYT?

[ncarter (slow), 2017/02/01 21:55:29]

I'm okay with moving these structs and typedefs to a header file. I'm objecting
to the use of an unnamed namespace in the header file, which is forbidden
generally:
https://google.github.io/styleguide/cppguide.html#Unnamed_Namespaces_and_Stat...

Things should compile the same if you drop the "namespace {", right?

+
+// From <wdm.h>
+typedef LONG KPRIORITY;
+typedef LONG KWAIT_REASON;  // Full definition is in wdm.h
+
+// From ntddk.h
+typedef struct _VM_COUNTERS {
+  SIZE_T PeakVirtualSize;
+  SIZE_T VirtualSize;
+  ULONG PageFaultCount;
+  // Padding here in 64-bit
+  SIZE_T PeakWorkingSetSize;
+  SIZE_T WorkingSetSize;
+  SIZE_T QuotaPeakPagedPoolUsage;
+  SIZE_T QuotaPagedPoolUsage;
+  SIZE_T QuotaPeakNonPagedPoolUsage;
+  SIZE_T QuotaNonPagedPoolUsage;
+  SIZE_T PagefileUsage;
+  SIZE_T PeakPagefileUsage;
+} VM_COUNTERS;
+
+// Two possibilities available from here:
+// http://stackoverflow.com/questions/28858849/where-is-system-information-class-defined
+
+typedef enum _SYSTEM_INFORMATION_CLASS {
+  SystemProcessInformation = 5,  // This is the number that we need.
+} SYSTEM_INFORMATION_CLASS;
+
+// https://msdn.microsoft.com/en-us/library/gg750647.aspx?f=255&MSPPError=-2147217396
+typedef struct {
+  HANDLE UniqueProcess;  // Actually process ID
+  HANDLE UniqueThread;   // Actually thread ID
+} CLIENT_ID;
+
+// From http://alax.info/blog/1182, with corrections and modifications
+// Originally from
+// http://undocumented.ntinternals.net/index.html?page=UserMode%2FUndocumented%20Functions%2FSystem%20Information%2FStructures%2FSYSTEM_THREAD.html
+struct SYSTEM_THREAD_INFORMATION {
+  ULONGLONG KernelTime;
+  ULONGLONG UserTime;
+  ULONGLONG CreateTime;
+  ULONG WaitTime;
+  // Padding here in 64-bit
+  PVOID StartAddress;
+  CLIENT_ID ClientId;
+  KPRIORITY Priority;
+  LONG BasePriority;
+  ULONG ContextSwitchCount;
+  ULONG State;
+  KWAIT_REASON WaitReason;
+};
+#if _M_X64
+static_assert(sizeof(SYSTEM_THREAD_INFORMATION) == 80,
+              "Structure size mismatch");
+#else
+static_assert(sizeof(SYSTEM_THREAD_INFORMATION) == 64,
+              "Structure size mismatch");
+#endif
+
+// From http://alax.info/blog/1182, with corrections and modifications
+// Originally from
+// http://undocumented.ntinternals.net/index.html?page=UserMode%2FUndocumented%20Functions%2FSystem%20Information%2FStructures%2FSYSTEM_THREAD.html
+struct SYSTEM_PROCESS_INFORMATION {
+  ULONG NextEntryOffset;
+  ULONG NumberOfThreads;
+  // http://processhacker.sourceforge.net/doc/struct___s_y_s_t_e_m___p_r_o_c_e_s_s___i_n_f_o_r_m_a_t_i_o_n.html
+  ULONGLONG WorkingSetPrivateSize;
+  ULONG HardFaultCount;
+  ULONG Reserved1;
+  ULONGLONG CycleTime;
+  ULONGLONG CreateTime;
+  ULONGLONG UserTime;
+  ULONGLONG KernelTime;
+  UNICODE_STRING ImageName;
+  KPRIORITY BasePriority;
+  HANDLE ProcessId;
+  HANDLE ParentProcessId;
+  ULONG HandleCount;
+  ULONG Reserved2[2];
+  // Padding here in 64-bit
+  VM_COUNTERS VirtualMemoryCounters;
+  size_t Reserved3;
+  IO_COUNTERS IoCounters;
+  SYSTEM_THREAD_INFORMATION Threads[1];
+};
+#if _M_X64
+static_assert(sizeof(SYSTEM_PROCESS_INFORMATION) == 336,
+              "Structure size mismatch");
+#else
+static_assert(sizeof(SYSTEM_PROCESS_INFORMATION) == 248,
+              "Structure size mismatch");
+#endif
+
+// ntstatus.h conflicts with windows.h so define this locally.
+#define STATUS_SUCCESS ((NTSTATUS)0x00000000L)
+#define STATUS_BUFFER_TOO_SMALL ((NTSTATUS)0xC0000023L)
+#define STATUS_INFO_LENGTH_MISMATCH ((NTSTATUS)0xC0000004L)

[ncarter (slow), 2017/01/31 19:35:03]

I'd leave these in the .cc file, or make them constants.

It looks like you don't need them from the current unittest, and there are
possible collisions, especially on STATUS_SUCCESS, which is used as an enum
value in a couple places in the codebase. Avoiding #defines in headers keeps
things namespaced & modular.

[Wez, 2017/02/01 01:24:46]

Good point; will do.

+
+typedef NTSTATUS(WINAPI* NTQUERYSYSTEMINFORMATION)(
+    SYSTEM_INFORMATION_CLASS SystemInformationClass,
+    PVOID SystemInformation,
+    ULONG SystemInformationLength,
+    PULONG ReturnLength);
+
+}  // namespace
+
+}  // namespace task_manager
+
+#endif  //  CHROME_BROWSER_TASK_MANAGER_SAMPLING_SHARED_SAMPLER_WIN_DEFINES_H_