Expose GlobalRequestID in NavigationHandle and ResourceRequestInfo.

chrome/browser/data_use_measurement/chrome_data_use_ascriber.h

Index: chrome/browser/data_use_measurement/chrome_data_use_ascriber.h
diff --git a/chrome/browser/data_use_measurement/chrome_data_use_ascriber.h b/chrome/browser/data_use_measurement/chrome_data_use_ascriber.h
index 4b8c3171c32c75f725374ef4c3ea1278d223934f..ed21a7471c151f4e56322e2bd3e06f22f3d7d5c6 100644
--- a/chrome/browser/data_use_measurement/chrome_data_use_ascriber.h
+++ b/chrome/browser/data_use_measurement/chrome_data_use_ascriber.h
@@ -5,8 +5,16 @@
 #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 <utility>
+
+#include "base/containers/hash_tables.h"
+#include "base/hash.h"
 #include "base/macros.h"
+#include "base/supports_user_data.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 {
@@ -44,6 +52,8 @@ class ChromeDataUseAscriber : public DataUseAscriber {
   // DataUseAscriber:
   DataUseRecorder* GetDataUseRecorder(net::URLRequest* request) override;
 
+  void OnBeforeUrlRequest(net::URLRequest* request) override;
+
   // Called when a render frame host is created.
   void RenderFrameCreated(int render_process_id,
                           int render_frame_id,
@@ -64,6 +74,7 @@ class ChromeDataUseAscriber : public DataUseAscriber {
 
   // Called when a main frame navigation is completed.
   void DidFinishMainFrameNavigation(GURL gurl,
+                                    content::GlobalRequestID global_request_id,
                                     int render_process_id,
                                     int render_frame_id,
                                     bool is_same_page_navigation,
@@ -75,7 +86,44 @@ class ChromeDataUseAscriber : public DataUseAscriber {
                                       int render_frame_id,
                                       void* navigation_handle);
 
+  void OnUrlRequestCompleted(net::URLRequest* request, bool started) override;
+
  private:
+  typedef std::pair<int, int> RenderFrameHostID;
+  typedef std::list<std::unique_ptr<data_use_measurement::DataUseRecorder>>::

[Raj, 2016/10/27 20:33:56]

I wonder whether following typedefs could be easier to understand.

typedef std::list<std::unique_ptr<DataUseRecorder>> DataUseRecorderList;
typedef DataUseRecorderList::iterator DataUseRecorderListIter;

[RyanSturm, 2016/10/27 22:41:21]

+1

maybe add a typedef for std::unique_ptr<data_use_measurement::DataUseRecorder>
as well

[Not at Google. Contact bengr, 2016/10/28 18:43:33]

std::list<std::unique_ptr<DataUseRecorder>> is only used once. So apart from the
statement here, the typedef will not used anywhere else. My preference is to use
typedefs where the type is referred to multiple times. When it is used just
once, it just adds more code. WDYT?

DataUseRecorderList::iterator is already typedefed as DataUseRecorderEntry.

std::unique_ptr<data_use_measurement::DataUseRecorder> type is never  referred
to apart from the same declaration.

+      iterator DataUseRecorderEntry;
+
+  class DataUseRecorderEntryAsUserData : public base::SupportsUserData::Data {

[RyanSturm, 2016/10/27 22:41:21]

I think you can anonymous namespace this class since you don't reference it in
the header.

[Not at Google. Contact bengr, 2016/10/28 18:43:33]

This class uses DataUseRecorderEntry which is a private typedef in
ChromeDataUseAscriber.

+   public:
+    explicit DataUseRecorderEntryAsUserData(DataUseRecorderEntry entry);
+
+    ~DataUseRecorderEntryAsUserData() override;
+
+    DataUseRecorderEntry recorder_entry() { return entry_; }
+
+    static const void* kUserDataKey;
+
+   private:
+    DataUseRecorderEntry entry_;
+  };
+
+  struct GlobalRequestIDHash {

[Raj, 2016/10/27 20:33:56]

nit: Could this be in anonymous namespace in this file.

[Not at Google. Contact bengr, 2016/10/28 18:43:33]

Yes, but doesn't get us anything. Any file including the .h will be able to use
it.

[RyanSturm, 2016/11/02 21:41:58]

This has to be used in this file, and anonymous namespaces don't make that much
sense in header files. Leave it private.

+   public:
+    std::size_t operator()(const content::GlobalRequestID& x) const {
+      return base::HashInts(x.child_id, x.request_id);
+    }
+  };
+
+  std::list<std::unique_ptr<DataUseRecorder>> data_use_recorders_;

[Raj, 2016/10/27 20:33:56]

If using DataUseRecorderList, change std::list<std::unique_ptr<DataUseRecorder>>

[Not at Google. Contact bengr, 2016/10/28 18:43:33]

See above.

+
+  base::hash_map<RenderFrameHostID, DataUseRecorderEntry>
+      render_frame_data_use_map_;
+
+  base::hash_map<content::GlobalRequestID,

[RyanSturm, 2016/10/27 22:41:21]

nit: Can this be?
std::unordered_map<RenderFrameHostID,
std::unique_ptr<data_use_measurement::DataUseRecorder>, GlobalRequestIDHash>

not much difference between the two since GlboalRequestId isn't std::pair, so I
would prefer STL

[Not at Google. Contact bengr, 2016/10/28 18:43:33]

Done.

+                 DataUseRecorderEntry,
+                 GlobalRequestIDHash>
+      pending_navigation_data_use_map_;
+
   DISALLOW_COPY_AND_ASSIGN(ChromeDataUseAscriber);
 };