Create a DataUseRecorder instance for each page load in Chrome.

chrome/browser/data_use_measurement/chrome_data_use_ascriber.h

 // 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.
 
 #ifndef CHROME_BROWSER_DATA_USE_MEASUREMENT_CHROME_DATA_USE_ASCRIBER_H_
 #define CHROME_BROWSER_DATA_USE_MEASUREMENT_CHROME_DATA_USE_ASCRIBER_H_
 
 #include <list>
 #include <memory>
+#include <string>
+#include <tuple>
 #include <unordered_map>
 #include <utility>
 
 #include "base/containers/hash_tables.h"
 #include "base/hash.h"
 #include "base/macros.h"
 #include "base/supports_user_data.h"
+#include "chrome/browser/data_use_measurement/chrome_data_use_recorder.h"
 #include "components/data_use_measurement/core/data_use_ascriber.h"
 #include "content/public/browser/global_request_id.h"
 #include "url/gurl.h"
 
 namespace content {
 class RenderFrameHost;
 }
 
 namespace data_use_measurement {
 
@@ skipping 18 matching lines @@
 //
 // This class lives entirely on the IO thread. It maintains a copy of frame and
 // navigation information on the IO thread.
 class ChromeDataUseAscriber : public DataUseAscriber {
  public:
   ChromeDataUseAscriber();
 
   ~ChromeDataUseAscriber() override;
 
   // DataUseAscriber implementation:
-  DataUseRecorder* GetDataUseRecorder(net::URLRequest* request) override;
+  ChromeDataUseRecorder* GetDataUseRecorder(net::URLRequest* request,
+                                            bool can_create_new) override;
 
-  // Called before a request is sent.
   void OnBeforeUrlRequest(net::URLRequest* request) override;

[RyanSturm, 2016/11/29 20:32:59]

Remove vertical whitespace between DataUseAscriber implementation methods

[Not at Google. Contact bengr, 2016/11/30 00:21:15]

Done.

 
-  // Called when a URLRequest is being destroyed.
   void OnUrlRequestDestroyed(net::URLRequest* request) override;
 
   std::unique_ptr<URLRequestClassifier> CreateURLRequestClassifier()
       const override;
 
   // Called when a render frame host is created.
   void RenderFrameCreated(int render_process_id,
                           int render_frame_id,
                           int parent_render_process_id,
                           int parent_render_frame_id);
@@ skipping 13 matching lines @@
   // Called when a main frame navigation is ready to be committed in a
   // renderer.
   void ReadyToCommitMainFrameNavigation(
       GURL gurl,
       content::GlobalRequestID global_request_id,
       int render_process_id,
       int render_frame_id,
       bool is_same_page_navigation,
       void* navigation_handle);
 
-  // Called when a main frame navigation is redirected.
-  void DidRedirectMainFrameNavigation(GURL gurl,
-                                      int render_process_id,
-                                      int render_frame_id,
-                                      void* navigation_handle);
 
  private:
-  // Use as a key in the render frame map. Corresponds to a unique
-  // RenderFrameHost.
-  typedef std::pair<int, int> RenderFrameHostID;
+  friend class ChromeDataUseAscriberTest;
 
   // Entry in the |data_use_recorders_| list which owns all instances of
   // DataUseRecorder.
-  typedef std::list<std::unique_ptr<data_use_measurement::DataUseRecorder>>::
-      iterator DataUseRecorderEntry;
+  typedef std::list<
+      std::unique_ptr<data_use_measurement::ChromeDataUseRecorder>>::iterator
+      DataUseRecorderEntry;

[RyanSturm, 2016/11/29 20:32:59]

nit: IMO, this would be easier to read if you added 
typedef std::list<std::unique_ptr<ChromeDataUseRecorder>> DataUseRecorderList;
and changed it to 
typedef DataUseRecorderList::iterator DataUseRecorderEntry;

and changed data_use_recorders_ to be DataUserRecordereList

[Not at Google. Contact bengr, 2016/11/30 00:21:15]

Done.

 
   struct GlobalRequestIDHash {
    public:
     std::size_t operator()(const content::GlobalRequestID& x) const {
       return base::HashInts(x.child_id, x.request_id);
     }
   };
 
   class DataUseRecorderEntryAsUserData : public base::SupportsUserData::Data {
    public:
     explicit DataUseRecorderEntryAsUserData(DataUseRecorderEntry entry);
 
     ~DataUseRecorderEntryAsUserData() override;
 
     DataUseRecorderEntry recorder_entry() { return entry_; }
 
     static const void* kUserDataKey;
 
    private:
     DataUseRecorderEntry entry_;
   };
 
-  void DeletePendingNavigationEntry(content::GlobalRequestID global_request_id);
+  DataUseRecorderEntry GetDataUseRecorderEntry(net::URLRequest* request,
+                                               bool can_create_new);
+
+  void OnDataUseCompleted(DataUseRecorderEntry entry);
+
+  DataUseRecorderEntry CreateNewDataUseRecorder(net::URLRequest* request);
+
+  bool IsRecorderInPendingNavigationMap(net::URLRequest* request);
+
+  bool IsRecorderInRenderFrameMap(net::URLRequest* request);
 
   // Owner for all instances of DataUseRecorder. An instance is kept in this
   // list if any entity (render frame hosts, URLRequests, pending navigations)
   // that ascribe data use to the instance exists, and deleted when all
   // ascribing entities go away.
-  std::list<std::unique_ptr<DataUseRecorder>> data_use_recorders_;
+  std::list<std::unique_ptr<ChromeDataUseRecorder>> data_use_recorders_;

[RyanSturm, 2016/11/29 20:32:59]

Is there a reason why this is std::list<std::unique_ptr<ChromeDataUseRecorder>>
instead of just std::list<ChromeDataUseRecorder> I opened a few other comments
around dereferencing semantics being hard to read, but I don't see anywhere that
std::moves entries into or out of the list, so it seems like you might not need
the unique_ptr. 

[Not at Google. Contact bengr, 2016/11/30 00:21:15]

Switched to std::list<ChromeDataUseRecorder>.

Using unique_ptr avoids having to define copy constructors for recorders as well
as its fields. It doesn't make sense for a recorder to be copied once it is
created and assigned to a page load so it seemed nice to have this be impossible
statically. But not sure the odd syntax is worth this. Added copy constructor
for recorder and DataUse.

[RyanSturm, 2016/12/01 00:40:46]

I think it improves syntax, and if you use emplace instead of insert, the copy
constructor would never be called AFAIK (but probably still needs a definition).
So at that point it's a style choice between having the copy constructor or
having some of the double pointer dereferencing etc. I think it's a grey area
for style choice, so do whatever looks neater. I just didn't know if there were
plans to std::move the unique_ptrs around at some point.

[Not at Google. Contact bengr, 2016/12/01 01:23:39]

Yes, don't see a reason to std::move the unique ptrs. Could change back if the
need arises.

Using Emplace is a good idea -- it will allow adding back
DISALLOW_COPY_AND_ASSIGN.

 
   // Map from RenderFrameHost to the DataUseRecorderEntry in
   // |data_use_recorders_| that the frame ascribe data use to.
   base::hash_map<RenderFrameHostID, DataUseRecorderEntry>
       render_frame_data_use_map_;
 
   // Map from pending navigations to the DataUseRecorderEntry in
   // |data_use_recorders_| that the navigation ascribes data use to.
   std::unordered_map<content::GlobalRequestID,
                      DataUseRecorderEntry,
                      GlobalRequestIDHash>
       pending_navigation_data_use_map_;
 
   DISALLOW_COPY_AND_ASSIGN(ChromeDataUseAscriber);
 };
 
 }  // namespace data_use_measurement
 
 #endif  // CHROME_BROWSER_DATA_USE_MEASUREMENT_CHROME_DATA_USE_ASCRIBER_H_