Include tab IDs when reporting data use accounting.

chrome/browser/net/chrome_network_delegate.cc

Index: chrome/browser/net/chrome_network_delegate.cc
diff --git a/chrome/browser/net/chrome_network_delegate.cc b/chrome/browser/net/chrome_network_delegate.cc
index 8547e4911c167441278fdf0d85cf53f286932f25..bd8a827afaa6e91e3de01c5c8cbe8faa5327c78a 100644
--- a/chrome/browser/net/chrome_network_delegate.cc
+++ b/chrome/browser/net/chrome_network_delegate.cc
@@ -33,6 +33,7 @@
 #include "chrome/browser/net/request_source_bandwidth_histograms.h"
 #include "chrome/browser/net/safe_search_util.h"
 #include "chrome/browser/profiles/profile_manager.h"
+#include "chrome/browser/sessions/session_tab_helper.h"
 #include "chrome/browser/task_management/task_manager_interface.h"
 #include "chrome/common/pref_names.h"
 #include "components/content_settings/core/browser/cookie_settings.h"
@@ -42,6 +43,7 @@
 #include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/render_view_host.h"
 #include "content/public/browser/resource_request_info.h"
+#include "content/public/browser/web_contents.h"
 #include "content/public/common/content_switches.h"
 #include "content/public/common/process_type.h"
 #include "net/base/host_port_pair.h"
@@ -54,6 +56,7 @@
 #include "net/http/http_status_code.h"
 #include "net/log/net_log.h"
 #include "net/url_request/url_request.h"
+#include "url/gurl.h"
 
 #if defined(OS_ANDROID)
 #include "chrome/browser/io_thread.h"
@@ -279,6 +282,31 @@ void RecordCacheStateStats(const net::URLRequest* request) {
   }
 }
 
+void RunTabIdCallback(const base::Callback<void(int32_t)>& tab_id_callback,
+                      int32_t tab_id) {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  tab_id_callback.Run(tab_id);
+}
+
+// Attempt to get the associated tab ID for a given render frame, and pass it to
+// |tab_id_callback| on the IO thread. A tab ID of -1 is passed if no associated
+// tab was found.
+void ProvideTabIdFromUIThread(
+    int render_process_id,
+    int render_frame_id,
+    const base::Callback<void(int32_t)>& tab_id_callback) {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  // TODO(sclittle): For prerendering tabs, investigate if it's possible to find
+  // the original tab that initiated the prerender.
+  int32_t tab_id =
+      SessionTabHelper::IdForTab(content::WebContents::FromRenderFrameHost(
+          content::RenderFrameHost::FromID(render_process_id,
+                                           render_frame_id)));
+  BrowserThread::PostTask(
+      BrowserThread::IO, FROM_HERE,
+      base::Bind(&RunTabIdCallback, tab_id_callback, tab_id));
+}
+
 }  // namespace
 
 ChromeNetworkDelegate::ChromeNetworkDelegate(
@@ -502,28 +530,12 @@ void ChromeNetworkDelegate::OnNetworkBytesReceived(
                                                         bytes_received);
 #endif  // defined(ENABLE_TASK_MANAGER)
 
-  if (data_use_aggregator_) {
-    if (is_data_usage_off_the_record_) {
-      data_use_aggregator_->ReportOffTheRecordDataUse(0 /* tx_bytes */,
-                                                      bytes_received);
-    } else {
-      data_use_aggregator_->ReportDataUse(request, -1 /* tab_id */,
-                                          0 /* tx_bytes */, bytes_received);
-    }
-  }
+  ReportDataUsageStats(request, 0 /* tx_bytes */, bytes_received);
 }
 
 void ChromeNetworkDelegate::OnNetworkBytesSent(const net::URLRequest& request,
                                                int64_t bytes_sent) {
-  if (data_use_aggregator_) {
-    if (is_data_usage_off_the_record_) {
-      data_use_aggregator_->ReportOffTheRecordDataUse(bytes_sent,
-                                                      0 /* rx_bytes */);
-    } else {
-      data_use_aggregator_->ReportDataUse(request, -1 /* tab_id */, bytes_sent,
-                                          0 /* rx_bytes */);
-    }
-  }
+  ReportDataUsageStats(request, bytes_sent, 0 /* rx_bytes */);
 }
 
 void ChromeNetworkDelegate::OnCompleted(net::URLRequest* request,
@@ -742,3 +754,38 @@ bool ChromeNetworkDelegate::OnCancelURLRequestWithPolicyViolatingReferrerHeader(
   ReportInvalidReferrerSend(target_url, referrer_url);
   return true;
 }
+
+void ChromeNetworkDelegate::ReportDataUsageStats(const net::URLRequest& request,
+                                                 int64_t tx_bytes,
+                                                 int64_t rx_bytes) {
+  if (!data_use_aggregator_)
+    return;
+
+  if (is_data_usage_off_the_record_) {
+    data_use_aggregator_->ReportOffTheRecordDataUse(tx_bytes, rx_bytes);
+    return;
+  }
+
+  net::LoadTimingInfo load_timing_info;
+  request.GetLoadTimingInfo(&load_timing_info);
+
+  base::Callback<void(int32_t)> report_data_use_with_tab_id_callback =
+      base::Bind(&data_usage::DataUseAggregator::ReportDataUse,
+                 data_use_aggregator_->GetWeakPtr(), tx_bytes, rx_bytes,
+                 request.url(), load_timing_info.request_start,
+                 request.first_party_for_cookies());
+
+  int render_process_id = -1, render_frame_id = -1;
+  if (!content::ResourceRequestInfo::GetRenderFrameForRequest(
+          &request, &render_process_id, &render_frame_id)) {
+    // Run the callback immediately if the request has no render frame.
+    report_data_use_with_tab_id_callback.Run(-1 /* tab_id */);
+    return;
+  }
+
+  // Hop to the UI thread and back to get the tab ID for the render frame.
+  BrowserThread::PostTask(
+      BrowserThread::UI, FROM_HERE,
+      base::Bind(&ProvideTabIdFromUIThread, render_process_id, render_frame_id,
+                 report_data_use_with_tab_id_callback));

[mmenke, 2015/10/28 22:43:12]

I don't think we want post two tasks every time a request receives data.  This
seems way too intrusive.  I'd be less concerned if you just did this when a
request completes or is redirected.

We also want to avoid more chrome/ dependencies on these IDs - long term, we
want to completely hide them from chrome/, and have content run the callbacks
that care on the IO thread.

[sclittle, 2015/10/28 23:04:56]

Which IDs are to be avoided, e.g. Tab IDs or render_process/frame_id? Do you
know of any other way of getting the tab ID for a URLRequest?

For the performance hit, would it be acceptable if the IO->UI->IO hop was only
done for the first chunk of data received for a URLRequest, then re-used for
later chunks for the same URLRequest?

[mmenke, 2015/10/28 23:18:42]

Apologies, I meant render process / frame IDs.  The plan is to pass the tab ID
to ResourceThrottle stack, which will ideally be moved to the UI thread (So
always incurring two extra thread hops, but only on start, redirect, headers
received, and auth challenges).  I'd be less concerned if we plugged this in as
a ResourceThrottle, but throttles don't currently get notified of completion,
which is a problem.
Yes, I'd be happy with that - unfortunately, unless we delay the request on the
first hop, not sure how we'd get it.  Let me think about this a bit tomorrow.

[bengr, 2015/10/30 15:56:07]

Is your concern that the request might not outlive the thread hops? 

+}