Record the tab state during data use

chrome/browser/data_use_measurement/chrome_data_use_ascriber.cc

 // Copyright 2016 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/data_use_measurement/chrome_data_use_ascriber.h"
 
 #include <string>
 
 #include "base/memory/ptr_util.h"
 #include "chrome/browser/data_use_measurement/chrome_data_use_recorder.h"
@@ skipping 33 matching lines @@
   DCHECK(data_use_recorders_.empty());
 }
 
 ChromeDataUseRecorder* ChromeDataUseAscriber::GetDataUseRecorder(
     net::URLRequest* request,
     bool can_create_new) {
   DataUseRecorderEntry entry = GetDataUseRecorderEntry(request, can_create_new);
   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) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
 
   // TODO(ryansturm): Handle PlzNavigate (http://crbug/664233).
   if (content::IsBrowserSideNavigationEnabled())
     return data_use_recorders_.end();
 
   // If a DataUseRecorder has already been set as user data, then return that.
@@ skipping 327 matching lines @@
                                                    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);
   }
 }
 
+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