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

chrome/chrome_browser.gypi

Index: chrome/chrome_browser.gypi
diff --git a/chrome/chrome_browser.gypi b/chrome/chrome_browser.gypi
index d96e3b9c2dfb3f7d7288e4ba5c456acebd50af1a..7e0ac109074266bb8299218ad9d9369e5a51a86e 100644
--- a/chrome/chrome_browser.gypi
+++ b/chrome/chrome_browser.gypi
@@ -2843,6 +2843,18 @@
       'browser/sync/sync_type_preference_provider.h',
     ],
     'chrome_browser_task_manager_sources': [
+      # New Task Manager Sources:
+      'browser/task_management/providers/browser_process/browser_process_task_provider.cc',

[Lei Zhang, 2015/04/01 22:47:52]

Do you need to have a browser_process sub-directory, and files within it named
browser_process_foo? Maybe just drop the sub-directory?

ls chrome/browser|wc -l returns 349
and
ls chrome/browser/extensions|wc -l return 519

and nobody is crying yet. (AFAIK)

[afakhry, 2015/04/03 01:51:01]

The idea was to keep the private interfaces in the top directory and related
implementation files are grouped together in sub-directories.

[Lei Zhang, 2015/04/03 02:39:37]

OTOH, #include
"chrome/browser/task_management/providers/browser_process/browser_process_task_provider.h"
doesn't fit within 80 columns. I think you are trying way too hard to give your
files hierarchy.

how about
chrome/browser/task_management/providers/browser_process_task_provider_impl.h?
One can easily spot the interface files as the shorter named files without _impl
in their names.

[ncarter (slow), 2015/04/03 04:09:03]

FWIW, no levels of hierarchy after providers/ was my suggestion as well earlier
in the review. I still would prefer at most one level of directory hierarchy,
because the convention in chrome seems to be a relatively flat organization.

I'm not sure I like the _impl pattern that Lei suggests. Usually when we have
foo_impl.cc it's the sole non-test implementation of the interface foo.h. In
this case we have many implementations of the taskprovider and task interfaces;
seems clumsy to me to call them all 'Impls.

I'm not bothered by the idea of task.h living alongside browser_process_task.h
and web_contents_task.h.

[afakhry, 2015/04/03 16:04:27]

Ok, I yielded although I'm not too happy about it :(.
At least for web_contents in the future I strongly believe they should have
their own sub-dir. There will be a lot of related files.

+      'browser/task_management/providers/browser_process/browser_process_task_provider.h',
+      'browser/task_management/providers/browser_process/browser_process_task.cc',
+      'browser/task_management/providers/browser_process/browser_process_task.h',
+      'browser/task_management/providers/task_provider_observer.h',
+      'browser/task_management/providers/task_provider.cc',
+      'browser/task_management/providers/task_provider.h',
+      'browser/task_management/providers/task.cc',
+      'browser/task_management/providers/task.h',
+      
+      # Old Task Manager Sources:
       'browser/task_manager/background_information.cc',
       'browser/task_manager/background_information.h',
       'browser/task_manager/browser_process_resource_provider.cc',