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

chrome/browser/task_management/providers/task.cc

+// 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.
+
+#include "chrome/browser/task_management/providers/task.h"
+
+namespace task_management {
+
+Task::Task(const base::string16& title,
+           const gfx::ImageSkia& icon,
+           base::ProcessHandle handle)
+    : task_id_(last_id_++),
+      network_usage_(0),
+      current_byte_count_(0),
+      title_(title),
+      icon_(icon),
+      process_handle_(handle),
+      is_first_in_group_(true) {
+}
+
+Task::~Task() {
+}
+
+base::string16 Task::GetProfileName() const {
+  return base::string16();
+}
+
+int Task::GetRoutingID() const {
+  return 0;
+}
+
+size_t Task::GetSqliteMemoryUsed() const {
+  return -1U;
+}
+
+size_t Task::GetV8MemoryAllocated() const {
+  return -1U;
+}
+
+size_t Task::GetV8MemoryUsed() const {
+  return -1U;
+}
+
+bool Task::ReportsWebCacheStats() const {
+  return false;
+}
+
+blink::WebCache::ResourceTypeStats Task::GetWebCacheStats() const {
+  return blink::WebCache::ResourceTypeStats();
+}
+
+void Task::Refresh(const base::TimeDelta& update_duration) {
+  // TODO(afakhry): Add code here to skip this when network usage refresh has
+  // never been requested.
+
+  int64 update_time_secs = update_duration.InSeconds();
+  DCHECK(update_time_secs != 0);
+  network_usage_ = current_byte_count_ / update_time_secs;

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

I really think using a floating point divide here (/ InSecondsF()) is
appropriate. For a 1.5 second duration this computation is needlessly skewed.
For a 0.2 second duration it's a needless divide by zero.

Not sure quite what was up with the original code, but maybe (a) it was trying
to do something like a 1-second windowed moving average for shorter
update_durations and just failed, or maybe (b) it just had a latent bug that we
never hit because the duration only took on a value of 1 second or 2 seconds
exactly...

[afakhry, 2015/03/31 23:22:09]

I don't think that this floating point operation is really needed. There's no
need to handle a situation that will never occur. Our |network_usage_| is
measured in bytes per seconds and it's an int64. Our update duration will be
given from that TaskManager which will get them based on his observers needs,
and the API for providing observers is planned to be:

TaskManager::AddObserver(TaskManagerObserver* observer,
                         int desired_update_interval,
                         int enabled_refresh_flags);

So it's going to be an 'int' as well. I don't think we'll ever need to have an
update interval of 1.2 seconds, right?

[afakhry, 2015/03/31 23:44:14]

We probably might not even need a TimeDelta object to achieve this.

[ncarter (slow), 2015/04/01 16:52:14]

What downside do you see to using InSecondsF()? Why not make this code compute
the average correctly for the range of possible input values, and make it
future-proof?

There's a broad convention of using TimeDeltas instead of ints to represent time
( http://crbug.com/108171 ). That would apply to the second argument of
TaskManager::AddObserver too -- in this example the argument would be that
AddObserver(this, TimeDelta::FromSeconds(1), FLAGS) is going to be more readable
than AddObserver(this, 1, FLAGS) -- it's clear that the 1 is in seconds, not
milliseconds or microseconds.

As for maybe not needing a TimeDelta -- TimeDelta is essentially free, it's just
an int64, really.

+
+  // Reset the current byte count for this task.
+  current_byte_count_ = 0;
+}
+
+void Task::OnBytesRead(int64 bytes_read) {
+  current_byte_count_ += bytes_read;
+}
+
+// static
+int64 Task::last_id_ = 0;
+
+}  // namespace task_management