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

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

Index: chrome/browser/task_management/private/tasks_providers/browser_process/browser_process_task_provider.h
diff --git a/chrome/browser/task_management/private/tasks_providers/browser_process/browser_process_task_provider.h b/chrome/browser/task_management/private/tasks_providers/browser_process/browser_process_task_provider.h
new file mode 100644
index 0000000000000000000000000000000000000000..453cbe7e61a16ad87cb9f9e18460baa6d69d15ab
--- /dev/null
+++ b/chrome/browser/task_management/private/tasks_providers/browser_process/browser_process_task_provider.h
@@ -0,0 +1,39 @@
+// 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_PROVIDER_H_
+#define CHROME_BROWSER_TASK_MANAGEMENT_PRIVATE_TASKS_PROVIDERS_BROWSER_PROCESS_BROWSER_PROCESS_TASK_PROVIDER_H_
+
+#include "chrome/browser/task_management/private/tasks_providers/browser_process/browser_process_task.h"
+#include "chrome/browser/task_management/private/tasks_providers/task_provider.h"
+
+namespace task_management {
+
+// This provides the browser process task which lives as long as the browser
+// lives.
+class BrowserProcessTaskProvider : public TaskProvider {
+ public:
+  BrowserProcessTaskProvider();
+  ~BrowserProcessTaskProvider() override;
+
+ private:
+  // task_management::TaskProvider:
+  scoped_refptr<Task> GetTaskOfUrlRequest(int origin_pid,
+                                          int child_id,
+                                          int route_id) override;
+  void StartUpdating() override;
+  void StopUpdating() override;

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

I would not make these "private:". They are public methods of TaskProvider, so
logically they are a part of this class's public interface. And practically, if
I have a BrowserProcessTaskProvider*, I can upcast to the TaskProvider type, so
I can call these methods whether or not you mark them private here.

In my mind, marking a method private is making a promise that I only have to
look for invocations in the corresponding .cc file, and in the .cc files of
declared friends. But that's not true here.

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

But they weren't public in the task provider in the previous patch. They were
protected. In this new patch however I made them private in both TaskProvider
and BrowserProcessTaskProvider.

By the way, it's a convention that I see followed here a lot. for example take a
look at views::View and ash::ActionableView which inherits from it. You'll see
overridden public methods from View are taken protected in ActionableView. There
are many other examples like this. However your comment is convincing.

[ncarter (slow), 2015/03/31 22:31:32]

Apologies, I think I was confusing the inheritance story of Task with that of
TaskProvider. This comment was meant to be about the sort of situation that
exists with Task/BrowserProcessTask, where public methods in the base class
change to private.

What's going on with TaskProvider is okay in the most recent patch. And I've
added a comment on BrowserProcessTask with my concerns there.

+
+// ----------------------------------------------------------------------
+// Fields:
+// ----------------------------------------------------------------------

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

Not needed.

+  scoped_refptr<BrowserProcessTask> browser_process_task_;
+
+  DISALLOW_COPY_AND_ASSIGN(BrowserProcessTaskProvider);
+};
+
+}  // namespace task_management
+
+
+#endif  // CHROME_BROWSER_TASK_MANAGEMENT_PRIVATE_TASKS_PROVIDERS_BROWSER_PROCESS_BROWSER_PROCESS_TASK_PROVIDER_H_