Get rid of GetMemoryInfo() in favor of using base::ProcessMetrics.

Use a test-specific subclass to isolate the stopgap use of 'ps'
to collect memory stats on Mac.
BUG=none
TEST=none
Review URL: http://codereview.chromium.org/342070

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@31144 0039d316-1c4b-4281-b951-d872f2087c98

8 files changed, 167 insertions, 150 deletions

diff --git a/chrome/test/chrome_process_util.cc b/chrome/test/chrome_process_util.cc
index 4454596..f53cbbd 100644
--- a/chrome/test/chrome_process_util.cc
+++ b/chrome/test/chrome_process_util.cc
@@ -116,3 +116,15 @@ ChromeProcessList GetRunningChromeProcesses(const FilePath& data_dir) {
 
   return result;
 }
+
+#if !defined(OS_MACOSX)
+
+size_t ChromeTestProcessMetrics::GetPagefileUsage() {
+  return process_metrics_->GetPagefileUsage();
+}
+
+size_t ChromeTestProcessMetrics::GetWorkingSetSize() {
+  return process_metrics_->GetWorkingSetSize();
+}
+
+#endif  // !defined(OS_MACOSX)
diff --git a/chrome/test/chrome_process_util.h b/chrome/test/chrome_process_util.h
index 576c2e0..19a5446 100644
--- a/chrome/test/chrome_process_util.h
+++ b/chrome/test/chrome_process_util.h
@@ -9,6 +9,7 @@
 
 #include "base/file_path.h"
 #include "base/process_util.h"
+#include "base/scoped_ptr.h"
 
 typedef std::vector<base::ProcessId> ChromeProcessList;
 
@@ -23,6 +24,48 @@ ChromeProcessList GetRunningChromeProcesses(const FilePath& data_dir);
 // Attempts to terminate all chrome processes associated with |data_dir|.
 void TerminateAllChromeProcesses(const FilePath& data_dir);
 
+// A wrapper class for tests to use in fetching process metrics.
+// Delegates everything we need to base::ProcessMetrics, except
+// memory stats on Mac (which have to parse ps output due to privilege
+// restrictions, behavior we don't want in base).  Long-term, if
+// the production base::ProcessMetrics gets updated to return
+// acceptable metrics on Mac, this class should disappear.
+class ChromeTestProcessMetrics {
+ public:
+  static ChromeTestProcessMetrics* CreateProcessMetrics(
+        base::ProcessHandle process) {
+    return new ChromeTestProcessMetrics(process);
+  }
+
+  size_t GetPagefileUsage();
+
+  size_t GetWorkingSetSize();
+
+  size_t GetPeakPagefileUsage() {
+    return process_metrics_->GetPeakPagefileUsage();
+  }
+
+  size_t GetPeakWorkingSetSize() {
+    return process_metrics_->GetPeakWorkingSetSize();
+  }
+
+  bool GetIOCounters(IoCounters* io_counters) {
+    return process_metrics_->GetIOCounters(io_counters);
+  }
+
+  base::ProcessHandle process_handle_;
+
+ private:
+  explicit ChromeTestProcessMetrics(base::ProcessHandle process) {
+    process_metrics_.reset(base::ProcessMetrics::CreateProcessMetrics(process));
+    process_handle_ = process;
+  }
+
+  scoped_ptr<base::ProcessMetrics> process_metrics_;
+
+  DISALLOW_COPY_AND_ASSIGN(ChromeTestProcessMetrics);
+};
+
 #if defined(OS_MACOSX)
 
 // These types and API are here to fetch the information about a set of running
diff --git a/chrome/test/chrome_process_util_mac.cc b/chrome/test/chrome_process_util_mac.cc
index 9b12e9c..4c60ad3 100644
--- a/chrome/test/chrome_process_util_mac.cc
+++ b/chrome/test/chrome_process_util_mac.cc
@@ -100,3 +100,50 @@ MacChromeProcessInfoList GetRunningMacProcessInfo(
 
   return result;
 }
+
+// Common interface for fetching memory values from parsed ps output.
+// We fill in both values we may get called for, even though our
+// callers typically only care about one, just to keep the code
+// simple and because this is a test.
+static bool GetMemoryValuesHack(uint32 process_id,
+                          size_t* virtual_size,
+                          size_t* working_set_size) {
+  DCHECK(virtual_size && working_set_size);
+
+  std::vector<base::ProcessId> processes;
+  processes.push_back(process_id);
+
+  MacChromeProcessInfoList process_info = GetRunningMacProcessInfo(processes);
+  if (process_info.empty())
+    return false;
+
+  bool found_process = false;
+  *virtual_size = 0;
+  *working_set_size = 0;
+
+  MacChromeProcessInfoList::iterator it = process_info.begin();
+  for (; it != process_info.end(); ++it) {
+    if (it->pid != static_cast<base::ProcessId>(process_id))
+      continue;
+    found_process = true;
+    *virtual_size = it->vsz_in_kb * 1024;
+    *working_set_size = it->rsz_in_kb * 1024;
+    break;
+  }
+
+  return found_process;
+}
+
+size_t ChromeTestProcessMetrics::GetPagefileUsage() {
+  size_t virtual_size;
+  size_t working_set_size;
+  GetMemoryValuesHack(process_handle_, &virtual_size, &working_set_size);
+  return virtual_size;
+}
+
+size_t ChromeTestProcessMetrics::GetWorkingSetSize() {
+  size_t virtual_size;
+  size_t working_set_size;
+  GetMemoryValuesHack(process_handle_, &virtual_size, &working_set_size);
+  return working_set_size;
+}
diff --git a/chrome/test/memory_test/memory_test.cc b/chrome/test/memory_test/memory_test.cc
index aba0a48..3c55d9a 100644
--- a/chrome/test/memory_test/memory_test.cc
+++ b/chrome/test/memory_test/memory_test.cc
@@ -245,16 +245,23 @@ class MemoryTest : public UITest {
 
     ChromeProcessList::const_iterator it;
     for (it = chrome_processes.begin(); it != chrome_processes.end(); ++it) {
-      scoped_ptr<base::ProcessMetrics> process_metrics;
       IoCounters io_counters;
       base::ProcessHandle process_handle;
       if (!base::OpenPrivilegedProcessHandle(*it, &process_handle)) {
         NOTREACHED();
       }
+
+      // TODO(sgk):  if/when base::ProcessMetrics can return real memory
+      // stats on mac, convert to:
+      //
+      // scoped_ptr<base::ProcessMetrics> process_metrics;
+      // process_metrics.reset(
+      //     base::ProcessMetrics::CreateProcessMetrics(process_handle));
+      scoped_ptr<ChromeTestProcessMetrics> process_metrics;
       process_metrics.reset(
-          base::ProcessMetrics::CreateProcessMetrics(process_handle));
-      bzero(&io_counters, sizeof(io_counters));
+          ChromeTestProcessMetrics::CreateProcessMetrics(process_handle));
 
+      bzero(&io_counters, sizeof(io_counters));
       if (process_metrics.get()->GetIOCounters(&io_counters)) {
         std::string chrome_name = (*it == browser_process_pid) ? "_b" : "_r";
 
@@ -297,23 +304,32 @@ class MemoryTest : public UITest {
     size_t browser_working_set_size = 0;
     size_t virtual_size = 0;
     size_t working_set_size = 0;
-    size_t num_chrome_processes = 0;
     ChromeProcessList::const_iterator it;
     for (it = chrome_processes.begin(); it != chrome_processes.end(); ++it) {
-      size_t peak_virtual_size;
-      size_t current_virtual_size;
-      size_t peak_working_set_size;
-      size_t current_working_set_size;
-      if (GetMemoryInfo(*it, &peak_virtual_size, &current_virtual_size,
-                        &peak_working_set_size, &current_working_set_size)) {
-        if (*it == browser_process_pid) {
-          browser_virtual_size = current_virtual_size;
-          browser_working_set_size = current_working_set_size;
-        }
-        virtual_size += current_virtual_size;
-        working_set_size += current_working_set_size;
-        num_chrome_processes++;
+      base::ProcessHandle process_handle;
+      if (!base::OpenPrivilegedProcessHandle(*it, &process_handle)) {
+        NOTREACHED();
+      }
+
+      // TODO(sgk):  if/when base::ProcessMetrics can return real memory
+      // stats on mac, convert to:
+      //
+      // scoped_ptr<base::ProcessMetrics> process_metrics;
+      // process_metrics.reset(
+      //     base::ProcessMetrics::CreateProcessMetrics(process_handle));
+      scoped_ptr<ChromeTestProcessMetrics> process_metrics;
+      process_metrics.reset(
+          ChromeTestProcessMetrics::CreateProcessMetrics(process_handle));
+
+      size_t current_virtual_size = process_metrics->GetPagefileUsage();
+      size_t current_working_set_size = process_metrics->GetWorkingSetSize();
+
+      if (*it == browser_process_pid) {
+        browser_virtual_size = current_virtual_size;
+        browser_working_set_size = current_working_set_size;
       }
+      virtual_size += current_virtual_size;
+      working_set_size += current_working_set_size;
     }
 
     std::string trace_name(test_name);
@@ -330,7 +346,7 @@ class MemoryTest : public UITest {
                 working_set_size / 1024, "kb",
                 true /* important */);
     PrintResult("processes", "", trace_name + "_proc",
-                num_chrome_processes, "",
+                chrome_processes.size(), "",
                 false /* not important */);
   }
 
diff --git a/chrome/test/perf/mem_usage.h b/chrome/test/perf/mem_usage.h
index 7d76a65..90a4824 100644
--- a/chrome/test/perf/mem_usage.h
+++ b/chrome/test/perf/mem_usage.h
@@ -7,35 +7,6 @@
 
 #include "base/basictypes.h"
 
-// Get memory information for the process with given process ID.
-//
-// The Windows psapi provides memory information of a process through structure
-// RPROCESS_MEMORY_COUNTERS_EX. Relevant fields are:
-// PagefileUsage: private (not shared) committed virtual space in process.
-//                This is "VM Size" in task manager processes tab.
-// PeakPagefileUsage: peak value of PagefileUsage.
-// WorkingSetSize: physical memory allocated to process including shared pages.
-//                 This is "Memory Usage" in task manager processes tab.
-//                 Contrary to its name, this value is not much useful for
-//                 tracking the memory demand of an application.
-// PeakWorkingSetSize: peak value of WorkingSetSize.
-//                     This is "Peak Memory Usage" in task manager processes
-//                     tab.
-// PrivateUsage: The current amount of memory that cannot be shared with other
-//               processes. Private bytes include memory that is committed and
-//               marked MEM_PRIVATE, data that is not mapped, and executable
-//               pages that have been written to. It usually gives the same
-//               value as PagefileUsage. No equivalent part in task manager.
-//
-// The measurements we use are PagefileUsage (returned by current_virtual_size)
-// and PeakPagefileUsage (returned by peak_virtual_size), Working Set and
-// Peak working Set.
-bool GetMemoryInfo(uint32 process_id,
-                   size_t* peak_virtual_size,
-                   size_t* current_virtual_size,
-                   size_t* peak_working_set_size,
-                   size_t* current_working_set_size);
-
 // Get the number of bytes currently committed by the system.
 // Returns -1 on failure.
 size_t GetSystemCommitCharge();
diff --git a/chrome/test/perf/mem_usage_linux.cc b/chrome/test/perf/mem_usage_linux.cc
index ec18d01..568b4fc 100644
--- a/chrome/test/perf/mem_usage_linux.cc
+++ b/chrome/test/perf/mem_usage_linux.cc
@@ -25,25 +25,6 @@ const size_t kMemCacheIndex = 10;
 
 }  // namespace
 
-bool GetMemoryInfo(uint32 process_id,
-                   size_t* peak_virtual_size,
-                   size_t* current_virtual_size,
-                   size_t* peak_working_set_size,
-                   size_t* current_working_set_size) {
-  if (!peak_virtual_size || !current_virtual_size)
-    return false;
-
-  base::ProcessMetrics* metrics =
-      base::ProcessMetrics::CreateProcessMetrics(process_id);
-  *peak_virtual_size = metrics->GetPeakPagefileUsage();
-  *current_virtual_size = metrics->GetPagefileUsage();
-  *peak_working_set_size = metrics->GetPeakWorkingSetSize();
-  *current_working_set_size = metrics->GetWorkingSetSize();
-  delete metrics;
-
-  return true;
-}
-
 size_t GetSystemCommitCharge() {
   // Used memory is: total - free - buffers - caches
   FilePath meminfo_file("/proc/meminfo");
diff --git a/chrome/test/perf/mem_usage_mac.cc b/chrome/test/perf/mem_usage_mac.cc
index 4cc9ef6..90d87e5 100644
--- a/chrome/test/perf/mem_usage_mac.cc
+++ b/chrome/test/perf/mem_usage_mac.cc
@@ -12,43 +12,6 @@
 #include "base/process_util.h"
 #include "chrome/test/chrome_process_util.h"
 
-bool GetMemoryInfo(uint32 process_id,
-                   size_t* peak_virtual_size,
-                   size_t* current_virtual_size,
-                   size_t* peak_working_set_size,
-                   size_t* current_working_set_size) {
-  DCHECK(current_virtual_size && current_working_set_size);
-
-  // Mac doesn't support retrieving peak sizes.
-  if (peak_virtual_size)
-    *peak_virtual_size = 0;
-  if (peak_working_set_size)
-    *peak_working_set_size = 0;
-
-  std::vector<base::ProcessId> processes;
-  processes.push_back(process_id);
-
-  MacChromeProcessInfoList process_info = GetRunningMacProcessInfo(processes);
-  if (process_info.empty())
-    return false;
-
-  bool found_process = false;
-  *current_virtual_size = 0;
-  *current_working_set_size = 0;
-
-  MacChromeProcessInfoList::iterator it = process_info.begin();
-  for (; it != process_info.end(); ++it) {
-    if (it->pid != static_cast<base::ProcessId>(process_id))
-      continue;
-    found_process = true;
-    *current_virtual_size = it->vsz_in_kb * 1024;
-    *current_working_set_size = it->rsz_in_kb * 1024;
-    break;
-  }
-
-  return found_process;
-}
-
 // Bytes committed by the system.
 size_t GetSystemCommitCharge() {
   host_name_port_t host = mach_host_self();
diff --git a/chrome/test/perf/mem_usage_win.cc b/chrome/test/perf/mem_usage_win.cc
index 8759c97..465b602 100644
--- a/chrome/test/perf/mem_usage_win.cc
+++ b/chrome/test/perf/mem_usage_win.cc
@@ -10,40 +10,11 @@
 #include "base/file_path.h"
 #include "base/path_service.h"
 #include "base/process_util.h"
+#include "base/scoped_ptr.h"
 #include "chrome/common/chrome_constants.h"
 #include "chrome/common/chrome_paths.h"
 #include "chrome/test/chrome_process_util.h"
 
-bool GetMemoryInfo(uint32 process_id,
-                   size_t* peak_virtual_size,
-                   size_t* current_virtual_size,
-                   size_t* peak_working_set_size,
-                   size_t* current_working_set_size) {
-  if (!peak_virtual_size || !current_virtual_size)
-    return false;
-
-  HANDLE process_handle = OpenProcess(PROCESS_QUERY_INFORMATION |
-                                      PROCESS_VM_READ,
-                                      FALSE, process_id);
-  if (!process_handle)
-    return false;
-
-  PROCESS_MEMORY_COUNTERS_EX pmc;
-  bool result = false;
-  if (GetProcessMemoryInfo(process_handle,
-                           reinterpret_cast<PPROCESS_MEMORY_COUNTERS>(&pmc),
-                           sizeof(pmc))) {
-    *peak_virtual_size = pmc.PeakPagefileUsage;
-    *current_virtual_size = pmc.PagefileUsage;
-    *peak_working_set_size = pmc.PeakWorkingSetSize;
-    *current_working_set_size = pmc.WorkingSetSize;
-    result = true;
-  }
-
-  CloseHandle(process_handle);
-  return result;
-}
-
 // GetPerformanceInfo is not available on WIN2K.  So we'll
 // load it on-the-fly.
 const wchar_t kPsapiDllName[] = L"psapi.dll";
@@ -91,23 +62,36 @@ void PrintChromeMemoryUsageInfo() {
 
   ChromeProcessList::const_iterator it;
   for (it = chrome_processes.begin(); it != chrome_processes.end(); ++it) {
-    size_t peak_virtual_size;
-    size_t current_virtual_size;
-    size_t peak_working_set_size;
-    size_t current_working_set_size;
-    if (GetMemoryInfo(*it, &peak_virtual_size, &current_virtual_size,
-                      &peak_working_set_size, &current_working_set_size)) {
-      if (*it == browser_process_pid) {
-        wprintf(L"browser_vm_peak = %d\n", peak_virtual_size);
-        wprintf(L"browser_vm_current = %d\n", current_virtual_size);
-        wprintf(L"browser_ws_peak = %d\n", peak_working_set_size);
-        wprintf(L"browser_ws_final = %d\n", current_working_set_size);
-      } else {
-        wprintf(L"render_vm_peak = %d\n", peak_virtual_size);
-        wprintf(L"render_vm_current = %d\n", current_virtual_size);
-        wprintf(L"render_ws_peak = %d\n", peak_working_set_size);
-        wprintf(L"render_ws_final = %d\n", current_working_set_size);
-      }
+    base::ProcessHandle process_handle;
+    if (!base::OpenPrivilegedProcessHandle(*it, &process_handle)) {
+      NOTREACHED();
+    }
+
+    // TODO(sgk):  if/when base::ProcessMetrics can return real memory
+    // stats on mac, convert to:
+    //
+    // scoped_ptr<base::ProcessMetrics> process_metrics;
+    // process_metrics.reset(
+    //     base::ProcessMetrics::CreateProcessMetrics(process_handle));
+    scoped_ptr<ChromeTestProcessMetrics> process_metrics;
+    process_metrics.reset(
+        ChromeTestProcessMetrics::CreateProcessMetrics(process_handle));
+
+    size_t peak_virtual_size = process_metrics->GetPeakPagefileUsage();
+    size_t current_virtual_size = process_metrics->GetPagefileUsage();
+    size_t peak_working_set_size = process_metrics->GetPeakWorkingSetSize();
+    size_t current_working_set_size = process_metrics->GetWorkingSetSize();
+
+    if (*it == browser_process_pid) {
+      wprintf(L"browser_vm_peak = %d\n", peak_virtual_size);
+      wprintf(L"browser_vm_current = %d\n", current_virtual_size);
+      wprintf(L"browser_ws_peak = %d\n", peak_working_set_size);
+      wprintf(L"browser_ws_final = %d\n", current_working_set_size);
+    } else {
+      wprintf(L"render_vm_peak = %d\n", peak_virtual_size);
+      wprintf(L"render_vm_current = %d\n", current_virtual_size);
+      wprintf(L"render_ws_peak = %d\n", peak_working_set_size);
+      wprintf(L"render_ws_final = %d\n", current_working_set_size);
     }
   };
 }