Replace HeapHashSet with SelfKeepAlive in AttributionSrcLoader
This simplifies ownership, since we intend the resource client to only
live as long as the attributionsrc fetch requires.
Change-Id: I12630355cc3d88bb46978698f9ac785e34b6690f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3539126
Reviewed-by: Nate Chapin <japhet@chromium.org>
Reviewed-by: Nan Lin <linnan@chromium.org>
Commit-Queue: Andrew Paseltiner <apaseltiner@chromium.org>
Cr-Commit-Position: refs/heads/main@{#987904}
diff --git a/third_party/blink/renderer/core/frame/attribution_src_loader.cc b/third_party/blink/renderer/core/frame/attribution_src_loader.cc
index abb000a..24e7f94 100644
--- a/third_party/blink/renderer/core/frame/attribution_src_loader.cc
+++ b/third_party/blink/renderer/core/frame/attribution_src_loader.cc
@@ -27,6 +27,7 @@
#include "third_party/blink/renderer/core/html/html_image_element.h"
#include "third_party/blink/renderer/core/inspector/identifiers_factory.h"
#include "third_party/blink/renderer/platform/heap/persistent.h"
+#include "third_party/blink/renderer/platform/heap/self_keep_alive.h"
#include "third_party/blink/renderer/platform/loader/cors/cors.h"
#include "third_party/blink/renderer/platform/loader/fetch/fetch_context.h"
#include "third_party/blink/renderer/platform/loader/fetch/fetch_initiator_type_names.h"
@@ -149,6 +150,8 @@
// Remote used for registering responses with the browser-process.
mojo::Remote<mojom::blink::AttributionDataHost> data_host_;
+
+ SelfKeepAlive<ResourceClient> keep_alive_{this};
};
AttributionSrcLoader::AttributionSrcLoader(LocalFrame* frame)
@@ -160,7 +163,6 @@
void AttributionSrcLoader::Trace(Visitor* visitor) const {
visitor->Trace(local_frame_);
- visitor->Trace(resource_clients_);
}
void AttributionSrcLoader::Register(const KURL& src_url,
@@ -208,7 +210,7 @@
return nullptr;
}
- if (resource_clients_.size() >= kMaxConcurrentRequests) {
+ if (num_resource_clients_ >= kMaxConcurrentRequests) {
out_register_result = RegisterResult::kFailedToRegister;
return nullptr;
}
@@ -263,7 +265,7 @@
auto* client = MakeGarbageCollected<ResourceClient>(
this, src_type, associated_with_navigation);
- resource_clients_.insert(client);
+ ++num_resource_clients_;
RawResource::Fetch(params, local_frame_->DomWindow()->Fetcher(), client);
RecordAttributionSrcRequestStatus(AttributionSrcRequestStatus::kRequested);
@@ -404,14 +406,16 @@
void AttributionSrcLoader::ResourceClient::NotifyFinished(Resource* resource) {
ClearResource();
- DCHECK(loader_->resource_clients_.Contains(this));
- loader_->resource_clients_.erase(this);
+ DCHECK_GT(loader_->num_resource_clients_, 0u);
+ --loader_->num_resource_clients_;
if (resource->ErrorOccurred()) {
RecordAttributionSrcRequestStatus(AttributionSrcRequestStatus::kFailed);
} else {
RecordAttributionSrcRequestStatus(AttributionSrcRequestStatus::kReceived);
}
+
+ keep_alive_.Clear();
}
void AttributionSrcLoader::ResourceClient::HandleResponseHeaders(
diff --git a/third_party/blink/renderer/core/frame/attribution_src_loader.h b/third_party/blink/renderer/core/frame/attribution_src_loader.h
index f5a7ee4..9ebfa448 100644
--- a/third_party/blink/renderer/core/frame/attribution_src_loader.h
+++ b/third_party/blink/renderer/core/frame/attribution_src_loader.h
@@ -11,7 +11,6 @@
#include "third_party/blink/public/mojom/conversions/attribution_data_host.mojom-blink-forward.h"
#include "third_party/blink/renderer/core/core_export.h"
#include "third_party/blink/renderer/core/inspector/inspector_audits_issue.h"
-#include "third_party/blink/renderer/platform/heap/collection_support/heap_hash_set.h"
#include "third_party/blink/renderer/platform/heap/forward.h"
#include "third_party/blink/renderer/platform/heap/garbage_collected.h"
#include "third_party/blink/renderer/platform/heap/member.h"
@@ -108,7 +107,7 @@
const absl::optional<String>& request_id);
const Member<LocalFrame> local_frame_;
- HeapHashSet<Member<ResourceClient>> resource_clients_;
+ size_t num_resource_clients_ = 0;
};
} // namespace blink