[Resource Timing] Align transferSize to spec

Aligning transferSize to the spec means that response header sizes will no longer be directly exposed, even for same origin resources or resources with Timing-Allow-Origin headers. This is because it seems unsafe[1] to expose header sizes directly. It also means that redirect chains won't count towards the size exposed in `transferSize`. While we could specify and accumulate redirect body sizes, it seems like we'd be better off reporting redirects directly[2] in the future, in case it becomes a priority. PSA: https://groups.google.com/a/chromium.org/g/blink-dev/c/Sny4FO5_y5E [1] w3c/resource-timing#238 [2] https://w3c.github.io/web-performance/meetings/2021/2021-03-18/index.html Bug: 1185801 Change-Id: Iba071c6bb74c5522e3f1ed2586f082f11a7a68a0 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2878693 Commit-Queue: Yoav Weiss <yoavweiss@chromium.org> Reviewed-by: Nicolás Peña Moreno <npm@chromium.org> Reviewed-by: Yutaka Hirano <yhirano@chromium.org> Reviewed-by: Charlie Harrison <csharrison@chromium.org> Reviewed-by: Daniel Cheng <dcheng@chromium.org> Cr-Commit-Position: refs/heads/master@{#883803} NOKEYCHECK=True GitOrigin-RevId: 8d16683fe77e78dc06e87c9cdc9fd7e77f7759ea
mjfroman · May 18, 2021 · c6cef0d · c6cef0d
1 parent e41b920
commit c6cef0d
Show file tree

Hide file tree

Showing 36 changed files with 298 additions and 423 deletions.
diff --git a/blink/public/mojom/timing/resource_timing.mojom b/blink/public/mojom/timing/resource_timing.mojom
@@ -31,6 +31,13 @@ struct ServerTimingInfo {
   string description;
 };
 
+// https://fetch.spec.whatwg.org/#concept-response-cache-state
+enum CacheState {
+  kNone,
+  kLocal,
+  kValidated,
+};
+
 // This struct holds the information from PerformanceResourceTiming that needs
 // to be passed between processes. This is currently used to send timing
 // information about cross-process iframes for window.performance.
@@ -72,9 +79,8 @@ struct ResourceTimingInfo {
   RequestContextType context_type;
   network.mojom.RequestDestination request_destination;
 
-  // Corresponds to |transferSize| in PerformanceResourceTiming
-  // (http://www.w3.org/TR/resource-timing-2/).
-  uint64 transfer_size;
+  // https://w3c.github.io/resource-timing/#dfn-cache-mode
+  CacheState cache_state;
 
   // Corresponds to |encodedBodySize| in PerformanceResourceTiming
   // (http://www.w3.org/TR/resource-timing-2/).

diff --git a/blink/public/platform/web_url_response.h b/blink/public/platform/web_url_response.h
@@ -319,6 +319,9 @@ class WebURLResponse {
   BLINK_PLATFORM_EXPORT void SetConnectionInfo(
       net::HttpResponseInfo::ConnectionInfo);
 
+  // Whether the response was cached and validated over the network.
+  BLINK_PLATFORM_EXPORT void SetIsValidated(bool);
+
   // Original size of the response before decompression.
   BLINK_PLATFORM_EXPORT void SetEncodedDataLength(int64_t);
 

diff --git a/blink/renderer/core/frame/web_frame_test.cc b/blink/renderer/core/frame/web_frame_test.cc
@@ -13412,18 +13412,6 @@ TEST_F(WebFrameTest, GetCanonicalUrlForSharingMultiple) {
             frame->GetDocument().CanonicalUrlForSharing());
 }
 
-TEST_F(WebFrameTest, NavigationTimingInfo) {
-  RegisterMockedHttpURLLoad("foo.html");
-  frame_test_helpers::WebViewHelper web_view_helper;
-  web_view_helper.InitializeAndLoad(base_url_ + "foo.html");
-  ResourceTimingInfo* navigation_timing_info = web_view_helper.LocalMainFrame()
-                                                   ->GetFrame()
-                                                   ->Loader()
-                                                   .GetDocumentLoader()
-                                                   ->GetNavigationTimingInfo();
-  EXPECT_EQ(navigation_timing_info->TransferSize(), static_cast<uint64_t>(34));
-}
-
 TEST_F(WebFrameSimTest, EnterFullscreenResetScrollAndScaleState) {
   UseAndroidSettings();
   WebView().MainFrameViewWidget()->Resize(gfx::Size(490, 500));

diff --git a/blink/renderer/core/loader/cross_thread_resource_timing_info_copier.cc b/blink/renderer/core/loader/cross_thread_resource_timing_info_copier.cc
@@ -32,7 +32,7 @@ CrossThreadCopier<blink::mojom::blink::ResourceTimingInfoPtr>::Copy(
       info->connection_info.IsolatedCopy(),
       info->timing ? info->timing->Clone() : nullptr,
       info->last_redirect_end_time, info->response_end, info->context_type,
-      info->request_destination, info->transfer_size, info->encoded_body_size,
+      info->request_destination, info->cache_state, info->encoded_body_size,
       info->decoded_body_size, info->did_reuse_connection,
       info->is_secure_transport, info->allow_timing_details,
       info->allow_redirect_details, info->allow_negative_values,

diff --git a/blink/renderer/core/loader/document_loader.cc b/blink/renderer/core/loader/document_loader.cc
@@ -936,8 +936,6 @@ void DocumentLoader::BodyLoadingFinished(
       // sizes.
       // TODO(yoav): copy the sizes info directly.
       navigation_timing_info_->SetFinalResponse(response_);
-      navigation_timing_info_->AddFinalTransferSize(
-          total_encoded_data_length == -1 ? 0 : total_encoded_data_length);
       if (report_timing_info_to_parent_) {
         navigation_timing_info_->SetLoadResponseEnd(completion_time);
         if (state_ >= kCommitted) {

diff --git a/blink/renderer/core/timing/performance.cc b/blink/renderer/core/timing/performance.cc
@@ -590,7 +590,7 @@ mojom::blink::ResourceTimingInfoPtr Performance::GenerateResourceTiming(
     result->last_redirect_end_time = base::TimeTicks();
   }
 
-  result->transfer_size = info.TransferSize();
+  result->cache_state = info.CacheState();
   result->encoded_body_size = final_response.EncodedBodyLength();
   result->decoded_body_size = final_response.DecodedBodyLength();
   result->did_reuse_connection = final_response.ConnectionReused();

diff --git a/blink/renderer/core/timing/performance_navigation_timing.cc b/blink/renderer/core/timing/performance_navigation_timing.cc
@@ -112,7 +112,8 @@ bool PerformanceNavigationTiming::DidReuseConnection() const {
 }
 
 uint64_t PerformanceNavigationTiming::GetTransferSize() const {
-  return resource_timing_info_->TransferSize();
+  return PerformanceResourceTiming::GetTransferSize(
+      resource_timing_info_->FinalResponse().EncodedBodyLength(), CacheState());
 }
 
 uint64_t PerformanceNavigationTiming::GetEncodedBodySize() const {

diff --git a/blink/renderer/core/timing/performance_resource_timing.cc b/blink/renderer/core/timing/performance_resource_timing.cc
@@ -80,7 +80,7 @@ PerformanceResourceTiming::PerformanceResourceTiming(
       response_end_(info.response_end),
       context_type_(info.context_type),
       request_destination_(info.request_destination),
-      transfer_size_(info.transfer_size),
+      cache_state_(info.cache_state),
       encoded_body_size_(info.encoded_body_size),
       decoded_body_size_(info.decoded_body_size),
       did_reuse_connection_(info.did_reuse_connection),
@@ -143,8 +143,23 @@ bool PerformanceResourceTiming::DidReuseConnection() const {
   return did_reuse_connection_;
 }
 
+uint64_t PerformanceResourceTiming::GetTransferSize(
+    uint64_t encoded_body_size,
+    mojom::blink::CacheState cache_state) {
+  switch (cache_state) {
+    case mojom::blink::CacheState::kLocal:
+      return 0;
+    case mojom::blink::CacheState::kValidated:
+      return kHeaderSize;
+    case mojom::blink::CacheState::kNone:
+      return encoded_body_size + kHeaderSize;
+  }
+  NOTREACHED();
+  return 0;
+}
+
 uint64_t PerformanceResourceTiming::GetTransferSize() const {
-  return transfer_size_;
+  return GetTransferSize(encoded_body_size_, cache_state_);
 }
 
 uint64_t PerformanceResourceTiming::GetEncodedBodySize() const {

diff --git a/blink/renderer/core/timing/performance_resource_timing.h b/blink/renderer/core/timing/performance_resource_timing.h
@@ -113,8 +113,14 @@ class CORE_EXPORT PerformanceResourceTiming
   bool CrossOriginIsolatedCapability() const {
     return cross_origin_isolated_capability_;
   }
+  mojom::blink::CacheState CacheState() const { return cache_state_; }
+  static uint64_t GetTransferSize(uint64_t encoded_body_size,
+                                  mojom::blink::CacheState cache_state);
 
  private:
+  // https://w3c.github.io/resource-timing/#dom-performanceresourcetiming-transfersize
+  static const size_t kHeaderSize = 300;
+
   AtomicString GetNextHopProtocol(const AtomicString& alpn_negotiated_protocol,
                                   const AtomicString& connection_info) const;
 
@@ -139,7 +145,7 @@ class CORE_EXPORT PerformanceResourceTiming
       mojom::blink::RequestContextType::UNSPECIFIED;
   network::mojom::RequestDestination request_destination_ =
       network::mojom::RequestDestination::kEmpty;
-  uint64_t transfer_size_ = 0;
+  mojom::blink::CacheState cache_state_ = mojom::blink::CacheState::kNone;
   uint64_t encoded_body_size_ = 0;
   uint64_t decoded_body_size_ = 0;
   bool did_reuse_connection_ = false;

diff --git a/blink/renderer/modules/service_worker/fetch_event.cc b/blink/renderer/modules/service_worker/fetch_event.cc
@@ -225,8 +225,6 @@ void FetchEvent::OnNavigationPreloadComplete(
   info->SetLoadResponseEnd(completion_time);
   info->SetInitialURL(request_->url());
   info->SetFinalResponse(resource_response);
-  info->AddFinalTransferSize(encoded_data_length == -1 ? 0
-                                                       : encoded_data_length);
   WorkerGlobalScopePerformance::performance(*worker_global_scope)
       ->GenerateAndAddResourceTiming(*info);
 }

diff --git a/blink/renderer/platform/exported/web_url_response.cc b/blink/renderer/platform/exported/web_url_response.cc
@@ -434,6 +434,10 @@ void WebURLResponse::SetAddressSpace(
   resource_response_->SetAddressSpace(remote_ip_address_space);
 }
 
+void WebURLResponse::SetIsValidated(bool is_validated) {
+  resource_response_->SetIsValidated(is_validated);
+}
+
 void WebURLResponse::SetEncodedDataLength(int64_t length) {
   resource_response_->SetEncodedDataLength(length);
 }

diff --git a/blink/renderer/platform/loader/fetch/resource_fetcher.cc b/blink/renderer/platform/loader/fetch/resource_fetcher.cc
@@ -275,20 +275,13 @@ void SetReferrer(
 
 void PopulateAndAddResourceTimingInfo(Resource* resource,
                                       scoped_refptr<ResourceTimingInfo> info,
-                                      base::TimeTicks response_end,
-                                      int64_t encoded_data_length) {
+                                      base::TimeTicks response_end) {
   info->SetInitialURL(
       resource->GetResourceRequest().GetRedirectInfo().has_value()
           ? resource->GetResourceRequest().GetRedirectInfo()->original_url
           : resource->GetResourceRequest().Url());
   info->SetFinalResponse(resource->GetResponse());
   info->SetLoadResponseEnd(response_end);
-  // encodedDataLength == -1 means "not available".
-  // TODO(ricea): Find cases where it is not available but the
-  // PerformanceResourceTiming spec requires it to be available and fix
-  // them.
-  info->AddFinalTransferSize(encoded_data_length == -1 ? 0
-                                                       : encoded_data_length);
 }
 
 }  // namespace
@@ -1863,8 +1856,7 @@ void ResourceFetcher::HandleLoaderFinish(Resource* resource,
   if (scoped_refptr<ResourceTimingInfo> info =
           resource_timing_info_map_.Take(resource)) {
     if (resource->GetResponse().IsHTTP()) {
-      PopulateAndAddResourceTimingInfo(resource, info, response_end,
-                                       encoded_data_length);
+      PopulateAndAddResourceTimingInfo(resource, info, response_end);
       auto receiver = Context().TakePendingWorkerTimingReceiver(
           resource->GetResponse().RequestId());
       info->SetWorkerTimingReceiver(std::move(receiver));
@@ -1911,9 +1903,7 @@ void ResourceFetcher::HandleLoaderError(Resource* resource,
 
   if (scoped_refptr<ResourceTimingInfo> info =
           resource_timing_info_map_.Take(resource)) {
-    PopulateAndAddResourceTimingInfo(
-        resource, info, finish_time,
-        resource->GetResponse().EncodedDataLength());
+    PopulateAndAddResourceTimingInfo(resource, info, finish_time);
     if (resource->Options().request_initiator_context == kDocumentContext)
       Context().AddResourceTiming(*info);
   }

diff --git a/blink/renderer/platform/loader/fetch/resource_fetcher_test.cc b/blink/renderer/platform/loader/fetch/resource_fetcher_test.cc
@@ -86,7 +86,6 @@ namespace {
 
 constexpr char kTestResourceFilename[] = "white-1x1.png";
 constexpr char kTestResourceMimeType[] = "image/png";
-constexpr uint32_t kTestResourceSize = 103;  // size of white-1x1.png
 
 const FetchClientSettingsObjectSnapshot& CreateFetchClientSettingsObject(
     network::mojom::IPAddressSpace address_space) {
@@ -572,20 +571,6 @@ TEST_F(ResourceFetcherTest, Vary) {
   new_resource->Loader()->Cancel();
 }
 
-TEST_F(ResourceFetcherTest, ResourceTimingInfo) {
-  auto info = ResourceTimingInfo::Create(
-      fetch_initiator_type_names::kDocument, base::TimeTicks::Now(),
-      mojom::blink::RequestContextType::UNSPECIFIED,
-      network::mojom::RequestDestination::kEmpty);
-  info->AddFinalTransferSize(5);
-  EXPECT_EQ(info->TransferSize(), static_cast<uint64_t>(5));
-  ResourceResponse redirect_response(KURL("https://example.com/original"));
-  redirect_response.SetHttpStatusCode(200);
-  redirect_response.SetEncodedDataLength(7);
-  info->AddRedirect(redirect_response, KURL("https://example.com/redirect"));
-  EXPECT_EQ(info->TransferSize(), static_cast<uint64_t>(12));
-}
-
 TEST_F(ResourceFetcherTest, VaryOnBack) {
   scoped_refptr<const SecurityOrigin> source_origin =
       SecurityOrigin::CreateUniqueOpaque();
@@ -854,50 +839,6 @@ class ScopedMockRedirectRequester {
   DISALLOW_COPY_AND_ASSIGN(ScopedMockRedirectRequester);
 };
 
-TEST_F(ResourceFetcherTest, SameOriginRedirect) {
-  const char kRedirectURL[] = "http://127.0.0.1:8000/redirect.html";
-  const char kFinalURL[] = "http://127.0.0.1:8000/final.html";
-  MockFetchContext* context = MakeGarbageCollected<MockFetchContext>();
-  ScopedMockRedirectRequester requester(platform_->GetURLLoaderMockFactory(),
-                                        context, CreateTaskRunner());
-  requester.RegisterRedirect(kRedirectURL, kFinalURL);
-  requester.RegisterFinalResource(kFinalURL);
-  requester.Request(kRedirectURL);
-
-  EXPECT_EQ(kRedirectResponseOverheadBytes + kTestResourceSize,
-            context->GetTransferSize());
-}
-
-TEST_F(ResourceFetcherTest, CrossOriginRedirect) {
-  const char kRedirectURL[] = "http://otherorigin.test/redirect.html";
-  const char kFinalURL[] = "http://127.0.0.1:8000/final.html";
-  MockFetchContext* context = MakeGarbageCollected<MockFetchContext>();
-  ScopedMockRedirectRequester requester(platform_->GetURLLoaderMockFactory(),
-                                        context, CreateTaskRunner());
-  requester.RegisterRedirect(kRedirectURL, kFinalURL);
-  requester.RegisterFinalResource(kFinalURL);
-  requester.Request(kRedirectURL);
-
-  EXPECT_EQ(kTestResourceSize, context->GetTransferSize());
-}
-
-TEST_F(ResourceFetcherTest, ComplexCrossOriginRedirect) {
-  const char kRedirectURL1[] = "http://127.0.0.1:8000/redirect1.html";
-  const char kRedirectURL2[] = "http://otherorigin.test/redirect2.html";
-  const char kRedirectURL3[] = "http://127.0.0.1:8000/redirect3.html";
-  const char kFinalURL[] = "http://127.0.0.1:8000/final.html";
-  MockFetchContext* context = MakeGarbageCollected<MockFetchContext>();
-  ScopedMockRedirectRequester requester(platform_->GetURLLoaderMockFactory(),
-                                        context, CreateTaskRunner());
-  requester.RegisterRedirect(kRedirectURL1, kRedirectURL2);
-  requester.RegisterRedirect(kRedirectURL2, kRedirectURL3);
-  requester.RegisterRedirect(kRedirectURL3, kFinalURL);
-  requester.RegisterFinalResource(kFinalURL);
-  requester.Request(kRedirectURL1);
-
-  EXPECT_EQ(kTestResourceSize, context->GetTransferSize());
-}
-
 TEST_F(ResourceFetcherTest, SynchronousRequest) {
   KURL url("http://127.0.0.1:8000/foo.png");
   RegisterMockedURLLoad(url);

diff --git a/blink/renderer/platform/loader/fetch/resource_response.cc b/blink/renderer/platform/loader/fetch/resource_response.cc
@@ -474,6 +474,17 @@ AtomicString ResourceResponse::ConnectionInfoString() const {
       connection_info_string.length());
 }
 
+mojom::blink::CacheState ResourceResponse::CacheState() const {
+  return is_validated_
+             ? mojom::blink::CacheState::kValidated
+             : (!encoded_data_length_ ? mojom::blink::CacheState::kLocal
+                                      : mojom::blink::CacheState::kNone);
+}
+
+void ResourceResponse::SetIsValidated(bool is_validated) {
+  is_validated_ = is_validated;
+}
+
 void ResourceResponse::SetEncodedDataLength(int64_t value) {
   encoded_data_length_ = value;
 }

diff --git a/blink/renderer/platform/loader/fetch/resource_response.h b/blink/renderer/platform/loader/fetch/resource_response.h
@@ -37,6 +37,7 @@
 #include "services/network/public/mojom/fetch_api.mojom-shared.h"
 #include "services/network/public/mojom/ip_address_space.mojom-shared.h"
 #include "third_party/abseil-cpp/absl/types/optional.h"
+#include "third_party/blink/public/mojom/timing/resource_timing.mojom-blink.h"
 #include "third_party/blink/public/platform/web_url_response.h"
 #include "third_party/blink/renderer/platform/network/http_header_map.h"
 #include "third_party/blink/renderer/platform/network/http_parsers.h"
@@ -430,6 +431,9 @@ class PLATFORM_EXPORT ResourceResponse final {
 
   AtomicString ConnectionInfoString() const;
 
+  mojom::blink::CacheState CacheState() const;
+  void SetIsValidated(bool is_validated);
+
   int64_t EncodedDataLength() const { return encoded_data_length_; }
   void SetEncodedDataLength(int64_t value);
 
@@ -678,6 +682,9 @@ class PLATFORM_EXPORT ResourceResponse final {
   net::HttpResponseInfo::ConnectionInfo connection_info_ =
       net::HttpResponseInfo::ConnectionInfo::CONNECTION_INFO_UNKNOWN;
 
+  // Whether the resource came from the cache and validated over the network.
+  bool is_validated_ = false;
+
   // Size of the response in bytes prior to decompression.
   int64_t encoded_data_length_ = 0;
 

diff --git a/blink/renderer/platform/loader/fetch/resource_timing_info.cc b/blink/renderer/platform/loader/fetch/resource_timing_info.cc
@@ -12,17 +12,6 @@ namespace blink {
 void ResourceTimingInfo::AddRedirect(const ResourceResponse& redirect_response,
                                      const KURL& new_url) {
   redirect_chain_.push_back(redirect_response);
-  if (has_cross_origin_redirect_)
-    return;
-  bool cross_origin = !SecurityOrigin::AreSameOrigin(
-      redirect_response.CurrentRequestUrl(), new_url);
-  if (cross_origin) {
-    has_cross_origin_redirect_ = true;
-    transfer_size_ = 0;
-  } else {
-    DCHECK_GE(redirect_response.EncodedDataLength(), 0);
-    transfer_size_ += redirect_response.EncodedDataLength();
-  }
 }
 
 }  // namespace blink
diff --git a/blink/renderer/platform/loader/fetch/resource_timing_info.h b/blink/renderer/platform/loader/fetch/resource_timing_info.h
@@ -35,6 +35,7 @@
 
 #include "base/macros.h"
 #include "third_party/blink/public/mojom/fetch/fetch_api_request.mojom-blink-forward.h"
+#include "third_party/blink/public/mojom/timing/resource_timing.mojom-blink.h"
 #include "third_party/blink/public/mojom/timing/worker_timing_container.mojom-blink.h"
 #include "third_party/blink/renderer/platform/loader/fetch/resource_request.h"
 #include "third_party/blink/renderer/platform/loader/fetch/resource_response.h"
@@ -78,10 +79,9 @@ class PLATFORM_EXPORT ResourceTimingInfo
     return redirect_chain_;
   }
 
-  void AddFinalTransferSize(uint64_t encoded_data_length) {
-    transfer_size_ += encoded_data_length;
+  mojom::blink::CacheState CacheState() const {
+    return final_response_.CacheState();
   }
-  uint64_t TransferSize() const { return transfer_size_; }
 
   // The timestamps in PerformanceResourceTiming are measured relative from the
   // time origin. In most cases these timestamps must be positive value, so we
@@ -127,7 +127,6 @@ class PLATFORM_EXPORT ResourceTimingInfo
   KURL initial_url_;
   ResourceResponse final_response_;
   Vector<ResourceResponse> redirect_chain_;
-  uint64_t transfer_size_ = 0;
   bool has_cross_origin_redirect_ = false;
   bool negative_allowed_ = false;
 

diff --git a/blink/renderer/platform/loader/fetch/url_loader/web_url_loader.cc b/blink/renderer/platform/loader/fetch/url_loader/web_url_loader.cc
@@ -842,6 +842,7 @@ void WebURLLoader::PopulateURLResponse(
   response->SetCorsExposedHeaderNames(cors_exposed_header_names);
   response->SetDidServiceWorkerNavigationPreload(
       head.did_service_worker_navigation_preload);
+  response->SetIsValidated(head.is_validated);
   response->SetEncodedDataLength(head.encoded_data_length);
   response->SetEncodedBodyLength(head.encoded_body_length);
   response->SetWasAlpnNegotiated(head.was_alpn_negotiated);