[Backport] Security bug 961614 3/8

Fix UMA for mis-attributed requests made by HTML imports

I relied on ResourceLoadObserver::DidStartRequest but it is not called
when the fetcher is detached. Move the logic into ResourceFetcher to fix
that.

Bug: 961614
Change-Id: Id8ba7e5f445dd87adb556bd2771bceb094e643a4
Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>

3 files changed, 19 insertions, 47 deletions

diff --git a/chromium/third_party/blink/renderer/core/dom/document.cc b/chromium/third_party/blink/renderer/core/dom/document.cc
index 580563f2c18..51242fd9c61 100644
--- a/chromium/third_party/blink/renderer/core/dom/document.cc
+++ b/chromium/third_party/blink/renderer/core/dom/document.cc
@@ -290,7 +290,6 @@
 #include "third_party/blink/renderer/platform/language.h"
 #include "third_party/blink/renderer/platform/loader/fetch/null_resource_fetcher_properties.h"
 #include "third_party/blink/renderer/platform/loader/fetch/resource_fetcher.h"
-#include "third_party/blink/renderer/platform/loader/fetch/resource_load_observer.h"
 #include "third_party/blink/renderer/platform/network/content_security_policy_parsers.h"
 #include "third_party/blink/renderer/platform/network/http_parsers.h"
 #include "third_party/blink/renderer/platform/network/network_state_notifier.h"
@@ -322,45 +321,6 @@ static WeakDocumentSet& liveDocumentSet();
 
 namespace blink {
 
-namespace {
-
-class ResourceLoadObserverForUnintentionalRequestsMadeByImportedDocument final
-    : public ResourceLoadObserver {
-  void DidStartRequest(const FetchParameters&, ResourceType type) override {
-    UMA_HISTOGRAM_ENUMERATION("HTMLImport.UnexpectedRequest", type);
-  }
-  void WillSendRequest(uint64_t identifier,
-                       const ResourceRequest&,
-                       const ResourceResponse& redirect_response,
-                       ResourceType,
-                       const FetchInitiatorInfo&) override {}
-  void DidChangePriority(uint64_t identifier,
-                         ResourceLoadPriority,
-                         int intra_priority_value) override {}
-  void DidReceiveResponse(uint64_t identifier,
-                          const ResourceRequest& request,
-                          const ResourceResponse& response,
-                          const Resource* resource,
-                          ResponseSource) override {}
-  void DidReceiveData(uint64_t identifier,
-                      base::span<const char> chunk) override {}
-  void DidReceiveTransferSizeUpdate(uint64_t identifier,
-                                    int transfer_size_diff) override {}
-  void DidDownloadToBlob(uint64_t identifier, BlobDataHandle*) override {}
-  void DidFinishLoading(uint64_t identifier,
-                        base::TimeTicks finish_time,
-                        int64_t encoded_data_length,
-                        int64_t decoded_body_length,
-                        bool should_report_corb_blocking) override {}
-  void DidFailLoading(const KURL&,
-                      uint64_t identifier,
-                      const ResourceError&,
-                      int64_t encoded_data_length,
-                      bool is_internal_request) override {}
-};
-
-}  // namespace
-
 using namespace html_names;
 
 class DocumentOutliveTimeReporter : public BlinkGCObserver {
@@ -1141,11 +1101,8 @@ Document::Document(const DocumentInit& initializer,
         GetTaskRunner(TaskType::kNetworking), nullptr /* loader_factory */));
 
     if (imports_controller_) {
-      // We don't expect the fetcher to be used, so add a ResourceLoadObserver
-      // which counts such unexpected use.
-      fetcher_->SetResourceLoadObserver(
-          MakeGarbageCollected<
-              ResourceLoadObserverForUnintentionalRequestsMadeByImportedDocument>());
+      // We don't expect the fetcher to be used, so count such unexpected use.
+      fetcher_->SetShouldLogRequestAsInvalidInImportedDocument();
     }
   }
   DCHECK(fetcher_);
diff --git a/chromium/third_party/blink/renderer/platform/loader/fetch/resource_fetcher.cc b/chromium/third_party/blink/renderer/platform/loader/fetch/resource_fetcher.cc
index 77ca7a74c0b..c7a591fbbbd 100644
--- a/chromium/third_party/blink/renderer/platform/loader/fetch/resource_fetcher.cc
+++ b/chromium/third_party/blink/renderer/platform/loader/fetch/resource_fetcher.cc
@@ -578,7 +578,8 @@ ResourceFetcher::ResourceFetcher(const ResourceFetcherInit& init)
       auto_load_images_(true),
       images_enabled_(true),
       allow_stale_resources_(false),
-      image_fetched_(false) {
+      image_fetched_(false),
+      should_log_request_as_invalid_in_imported_document_(false) {
   stale_while_revalidate_enabled_ =
       RuntimeEnabledFeatures::StaleWhileRevalidateEnabledByRuntimeFlag();
   InstanceCounters::IncrementCounter(InstanceCounters::kResourceFetcherCounter);
@@ -948,6 +949,13 @@ Resource* ResourceFetcher::RequestResource(FetchParameters& params,
                                            ResourceClient* client) {
   base::AutoReset<bool> r(&is_in_request_resource_, true);
 
+  if (should_log_request_as_invalid_in_imported_document_) {
+    DCHECK(properties_->IsDetached());
+    // We don't expect the fetcher to be used, so count such unexpected use.
+    UMA_HISTOGRAM_ENUMERATION("HTMLImport.UnexpectedRequest",
+                              factory.GetType());
+  }
+
   // If detached, we do very early return here to skip all processing below.
   if (properties_->IsDetached()) {
     return ResourceForBlockedRequest(
diff --git a/chromium/third_party/blink/renderer/platform/loader/fetch/resource_fetcher.h b/chromium/third_party/blink/renderer/platform/loader/fetch/resource_fetcher.h
index 8def1628b91..e48d1a1867b 100644
--- a/chromium/third_party/blink/renderer/platform/loader/fetch/resource_fetcher.h
+++ b/chromium/third_party/blink/renderer/platform/loader/fetch/resource_fetcher.h
@@ -128,6 +128,7 @@ class PLATFORM_EXPORT ResourceFetcher
   }
   // This must be called right after construction.
   void SetResourceLoadObserver(ResourceLoadObserver* observer) {
+    DCHECK(!IsDetached());
     DCHECK(!resource_load_observer_);
     resource_load_observer_ = observer;
   }
@@ -269,6 +270,10 @@ class PLATFORM_EXPORT ResourceFetcher
                                speculative_preload_type, is_link_preload);
   }
 
+  void SetShouldLogRequestAsInvalidInImportedDocument() {
+    should_log_request_as_invalid_in_imported_document_ = true;
+  }
+
  private:
   friend class ResourceCacheValidationSuppressor;
   enum class StopFetchingTarget {
@@ -417,12 +422,14 @@ class PLATFORM_EXPORT ResourceFetcher
   // This is not in the bit field below because we want to use AutoReset.
   bool is_in_request_resource_ = false;
 
-  // 27 bits left
+  // 26 bits left
   bool auto_load_images_ : 1;
   bool images_enabled_ : 1;
   bool allow_stale_resources_ : 1;
   bool image_fetched_ : 1;
   bool stale_while_revalidate_enabled_ : 1;
+  // for https://crbug.com/961614
+  bool should_log_request_as_invalid_in_imported_document_ : 1;
 
   static constexpr uint32_t kKeepaliveInflightBytesQuota = 64 * 1024;