Record the tab state during data use

chrome/browser/data_use_measurement/chrome_data_use_ascriber.cc

Index: chrome/browser/data_use_measurement/chrome_data_use_ascriber.cc
diff --git a/chrome/browser/data_use_measurement/chrome_data_use_ascriber.cc b/chrome/browser/data_use_measurement/chrome_data_use_ascriber.cc
index c67b8de820268891065211f71a6b5e9e1f5d010f..8eb84db3a5a3319109442536a9d28bda447479ee 100644
--- a/chrome/browser/data_use_measurement/chrome_data_use_ascriber.cc
+++ b/chrome/browser/data_use_measurement/chrome_data_use_ascriber.cc
@@ -51,6 +51,16 @@ ChromeDataUseRecorder* ChromeDataUseAscriber::GetDataUseRecorder(
   return entry == data_use_recorders_.end() ? nullptr : &(*entry);
 }
 
+const DataUseRecorder* ChromeDataUseAscriber::GetDataUseRecorderIfExists(

[RyanSturm, 2016/12/16 16:26:10]

Since you are adding this for const-ness, can you remove the "bool
can_create_new" arg from GetDataUseRecorder and GetDataUseRecorderEntry. Passing
false to that method does the same thing as this method without the const
request or the const on the method. DataReductionProxyData has a similar pair of
methods (except they are static instead of const methods).

That will also require removing the const from the return type of this method.

[Raj, 2016/12/16 19:59:10]

Changed as you suggested.
But I still think being const-function and returning-const would be better.

[RyanSturm, 2016/12/19 19:31:49]

Sorry I was a little unclear. Const-method is good.

I think returning const is a bad idea, because users will most likely want to
call non-const on the DataUseRecorder.

+    const net::URLRequest* request) const {
+  // If a DataUseRecorder has already been set as user data, then return that.
+  auto user_data = static_cast<DataUseRecorderEntryAsUserData*>(
+      request->GetUserData(DataUseRecorderEntryAsUserData::kUserDataKey));
+  return (user_data && user_data->recorder_entry() != data_use_recorders_.end())

[RyanSturm, 2016/12/16 16:26:10]

You shouldn't need to check user_data->recorder_entry() !=
data_use_recorders_.end(), as an assumption of the user data class is that a
valid entry is being passed to the constructor. 

[Raj, 2016/12/16 19:59:10]

Done.

+             ? &(*user_data->recorder_entry())
+             : nullptr;
+}
+
 ChromeDataUseAscriber::DataUseRecorderEntry
 ChromeDataUseAscriber::GetDataUseRecorderEntry(net::URLRequest* request,
                                                bool can_create_new) {
@@ -398,4 +408,15 @@ void ChromeDataUseAscriber::RenderFrameHostChanged(int old_render_process_id,
   }
 }
 
+void ChromeDataUseAscriber::RenderFrameHostChanged(int old_render_process_id,

[RyanSturm, 2016/12/16 16:26:10]

You copied this method from above somehow I think.

[Raj, 2016/12/16 19:59:10]

Oops. Should be due to rebase. Removed.

+                                                   int old_render_frame_id,
+                                                   int new_render_process_id,
+                                                   int new_render_frame_id) {
+  if (visible_main_render_frames_.find(
+          RenderFrameHostID(old_render_process_id, old_render_frame_id)) !=
+      visible_main_render_frames_.end()) {
+    WasShownOrHidden(new_render_process_id, new_render_frame_id, true);
+  }
+}
+
 }  // namespace data_use_measurement