TaskManager: use an unordered_map for tracking network usage

chrome/browser/task_manager/sampling/task_manager_impl.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_manager/sampling/task_manager_impl.h"
 
 #include <algorithm>
 #include <string>
 #include <unordered_map>
 #include <unordered_set>
@@ skipping 437 matching lines @@
   // Invalidate the cached sorted IDs by clearing the list.
   sorted_task_ids_.clear();
 }
 
 void TaskManagerImpl::TaskUnresponsive(Task* task) {
   DCHECK(task);
   NotifyObserversOnTaskUnresponsive(task->task_id());
 }
 
 // static
-void TaskManagerImpl::OnMultipleBytesTransferredUI(
-    std::vector<BytesTransferredParam>* params) {
+void TaskManagerImpl::OnMultipleBytesTransferredUI(BytesTransferredMap params) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);

[ncarter (slow), 2017/06/30 20:08:28]

Inside the for loop below, prefer "const auto&" to "auto".

(see "Range based for loops", https://chromium-cpp.appspot.com/)

[cburn, 2017/07/05 16:30:05]

Done.

-  DCHECK(params);
-
-  for (BytesTransferredParam& param : *params) {
-    if (!GetInstance()->UpdateTasksWithBytesTransferred(param)) {
+  for (auto it : params) {

[ncarter (slow), 2017/06/30 20:08:28]

The name |it|, by convention, suggests that this is an iterator. But when you
use a range-based for loop, you actually get not an iterator, but a reference to
an BytesTransferredMap::value_type i.e. "std::pair<BytesTransferredKey,
BytesTransferredParam>&".  (An iterator is constructed under the hood, but it's
completely hidden from you. Conceptually, the loop variable takes on the value
of (*it) at each step.)

So rename this. Ideas: |param| or |bytes_transferred| or |map_entry| or |entry|
or |key_value_pair| or |process_bytes|

[cburn, 2017/07/05 16:30:05]

I went with entry. I may try to go through and change all of params to something
that is more readable about what it actually is.

+    if (!GetInstance()->UpdateTasksWithBytesTransferred(it.first, it.second)) {

[ncarter (slow), 2017/06/30 20:08:28]

The loop body might get more readable if you create these aliases at the top of
the loop:

const BytesTransferredKey& process_info = map_entry.first;
const BytesTransferredParam& bytes_transferred = map_entry.second;

(names totally up to you, and this suggestion is optional.)

[cburn, 2017/07/05 16:30:05]

Done. I am all for making loops more readable. 

       // We can't match a task to the notification.  That might mean the
       // tab that started a download was closed, or the request may have had
       // no originating task associated with it in the first place.
       // We attribute orphaned/unaccounted activity to the Browser process.
-      DCHECK(param.origin_pid || (param.child_id != -1));
-
-      param.origin_pid = 0;
-      param.child_id = param.route_id = -1;
-      GetInstance()->UpdateTasksWithBytesTransferred(param);
+      DCHECK(it.first.origin_pid || (it.first.child_id != -1));
+      // Since the key is meant to be immutable we create a fake key for the
+      // purpose of attributing the orphaned/unaccounted activity to the Browser
+      // process.
+      int dummy_origin_pid = 0;
+      int dummy_child_id = -1;
+      int dummy_route_id = -1;
+      BytesTransferredKey dummy_key = {dummy_origin_pid, dummy_child_id,
+                                       dummy_route_id};
+      GetInstance()->UpdateTasksWithBytesTransferred(dummy_key, it.second);
     }
   }
 }
 
 void TaskManagerImpl::OnVideoMemoryUsageStatsUpdate(
     const gpu::VideoMemoryUsageStats& gpu_memory_stats) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
 
   gpu_memory_stats_ = gpu_memory_stats;
 }
@@ skipping 16 matching lines @@
 
 void TaskManagerImpl::StartUpdating() {
   if (is_running_)
     return;
 
   is_running_ = true;
 
   for (const auto& provider : task_providers_)
     provider->SetObserver(this);
 
-  io_thread_helper_manager_.reset(new IoThreadHelperManager);
+  io_thread_helper_manager_.reset(new IoThreadHelperManager(
+      base::BindRepeating(&TaskManagerImpl::OnMultipleBytesTransferredUI)));
 }
 
 void TaskManagerImpl::StopUpdating() {
   if (!is_running_)
     return;
 
   is_running_ = false;
 
   io_thread_helper_manager_.reset();
 
@@ skipping 11 matching lines @@
   for (const auto& task_provider : task_providers_) {
     Task* task =
         task_provider->GetTaskOfUrlRequest(origin_pid, child_id, route_id);
     if (task)
       return task;
   }
   return nullptr;
 }
 
 bool TaskManagerImpl::UpdateTasksWithBytesTransferred(
+    const BytesTransferredKey& key,
     const BytesTransferredParam& param) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
 
-  Task* task =
-      GetTaskByPidOrRoute(param.origin_pid, param.child_id, param.route_id);
+  Task* task = GetTaskByPidOrRoute(key.origin_pid, key.child_id, key.route_id);
   if (task) {
     task->OnNetworkBytesRead(param.byte_read_count);
     task->OnNetworkBytesSent(param.byte_sent_count);
     return true;
   }
 
   // Couldn't match the bytes to any existing task.
   return false;
 }
 
@@ skipping 15 matching lines @@
         groups_itr.second->AreBackgroundCalculationsDone();
   }
   if (are_all_processes_data_ready) {
     NotifyObserversOnRefreshWithBackgroundCalculations(GetTaskIdsList());
     for (const auto& groups_itr : task_groups_by_proc_id_)
       groups_itr.second->ClearCurrentBackgroundCalculationsFlags();
   }
 }
 
 }  // namespace task_manager