Add more info to ResourceLoadInfo

Adds additional information to the WebContentsObserver
ResourceLoadComplete notification that is required by the page load
metrics code:
- host IP and port for the request
- the total bytes count
- the raw bytes count

Also passing along the RenderFrameHost as part of the callback.

Change-Id: I9646221498312800488660d6b1b76abed467031b
Reviewed-on: https://chromium-review.googlesource.com/1050927Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Commit-Queue: Jay Civelli <jcivelli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#558016}

chrome/browser/renderer_context_menu/render_view_context_menu_browsertest.cc

@@ -1160,8 +1160,8 @@ class LoadImageRequestObserver : public content::WebContentsObserver {
       : content::WebContentsObserver(web_contents), path_(path) {}
 
   void ResourceLoadComplete(
-      const content::mojom::ResourceLoadInfo& resource_load_info,
-      bool is_main_frame) override {
+      content::RenderFrameHost* render_frame_host,
+      const content::mojom::ResourceLoadInfo& resource_load_info) override {
     if (resource_load_info.url.path() == path_)
       run_loop_.Quit();
   }

chrome/browser/safe_browsing/client_side_detection_host.cc

@@ -446,12 +446,13 @@ void ClientSideDetectionHost::DidFinishNavigation(
 }
 
 void ClientSideDetectionHost::ResourceLoadComplete(
-    const content::mojom::ResourceLoadInfo& resource_load_info,
-    bool is_main_frame) {
+    content::RenderFrameHost* render_frame_host,
+    const content::mojom::ResourceLoadInfo& resource_load_info) {
   if (!content::IsResourceTypeFrame(resource_load_info.resource_type) &&
       browse_info_.get() && should_extract_malware_features_ &&
-      resource_load_info.url.is_valid() && resource_load_info.ip.has_value()) {
-    UpdateIPUrlMap(resource_load_info.ip->ToString(),
+      resource_load_info.url.is_valid() &&
+      resource_load_info.network_info->ip_port_pair.has_value()) {
+    UpdateIPUrlMap(resource_load_info.network_info->ip_port_pair->host(),
                    resource_load_info.url.spec(), resource_load_info.method,
                    resource_load_info.referrer.spec(),
                    resource_load_info.resource_type);

chrome/browser/safe_browsing/client_side_detection_host.h

@@ -46,8 +46,8 @@ class ClientSideDetectionHost : public content::WebContentsObserver,
   void DidFinishNavigation(
       content::NavigationHandle* navigation_handle) override;
   void ResourceLoadComplete(
-      const content::mojom::ResourceLoadInfo& resource_load_info,
-      bool is_main_frame) override;
+      content::RenderFrameHost* render_frame_host,
+      const content::mojom::ResourceLoadInfo& resource_load_info) override;
 
   // Called when the SafeBrowsingService found a hit with one of the
   // SafeBrowsing lists.  This method is called on the UI thread.

chrome/browser/safe_browsing/client_side_detection_host_unittest.cc

@@ -1242,7 +1242,8 @@ TEST_F(ClientSideDetectionHostTest,
   resource_load_info->referrer = GURL("http://host2.com");
   resource_load_info->method = "GET";
   resource_load_info->resource_type = content::RESOURCE_TYPE_SUB_FRAME;
-  csd_host_->ResourceLoadComplete(*resource_load_info, /*is_main_frame=*/false);
+  csd_host_->ResourceLoadComplete(/*render_frame_host=*/nullptr,
+                                  *resource_load_info);
 
   EXPECT_EQ(0u, GetBrowseInfo()->ips.size());
 }

content/browser/web_contents/web_contents_impl.cc

@@ -4154,9 +4154,8 @@ void WebContentsImpl::SubresourceResponseStarted(const GURL& url,
 void WebContentsImpl::ResourceLoadComplete(
     RenderFrameHost* render_frame_host,
     mojom::ResourceLoadInfoPtr resource_load_info) {
-  bool is_main_frame = !render_frame_host->GetParent();
   for (auto& observer : observers_) {
-    observer.ResourceLoadComplete(*resource_load_info, is_main_frame);
+    observer.ResourceLoadComplete(render_frame_host, *resource_load_info);
   }
 }
 

content/browser/web_contents/web_contents_impl_browsertest.cc

@@ -9,9 +9,11 @@
 #include "base/macros.h"
 #include "base/memory/ref_counted.h"
 #include "base/optional.h"
+#include "base/path_service.h"
 #include "base/run_loop.h"
 #include "base/strings/pattern.h"
 #include "base/strings/utf_string_conversions.h"
+#include "base/threading/thread_restrictions.h"
 #include "base/time/time.h"
 #include "base/values.h"
 #include "build/build_config.h"
@@ -663,46 +665,61 @@ class ResourceLoadObserver : public WebContentsObserver {
 
   // Use this method with the SCOPED_TRACE macro, so it shows the caller context
   // if it fails.
-  void CheckResourceLoaded(const GURL& url,
-                           const GURL& referrer,
-                           const std::string& load_method,
-                           content::ResourceType resource_type,
-                           const std::string& mime_type,
-                           const std::string& ip_address,
-                           bool was_cached,
-                           bool first_network_request,
-                           const base::TimeTicks& before_request,
-                           const base::TimeTicks& after_request) {
+  void CheckResourceLoaded(
+      const GURL& url,
+      const GURL& referrer,
+      const std::string& load_method,
+      content::ResourceType resource_type,
+      const base::FilePath::StringPieceType& served_file_name,
+      const std::string& mime_type,
+      const std::string& ip_address,
+      bool was_cached,
+      bool first_network_request,
+      const base::TimeTicks& before_request,
+      const base::TimeTicks& after_request) {
     bool resource_load_info_found = false;
     for (const auto& resource_load_info : resource_load_infos_) {
-      if (resource_load_info->url == url) {
-        resource_load_info_found = true;
-        EXPECT_EQ(referrer, resource_load_info->referrer);
-        EXPECT_EQ(load_method, resource_load_info->method);
-        EXPECT_EQ(resource_type, resource_load_info->resource_type);
-        if (!first_network_request)
-          EXPECT_GT(resource_load_info->request_id, 0);
-        EXPECT_EQ(mime_type, resource_load_info->mime_type);
-        ASSERT_TRUE(resource_load_info->ip);
-        EXPECT_EQ(ip_address, resource_load_info->ip->ToString());
-        EXPECT_EQ(was_cached, resource_load_info->was_cached);
-        // Simple sanity check of the load timing info.
-        auto CheckTime = [before_request, after_request](auto actual) {
-          EXPECT_LE(before_request, actual);
-          EXPECT_GT(after_request, actual);
-        };
-        const net::LoadTimingInfo& timing =
-            resource_load_info->load_timing_info;
-        CheckTime(timing.request_start);
-        CheckTime(timing.receive_headers_end);
-        CheckTime(timing.send_start);
-        CheckTime(timing.send_end);
-        if (!was_cached) {
-          CheckTime(timing.connect_timing.dns_start);
-          CheckTime(timing.connect_timing.dns_end);
-          CheckTime(timing.connect_timing.connect_start);
-          CheckTime(timing.connect_timing.connect_end);
-        }
+      if (resource_load_info->url != url)
+        continue;
+
+      resource_load_info_found = true;
+      int64_t file_size = -1;
+      if (!served_file_name.empty()) {
+        base::ScopedAllowBlockingForTesting allow_blocking;
+        base::FilePath test_dir;
+        ASSERT_TRUE(base::PathService::Get(content::DIR_TEST_DATA, &test_dir));
+        base::FilePath served_file = test_dir.Append(served_file_name);
+        ASSERT_TRUE(GetFileSize(served_file, &file_size));
+      }
+      EXPECT_EQ(referrer, resource_load_info->referrer);
+      EXPECT_EQ(load_method, resource_load_info->method);
+      EXPECT_EQ(resource_type, resource_load_info->resource_type);
+      if (!first_network_request)
+        EXPECT_GT(resource_load_info->request_id, 0);
+      EXPECT_EQ(mime_type, resource_load_info->mime_type);
+      ASSERT_TRUE(resource_load_info->network_info->ip_port_pair);
+      EXPECT_EQ(ip_address,
+                resource_load_info->network_info->ip_port_pair->host());
+      EXPECT_EQ(was_cached, resource_load_info->was_cached);
+      // Simple sanity check of the load timing info.
+      auto CheckTime = [before_request, after_request](auto actual) {
+        EXPECT_LE(before_request, actual);
+        EXPECT_GT(after_request, actual);
+      };
+      const net::LoadTimingInfo& timing = resource_load_info->load_timing_info;
+      CheckTime(timing.request_start);
+      CheckTime(timing.receive_headers_end);
+      CheckTime(timing.send_start);
+      CheckTime(timing.send_end);
+      if (!was_cached) {
+        CheckTime(timing.connect_timing.dns_start);
+        CheckTime(timing.connect_timing.dns_end);
+        CheckTime(timing.connect_timing.connect_start);
+        CheckTime(timing.connect_timing.connect_end);
+      }
+      if (file_size != -1) {
+        EXPECT_EQ(file_size, resource_load_info->raw_body_bytes);
+        EXPECT_LT(file_size, resource_load_info->total_received_bytes);
       }
     }
     EXPECT_TRUE(resource_load_info_found);
@@ -716,10 +733,13 @@ class ResourceLoadObserver : public WebContentsObserver {
 
  private:
   // WebContentsObserver implementation:
-  void ResourceLoadComplete(const mojom::ResourceLoadInfo& resource_load_info,
-                            bool is_main_frame) override {
+  void ResourceLoadComplete(
+      content::RenderFrameHost* render_frame_host,
+      const mojom::ResourceLoadInfo& resource_load_info) override {
+    EXPECT_NE(nullptr, render_frame_host);
     resource_load_infos_.push_back(resource_load_info.Clone());
-    resource_is_associated_with_main_frame_.push_back(is_main_frame);
+    resource_is_associated_with_main_frame_.push_back(
+        render_frame_host->GetParent() == nullptr);
   }
 
   void DidLoadResourceFromMemoryCache(const GURL& url,
@@ -746,17 +766,17 @@ IN_PROC_BROWSER_TEST_F(WebContentsImplBrowserTest, ResourceLoadComplete) {
   ASSERT_EQ(3U, observer.resource_load_infos().size());
   SCOPE_TRACED(observer.CheckResourceLoaded(
       page_url, /*referrer=*/GURL(), "GET", content::RESOURCE_TYPE_MAIN_FRAME,
-      "text/html", "127.0.0.1",
+      FILE_PATH_LITERAL("page_with_iframe.html"), "text/html", "127.0.0.1",
       /*was_cached=*/false, /*first_network_request=*/true, before, after));
   SCOPE_TRACED(observer.CheckResourceLoaded(
       embedded_test_server()->GetURL("/image.jpg"),
-      /*referrer=*/page_url, "GET", content::RESOURCE_TYPE_IMAGE, "image/jpeg",
-      "127.0.0.1",
+      /*referrer=*/page_url, "GET", content::RESOURCE_TYPE_IMAGE,
+      FILE_PATH_LITERAL("image.jpg"), "image/jpeg", "127.0.0.1",
       /*was_cached=*/false, /*first_network_request=*/false, before, after));
   SCOPE_TRACED(observer.CheckResourceLoaded(
       embedded_test_server()->GetURL("/title1.html"),
       /*referrer=*/page_url, "GET", content::RESOURCE_TYPE_SUB_FRAME,
-      "text/html", "127.0.0.1",
+      FILE_PATH_LITERAL("title1.html"), "text/html", "127.0.0.1",
       /*was_cached=*/false, /*first_network_request=*/false, before, after));
 }
 
@@ -776,12 +796,13 @@ IN_PROC_BROWSER_TEST_F(WebContentsImplBrowserTest,
   ASSERT_EQ(2U, observer.resource_load_infos().size());
   SCOPE_TRACED(observer.CheckResourceLoaded(
       page_url, /*referrer=*/GURL(), "GET", content::RESOURCE_TYPE_MAIN_FRAME,
-      "text/html", "127.0.0.1", /*was_cached=*/false,
+      /*served_file_name=*/FILE_PATH_LITERAL(""), "text/html", "127.0.0.1",
+      /*was_cached=*/false,
       /*first_network_request=*/true, before, after));
 
   SCOPE_TRACED(observer.CheckResourceLoaded(
       resource_url, /*referrer=*/page_url, "GET", content::RESOURCE_TYPE_SCRIPT,
-      "text/html", "127.0.0.1",
+      /*served_file_name=*/FILE_PATH_LITERAL(""), "text/html", "127.0.0.1",
       /*was_cached=*/false, /*first_network_request=*/false, before, after));
   EXPECT_TRUE(
       observer.resource_load_infos()[1]->network_info->network_accessed);
@@ -795,7 +816,7 @@ IN_PROC_BROWSER_TEST_F(WebContentsImplBrowserTest,
   ASSERT_EQ(1U, observer.resource_load_infos().size());
   SCOPE_TRACED(observer.CheckResourceLoaded(
       page_url, /*referrer=*/GURL(), "GET", content::RESOURCE_TYPE_MAIN_FRAME,
-      "text/html", "127.0.0.1",
+      /*served_file_name=*/FILE_PATH_LITERAL(""), "text/html", "127.0.0.1",
       /*was_cached=*/false, /*first_network_request=*/false, before, after));
   ASSERT_EQ(1U, observer.memory_cached_loaded_urls().size());
   EXPECT_EQ(resource_url, observer.memory_cached_loaded_urls()[0]);
@@ -812,11 +833,11 @@ IN_PROC_BROWSER_TEST_F(WebContentsImplBrowserTest,
   ASSERT_EQ(2U, observer.resource_load_infos().size());
   SCOPE_TRACED(observer.CheckResourceLoaded(
       page_url, /*referrer=*/GURL(), "GET", content::RESOURCE_TYPE_MAIN_FRAME,
-      "text/html", "127.0.0.1",
+      /*served_file_name=*/FILE_PATH_LITERAL(""), "text/html", "127.0.0.1",
       /*was_cached=*/false, /*first_network_request=*/true, before, after));
   SCOPE_TRACED(observer.CheckResourceLoaded(
       resource_url, /*referrer=*/page_url, "GET", content::RESOURCE_TYPE_SCRIPT,
-      "text/html", "127.0.0.1",
+      /*served_file_name=*/FILE_PATH_LITERAL(""), "text/html", "127.0.0.1",
       /*was_cached=*/true, /*first_network_request=*/false, before, after));
   EXPECT_TRUE(observer.memory_cached_loaded_urls().empty());
   EXPECT_FALSE(

content/public/browser/web_contents_observer.h

@@ -256,11 +256,11 @@ class CONTENT_EXPORT WebContentsObserver : public IPC::Listener {
       const std::string& mime_type,
       ResourceType resource_type) {}
 
-  // This method is invoked when a resource has been loaded, successfully or
-  // not.
+  // This method is invoked when a resource associate with the frame
+  // |render_frame_host| has been loaded, successfully or not.
   virtual void ResourceLoadComplete(
-      const mojom::ResourceLoadInfo& resource_load_info,
-      bool is_main_frame) {}
+      RenderFrameHost* render_frame_host,
+      const mojom::ResourceLoadInfo& resource_load_info) {}
 
   // This method is invoked when a new non-pending navigation entry is created.
   // This corresponds to one NavigationController entry being created

content/public/common/resource_load_info.mojom

@@ -7,6 +7,7 @@ module content.mojom;
 import "content/public/common/load_timing_info.mojom";
 import "content/public/common/resource_type.mojom";
 import "net/interfaces/ip_address.mojom";
+import "services/network/public/mojom/network_param.mojom";
 import "url/mojom/url.mojom";
 
 // Network related information reported for loads and redirects.
@@ -17,6 +18,9 @@ struct CommonNetworkInfo {
   // True if loading this resource always requires accessing the network, either
   // because it should always be revalidated or because it should not be cached.
   bool always_access_network;
+
+  // The host IP and port of the response.
+  network.mojom.HostPortPair? ip_port_pair;
 };
 
 // Information for a redirect.
@@ -53,9 +57,6 @@ struct ResourceLoadInfo {
   // The mime type.
   string mime_type;
 
-  // The host IP.
-  net.interfaces.IPAddress? ip;
-
   // True if the response was fetched from the network cache.
   bool was_cached;
 
@@ -68,6 +69,16 @@ struct ResourceLoadInfo {
   // The timing info.
   content.mojom.LoadTimingInfo load_timing_info;
 
+  // The size of the response body before removing any content encodings.
+  // Does not include redirects or sub-requests issued at lower levels (range
+  // requests or auth). Does include bytes from the cache.
+  int64 raw_body_bytes;
+
+  // The total amount of data received from network after SSL decoding and proxy
+  // handling. Pertains only to the last request, i.e. reset on redirects, but
+  // not reset when multiple roundtrips are used for range requests or auth.
+  int64 total_received_bytes;
+
   // The list of redirects that led to this resource load. Empty if there were
   // no redirects.
   array<RedirectInfo> redirect_info_chain;

content/renderer/loader/resource_dispatcher.cc

@@ -192,7 +192,6 @@ void ResourceDispatcher::OnReceivedResponse(
     response_head = initial_response_head;
   }
 
-  request_info->response_start = base::TimeTicks::Now();
   request_info->mime_type = response_head.mime_type;
   request_info->network_accessed = response_head.network_accessed;
   request_info->always_access_network =
@@ -204,14 +203,7 @@ void ResourceDispatcher::OnReceivedResponse(
     DCHECK(new_peer);
     request_info->peer = std::move(new_peer);
   }
-
-  if (!response_head.socket_address.host().empty()) {
-    ignore_result(request_info->parsed_ip.AssignFromIPLiteral(
-        response_head.socket_address.host()));
-  }
-
-  request_info->mime_type = response_head.mime_type;
-  request_info->network_accessed = response_head.network_accessed;
+  request_info->host_port_pair = renderer_response_info.socket_address;
   if (!IsResourceTypeFrame(request_info->resource_type)) {
     NotifySubresourceStarted(RenderThreadImpl::DeprecatedGetMainTaskRunner(),
                              request_info->render_frame_id,
@@ -287,6 +279,8 @@ void ResourceDispatcher::OnReceivedRedirect(
         response_head.network_accessed;
     net_redirect_info->network_info->always_access_network =
         AlwaysAccessNetwork(response_head.headers);
+    net_redirect_info->network_info->ip_port_pair =
+        response_head.socket_address;
     request_info->redirect_info_chain.push_back(std::move(net_redirect_info));
 
     if (!request_info->is_deferred)
@@ -326,19 +320,20 @@ void ResourceDispatcher::OnRequestComplete(
   resource_load_info->method = request_info->response_method;
   resource_load_info->resource_type = request_info->resource_type;
   resource_load_info->request_id = request_id;
-  if (request_info->parsed_ip.IsValid())
-    resource_load_info->ip = request_info->parsed_ip;
   resource_load_info->mime_type = request_info->mime_type;
   resource_load_info->network_info = mojom::CommonNetworkInfo::New();
   resource_load_info->network_info->network_accessed =
       request_info->network_accessed;
   resource_load_info->network_info->always_access_network =
       request_info->always_access_network;
+  resource_load_info->network_info->ip_port_pair = request_info->host_port_pair;
   resource_load_info->load_timing_info = request_info->load_timing_info;
   resource_load_info->was_cached = status.exists_in_cache;
   resource_load_info->net_error = status.error_code;
   resource_load_info->redirect_info_chain =
       std::move(request_info->redirect_info_chain);
+  resource_load_info->total_received_bytes = status.encoded_data_length;
+  resource_load_info->raw_body_bytes = status.encoded_body_length;
 
   NotifyResourceLoadComplete(RenderThreadImpl::DeprecatedGetMainTaskRunner(),
                              request_info->render_frame_id,

content/renderer/loader/resource_dispatcher.h

@@ -27,6 +27,7 @@
 #include "content/public/common/resource_type.h"
 #include "content/public/common/url_loader_throttle.h"
 #include "mojo/public/cpp/system/data_pipe.h"
+#include "net/base/host_port_pair.h"
 #include "net/base/request_priority.h"
 #include "net/traffic_annotation/network_traffic_annotation.h"
 #include "services/network/public/cpp/shared_url_loader_factory.h"
@@ -208,7 +209,7 @@ class CONTENT_EXPORT ResourceDispatcher {
     net::LoadTimingInfo load_timing_info;
     linked_ptr<base::SharedMemory> buffer;
     int buffer_size;
-    net::IPAddress parsed_ip;
+    net::HostPortPair host_port_pair;
     bool network_accessed = false;
     std::string mime_type;
     std::unique_ptr<NavigationResponseOverrideParameters>