New Task Manager - Phase 1.1: Implement Browser Process Task Providing

chrome/browser/task_management/private/tasks_providers/browser_process/browser_process_task.h

Index: chrome/browser/task_management/private/tasks_providers/browser_process/browser_process_task.h
diff --git a/chrome/browser/task_management/private/tasks_providers/browser_process/browser_process_task.h b/chrome/browser/task_management/private/tasks_providers/browser_process/browser_process_task.h
new file mode 100644
index 0000000000000000000000000000000000000000..f26951f1c894c1af8750cc97fd2320a7916a5414
--- /dev/null
+++ b/chrome/browser/task_management/private/tasks_providers/browser_process/browser_process_task.h
@@ -0,0 +1,40 @@
+// Copyright 2015 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_MANAGEMENT_PRIVATE_TASKS_PROVIDERS_BROWSER_PROCESS_BROWSER_PROCESS_TASK_H_
+#define CHROME_BROWSER_TASK_MANAGEMENT_PRIVATE_TASKS_PROVIDERS_BROWSER_PROCESS_BROWSER_PROCESS_TASK_H_
+
+#include "chrome/browser/task_management/private/tasks_providers/task.h"

[ncarter (slow), 2015/03/27 18:16:20]

Three additional levels of hierarchical organization seems excessive here. Can
we figure out a way to do this with at most one additional level of directory
structure? Spend a little time clicking around src/chrome/browser, and notice
how flat things are on average. Let's try to be consistent with that.

The goal of putting the implementation bits in a private/ directory, presumably,
is to make it easier for readers of the code to identify the public-facing API
and not waste time looking at the internals? That goal is achieved only if
private/ is understood by convention to have that meaning ... and I do not think
it does. Apart from the extensions private APIs (in which context "private"
means a privilege granted only to trusted or built-in extensions), src/chrome
has zero uses of "private" as a directory name like this.

One convention we do have that helps us find the public entry point for a bunch
of C++ files, in Google C++ style, is this: chrome/browser/foo_bar/foo_bar.h is
the probably the class you'd start looking at.

In truth, if you want to hide the guts, throwing them in a subdirectory does the
job. The subdirectory doesn't need to announce "no visitors" by calling itself
"private"; just give it a descriptive name. I.e., you don't need
task_manager/private/intestines, just call it task_manager/intestines, and
people will probably not be interested in looking at the intestines.

ALSO: when thinking of directory and class names, I usually think it's helpful
to limit repetitions of a single word. In the name of this class, for instance,
"browser_process" appears twice and "task" appears three times.

So maybe, could this be just
.../task_management/providers/browser_process_task.h ? Something like that?

[afakhry, 2015/03/31 00:19:19]

Modified as agreed!

+
+namespace task_management {
+
+class BrowserProcessTask : public Task {
+ public:
+  BrowserProcessTask();
+
+ private:
+  ~BrowserProcessTask() override;
+
+  // task_management::Task:
+  Type GetType() const override;
+  int GetChildProcessUniqueID() const override;
+  size_t GetSqliteMemoryUsed() const override;
+  size_t GetV8MemoryAllocated() const override;
+  size_t GetV8MemoryUsed() const override;
+  void Refresh(const base::TimeDelta& update_time) override;
+
+// ----------------------------------------------------------------------
+// Fields:
+// ----------------------------------------------------------------------

[ncarter (slow), 2015/03/27 18:16:20]

In Google C++ style (and so, Chromium style) we don't really need a "// Fields:"
block comment. The data members of a class always come last in the "private:"
section, so you know to look for them there.

If you want a comment here (e.g. for the sake of having vertical whitespace), a
better choice is to comment the meaning of the data member, like:

  // The task representing the browser's one and only main process.

[afakhry, 2015/03/31 00:19:19]

Removed.

+  size_t allocated_v8_memory_;
+  size_t used_v8_memory_;
+  size_t used_sqlite_memory_;
+  bool reports_v8_stats_;
+
+  DISALLOW_COPY_AND_ASSIGN(BrowserProcessTask);
+};
+
+}  // namespace task_management
+
+#endif  // CHROME_BROWSER_TASK_MANAGEMENT_PRIVATE_TASKS_PROVIDERS_BROWSER_PROCESS_BROWSER_PROCESS_TASK_H_