[Background Fetch] Add WPT tests for BackgroundFetchRegistration::abort

Re-resubmit cr/1261477

The issue was that there is a race condition between the abort going
through, and the downloaded file being persisted on disk. I added an
extra check in the WPT test.

Change-Id: I98aa005b4637c08f60a126a6c7c896c1c0edfddf
Reviewed-on: https://chromium-review.googlesource.com/c/1276557
Commit-Queue: Rayan Kanso <rayankans@chromium.org>
Reviewed-by: Mugdha Lakhani <nator@chromium.org>
Reviewed-by: Peter Beverloo <peter@chromium.org>
Cr-Commit-Position: refs/heads/master@{#598848}

content/browser/background_fetch/background_fetch_context.h

@@ -201,12 +201,12 @@ class CONTENT_EXPORT BackgroundFetchContext
   void DidFinishJob(
       base::OnceCallback<void(blink::mojom::BackgroundFetchError)> callback,
       const BackgroundFetchRegistrationId& registration_id,
-      blink::mojom::BackgroundFetchFailureReason reason_to_abort);
+      blink::mojom::BackgroundFetchFailureReason failure_reason);
 
   // Called when the data manager finishes marking a registration as deleted.
   void DidMarkForDeletion(
       const BackgroundFetchRegistrationId& registration_id,
-      blink::mojom::BackgroundFetchFailureReason reason_to_abort,
+      blink::mojom::BackgroundFetchFailureReason failure_reason,
       base::OnceCallback<void(blink::mojom::BackgroundFetchError)> callback,
       blink::mojom::BackgroundFetchError error);
 
@@ -214,7 +214,6 @@ class CONTENT_EXPORT BackgroundFetchContext
   // retrieved from storage, and the Service Worker event can be invoked.
   void DidGetSettledFetches(
       const BackgroundFetchRegistrationId& registration_id,
-      std::unique_ptr<BackgroundFetchRegistration> registration,
       blink::mojom::BackgroundFetchError error,
       blink::mojom::BackgroundFetchFailureReason failure_reason,
       std::vector<BackgroundFetchSettledFetch> settled_fetches,
@@ -230,6 +229,11 @@ class CONTENT_EXPORT BackgroundFetchContext
       std::vector<BackgroundFetchSettledFetch> settled_fetches,
       std::vector<std::unique_ptr<storage::BlobDataHandle>> blob_data_handles);
 
+  // Dispatches an appropriate event (success, fail, abort).
+  void DispatchCompletionEvent(
+      const BackgroundFetchRegistrationId& registration_id,
+      std::unique_ptr<BackgroundFetchRegistration> registration);
+
   // Called when the notification UI for the background fetch job associated
   // with |unique_id| is activated.
   void DispatchClickEvent(const std::string& unique_id);
@@ -241,25 +245,17 @@ class CONTENT_EXPORT BackgroundFetchContext
           initialization_data);
 
   // Called when all processing for the |registration_id| has been finished and
-  // the job is ready to be deleted. |blob_handles| are unused, but some callers
-  // use it to keep blobs alive for the right duration.
-  // |partial cleanup|, when set, preserves  the registration ID, and the result
-  // of Fetch when it completed, in |completed_fetches_|. This is not done when
-  // fetch is aborted or cancelled. We use this information to propagate
-  // BackgroundFetchClicked event to the developer, when the user taps the UI.
+  // the job is ready to be deleted.
+  // |preserve_info_to_dispatch_click_event|, when set, preserves the
+  // registration ID, and the result of the Fetch when it completed, in
+  // |completed_fetches_|. This is not done when fetch is aborted or cancelled.
+  // We use this information to propagate BackgroundFetchClicked event to the
+  // developer, when the user taps the UI.
   void CleanupRegistration(
       const BackgroundFetchRegistrationId& registration_id,
-      const std::vector<std::unique_ptr<storage::BlobDataHandle>>&
-          blob_data_handles,
       blink::mojom::BackgroundFetchResult background_fetch_result,
       bool preserve_info_to_dispatch_click_event = false);
 
-  // Called when the last JavaScript BackgroundFetchRegistration object has been
-  // garbage collected for a registration marked for deletion, and so it is now
-  // safe to delete the underlying registration data.
-  void LastObserverGarbageCollected(
-      const BackgroundFetchRegistrationId& registration_id);
-
   // Switches out |data_manager_| with a DataManager configured for testing
   // environments. Must be called directly after the constructor.
   void SetDataManagerForTesting(

content/browser/background_fetch/background_fetch_job_controller.cc

@@ -13,6 +13,8 @@
 
 namespace content {
 
+using blink::mojom::BackgroundFetchFailureReason;
+
 BackgroundFetchJobController::BackgroundFetchJobController(
     BackgroundFetchDelegateProxy* delegate_proxy,
     BackgroundFetchScheduler* scheduler,
@@ -198,7 +200,7 @@ BackgroundFetchJobController::NewRegistration(
   return std::make_unique<BackgroundFetchRegistration>(
       registration_id().developer_id(), registration_id().unique_id(),
       0 /* upload_total */, 0 /* uploaded */, total_downloads_size_,
-      complete_requests_downloaded_bytes_cache_, result, reason_to_abort_);
+      complete_requests_downloaded_bytes_cache_, result, failure_reason_);
 }
 
 uint64_t BackgroundFetchJobController::GetInProgressDownloadedBytes() {
@@ -206,8 +208,8 @@ uint64_t BackgroundFetchJobController::GetInProgressDownloadedBytes() {
 }
 
 void BackgroundFetchJobController::Abort(
-    blink::mojom::BackgroundFetchFailureReason reason_to_abort) {
-  reason_to_abort_ = reason_to_abort;
+    BackgroundFetchFailureReason failure_reason) {
+  failure_reason_ = failure_reason;
 
   // Stop propagating any in-flight events to the scheduler.
   active_request_finished_callback_.Reset();
@@ -215,7 +217,14 @@ void BackgroundFetchJobController::Abort(
   // Cancel any in-flight downloads and UI through the BGFetchDelegate.
   delegate_proxy_->Abort(registration_id().unique_id());
 
-  Finish(reason_to_abort);
+  Finish(failure_reason_);
+}
+
+BackgroundFetchFailureReason BackgroundFetchJobController::MergeFailureReason(
+    BackgroundFetchFailureReason failure_reason) {
+  if (failure_reason_ == BackgroundFetchFailureReason::NONE)
+    failure_reason_ = failure_reason;
+  return failure_reason_;
 }
 
 }  // namespace content

content/browser/background_fetch/background_fetch_job_controller.h

@@ -92,6 +92,11 @@ class CONTENT_EXPORT BackgroundFetchJobController final
   // Returns the number of requests that comprise the whole job.
   int total_downloads() const { return total_downloads_; }
 
+  // If |failure_reason_| is none, overwrites it with |failure_reason|, and
+  // returns the new value.
+  blink::mojom::BackgroundFetchFailureReason MergeFailureReason(
+      blink::mojom::BackgroundFetchFailureReason failure_reason);
+
   base::WeakPtr<BackgroundFetchJobController> GetWeakPtr() {
     return weak_ptr_factory_.GetWeakPtr();
   }
@@ -112,7 +117,7 @@ class CONTENT_EXPORT BackgroundFetchJobController final
   std::vector<scoped_refptr<BackgroundFetchRequestInfo>>
   TakeOutstandingRequests() override;
   void Abort(
-      blink::mojom::BackgroundFetchFailureReason reason_to_abort) override;
+      blink::mojom::BackgroundFetchFailureReason failure_reason) override;
 
  private:
   // Performs mixed content checks on the |request| for Background Fetch.
@@ -165,7 +170,7 @@ class CONTENT_EXPORT BackgroundFetchJobController final
   int completed_downloads_ = 0;
 
   // The reason background fetch was aborted.
-  blink::mojom::BackgroundFetchFailureReason reason_to_abort_ =
+  blink::mojom::BackgroundFetchFailureReason failure_reason_ =
       blink::mojom::BackgroundFetchFailureReason::NONE;
 
   base::WeakPtrFactory<BackgroundFetchJobController> weak_ptr_factory_;

third_party/WebKit/LayoutTests/external/wpt/background-fetch/abort.https.window.js

+// META: script=/service-workers/service-worker/resources/test-helpers.sub.js
+// META: script=resources/utils.js
+'use strict';
+
+// Covers basic functionality provided by BackgroundFetchManager.abort().
+// https://wicg.github.io/background-fetch/#background-fetch-registration-abort
+
+backgroundFetchTest(async (test, backgroundFetch) => {
+  const registration = await backgroundFetch.fetch(
+      uniqueId(),
+      ['resources/feature-name.txt', '/serviceworker/resources/slow-response.php']);
+
+  assert_true(await registration.abort());
+  assert_false(await registration.abort());
+
+}, 'Aborting the same registration twice fails');
+
+backgroundFetchTest(async (test, backgroundFetch) => {
+  const registration = await backgroundFetch.fetch(
+      uniqueId(),
+      ['resources/feature-name.txt', '/serviceworker/resources/slow-response.php']);
+
+  await new Promise(resolve => {
+    let aborted = false;
+    const expectedResultText = 'Background Fetch';
+
+    registration.onprogress = async event => {
+      if (event.target.downloaded < expectedResultText.length)
+        return;
+
+      if (aborted)
+        return;
+
+      // Abort after the first file has been downloaded and check the results.
+
+      aborted = true;
+      assert_true(await registration.abort());
+
+      const {type, eventRegistration, results} = await getMessageFromServiceWorker();
+
+      assert_equals(eventRegistration.result, 'failure');
+      assert_equals(eventRegistration.failureReason, 'aborted');
+      assert_equals(registration.result, 'failure');
+      assert_equals(registration.failureReason, 'aborted');
+
+      assert_equals(type, 'backgroundfetchabort');
+
+      // The abort might have gone through before the first result was persisted.
+      if (results.length === 1) {
+        assert_true(results[0].url.includes('resources/feature-name.txt'));
+        assert_equals(results[0].status, 200);
+        assert_equals(results[0].text, expectedResultText);
+      }
+
+      resolve();
+    };
+  });
+
+}, 'Calling BackgroundFetchRegistration.abort sets the correct fields and responses are still available');
\ No newline at end of file

third_party/WebKit/LayoutTests/external/wpt/background-fetch/service_workers/sw.js

@@ -27,3 +27,4 @@ function handleBackgroundFetchUpdateEvent(event) {
 
 self.addEventListener('backgroundfetchsuccess', handleBackgroundFetchUpdateEvent);
 self.addEventListener('backgroundfetchfail', handleBackgroundFetchUpdateEvent);
+self.addEventListener('backgroundfetchabort', handleBackgroundFetchUpdateEvent);