Commit 8b1cca92 authored by Mugdha Lakhani's avatar Mugdha Lakhani Committed by Commit Bot

[Background Fetch] Remove records_ from BackgroundFetchRegistration.

This had been introduced to simplify access to active fetches, but has
the following issues:
1. It makes match() and matchAll() inconsistent with Cache API's match()
and matchAll(), which return new independently-consumable response(s) every
time they're called.
2. It breaks retries for responses, when downloading from a server that
doesn't support resuming of downloads.

For details, see
https://docs.google.com/document/d/1CrbWrnnshhyp_SfiAeuODpnQX36GK3Bsi19rXQGez6Q/edit?usp=sharing

The CL also updates the WPT test accordingly.

Bug: 875201
Change-Id: I2c33717cbee36f0d707e814506c0bae9b5a4f31f
Reviewed-on: https://chromium-review.googlesource.com/c/1355121
Commit-Queue: Mugdha Lakhani <nator@chromium.org>
Reviewed-by: default avatarPeter Beverloo <peter@chromium.org>
Cr-Commit-Position: refs/heads/master@{#613079}
parent 8fc78866
...@@ -88,8 +88,6 @@ void BackgroundFetchRegistration::OnProgress( ...@@ -88,8 +88,6 @@ void BackgroundFetchRegistration::OnProgress(
result_ = result; result_ = result;
failure_reason_ = failure_reason; failure_reason_ = failure_reason;
// TODO(crbug.com/875201): Update records in |records_|.
ExecutionContext* context = GetExecutionContext(); ExecutionContext* context = GetExecutionContext();
if (!context || context->IsContextDestroyed()) if (!context || context->IsContextDestroyed())
return; return;
...@@ -242,21 +240,11 @@ void BackgroundFetchRegistration::DidGetMatchingRequests( ...@@ -242,21 +240,11 @@ void BackgroundFetchRegistration::DidGetMatchingRequests(
ScriptState::Scope scope(script_state); ScriptState::Scope scope(script_state);
HeapVector<Member<BackgroundFetchRecord>> to_return; HeapVector<Member<BackgroundFetchRecord>> to_return;
to_return.ReserveInitialCapacity(settled_fetches.size()); to_return.ReserveInitialCapacity(settled_fetches.size());
for (auto& fetch : settled_fetches) { for (auto& fetch : settled_fetches) {
// If there isn't a record for this fetch in records_ already, create one.
auto iter = records_.find(fetch->request->url);
BackgroundFetchRecord* record = nullptr;
if (iter == records_.end()) {
Request* request = Request::Create(script_state, *(fetch->request)); Request* request = Request::Create(script_state, *(fetch->request));
auto* new_record = auto* record =
MakeGarbageCollected<BackgroundFetchRecord>(request, script_state); MakeGarbageCollected<BackgroundFetchRecord>(request, script_state);
DCHECK(new_record);
records_.Set(request->url(), new_record);
record = new_record;
} else {
record = iter->value;
}
UpdateRecord(record, fetch->response); UpdateRecord(record, fetch->response);
to_return.push_back(record); to_return.push_back(record);
...@@ -268,11 +256,13 @@ void BackgroundFetchRegistration::DidGetMatchingRequests( ...@@ -268,11 +256,13 @@ void BackgroundFetchRegistration::DidGetMatchingRequests(
resolver->Resolve(); resolver->Resolve();
return; return;
} }
DCHECK_EQ(settled_fetches.size(), 1u); DCHECK_EQ(settled_fetches.size(), 1u);
DCHECK_EQ(to_return.size(), 1u); DCHECK_EQ(to_return.size(), 1u);
resolver->Resolve(to_return[0]); resolver->Resolve(to_return[0]);
return; return;
} }
resolver->Resolve(to_return); resolver->Resolve(to_return);
} }
...@@ -373,7 +363,6 @@ void BackgroundFetchRegistration::Dispose() { ...@@ -373,7 +363,6 @@ void BackgroundFetchRegistration::Dispose() {
void BackgroundFetchRegistration::Trace(Visitor* visitor) { void BackgroundFetchRegistration::Trace(Visitor* visitor) {
visitor->Trace(registration_); visitor->Trace(registration_);
visitor->Trace(records_);
EventTargetWithInlineData::Trace(visitor); EventTargetWithInlineData::Trace(visitor);
} }
......
...@@ -12,7 +12,6 @@ ...@@ -12,7 +12,6 @@
#include "third_party/blink/renderer/platform/bindings/script_wrappable.h" #include "third_party/blink/renderer/platform/bindings/script_wrappable.h"
#include "third_party/blink/renderer/platform/heap/garbage_collected.h" #include "third_party/blink/renderer/platform/heap/garbage_collected.h"
#include "third_party/blink/renderer/platform/heap/handle.h" #include "third_party/blink/renderer/platform/heap/handle.h"
#include "third_party/blink/renderer/platform/weborigin/kurl_hash.h"
#include "third_party/blink/renderer/platform/wtf/text/wtf_string.h" #include "third_party/blink/renderer/platform/wtf/text/wtf_string.h"
namespace blink { namespace blink {
...@@ -122,9 +121,6 @@ class BackgroundFetchRegistration final ...@@ -122,9 +121,6 @@ class BackgroundFetchRegistration final
Member<ServiceWorkerRegistration> registration_; Member<ServiceWorkerRegistration> registration_;
// TODO(crbug.com/774054): Update the key once we support duplicate requests.
HeapHashMap<KURL, Member<BackgroundFetchRecord>> records_;
// Corresponds to IDL 'id' attribute. Not unique - an active registration can // Corresponds to IDL 'id' attribute. Not unique - an active registration can
// have the same |developer_id_| as one or more inactive registrations. // have the same |developer_id_| as one or more inactive registrations.
String developer_id_; String developer_id_;
......
...@@ -321,6 +321,6 @@ backgroundFetchTest(async (test, backgroundFetch) => { ...@@ -321,6 +321,6 @@ backgroundFetchTest(async (test, backgroundFetch) => {
assert_true(results[1].url.includes('resources/feature-name.txt')); assert_true(results[1].url.includes('resources/feature-name.txt'));
assert_equals(results[1].status, 200); assert_equals(results[1].status, 200);
assert_equals(results[1].text, 'error'); assert_equals(results[1].text, 'Background Fetch');
}, 'Matching multiple times on the same request works as expected.'); }, 'Matching multiple times on the same request works as expected.');
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment