Pass around BackgroundFetch options as mojom type.

CL https://chromium-review.googlesource.com/c/chromium/src/+/956183
caused a few tests to be flaky, which was due to the garbage collector
getting rid of BackgroundFetchOptions from under us.

This change creates a copy of background fetch options early on,
converting them from blink::BackgroundFetchOptions to
mojom::blink::BackgroundFetchRegistrationPtr. This copy is not going to
be deleted by the GC, hence ensuring accurate data later on in the
processing of the fetch.

Bug: 822276
Change-Id: Iaab780b18182372cf607b57eb15815c852d4595a
Reviewed-on: https://chromium-review.googlesource.com/977909
Commit-Queue: Mugdha Lakhani <nator@chromium.org>
Reviewed-by: Peter Beverloo <peter@chromium.org>
Cr-Commit-Position: refs/heads/master@{#545805}

chrome/browser/background_fetch/background_fetch_browsertest.cc

@@ -274,8 +274,7 @@ class BackgroundFetchBrowserTest : public InProcessBrowserTest {
 };
 
 // Flaky. See https://crbug.com/822276
-IN_PROC_BROWSER_TEST_F(BackgroundFetchBrowserTest,
-                       DISABLED_DownloadService_Acceptance) {
+IN_PROC_BROWSER_TEST_F(BackgroundFetchBrowserTest, DownloadService_Acceptance) {
   // Starts a Background Fetch for a single to-be-downloaded file and waits for
   // that request to be scheduled with the Download Service.
 
@@ -295,7 +294,7 @@ IN_PROC_BROWSER_TEST_F(BackgroundFetchBrowserTest,
 
 // TODO(crbug.com/822944): Disabled since flaky.
 IN_PROC_BROWSER_TEST_F(BackgroundFetchBrowserTest,
-                       DISABLED_OfflineItemCollection_SingleFileMetadata) {
+                       OfflineItemCollection_SingleFileMetadata) {
   // Starts a Background Fetch for a single to-be-downloaded file and waits for
   // the fetch to be registered with the offline items collection. We then
   // verify that all the appropriate values have been set.
@@ -326,7 +325,7 @@ IN_PROC_BROWSER_TEST_F(BackgroundFetchBrowserTest,
 
 // Flaky. See https://crbug.com/822276
 IN_PROC_BROWSER_TEST_F(BackgroundFetchBrowserTest,
-                       DISABLED_OfflineItemCollection_VerifyIconReceived) {
+                       OfflineItemCollection_VerifyIconReceived) {
   // Starts a Background Fetch for a single to-be-downloaded file and waits for
   // the fetch to be registered with the offline items collection. We then
   // verify that the expected icon is associated with the newly added offline

third_party/WebKit/Source/modules/background_fetch/BackgroundFetchBridge.cpp

@@ -41,15 +41,15 @@ BackgroundFetchBridge::BackgroundFetchBridge(
 
 BackgroundFetchBridge::~BackgroundFetchBridge() = default;
 
-void BackgroundFetchBridge::Fetch(const String& developer_id,
-                                  Vector<WebServiceWorkerRequest> requests,
-                                  const BackgroundFetchOptions& options,
-                                  const SkBitmap& icon,
-                                  RegistrationCallback callback) {
+void BackgroundFetchBridge::Fetch(
+    const String& developer_id,
+    Vector<WebServiceWorkerRequest> requests,
+    mojom::blink::BackgroundFetchOptionsPtr options,
+    const SkBitmap& icon,
+    RegistrationCallback callback) {
   GetService()->Fetch(
       GetSupplementable()->WebRegistration()->RegistrationId(), developer_id,
-      std::move(requests), mojom::blink::BackgroundFetchOptions::From(options),
-      icon,
+      std::move(requests), std::move(options), icon,
       WTF::Bind(&BackgroundFetchBridge::DidGetRegistration,
                 WrapPersistent(this), WTF::Passed(std::move(callback))));
 }

third_party/WebKit/Source/modules/background_fetch/BackgroundFetchBridge.h

@@ -16,7 +16,6 @@
 
 namespace blink {
 
-class BackgroundFetchOptions;
 class BackgroundFetchRegistration;
 class WebServiceWorkerRequest;
 
@@ -48,11 +47,11 @@ class BackgroundFetchBridge final
   virtual ~BackgroundFetchBridge();
 
   // Creates a new Background Fetch registration identified by |developer_id|
-  // with the given |options| for the sequence of |requests|. The |callback|
-  // will be invoked when the registration has been created.
+  // for the sequence of |requests|. The |callback| will be invoked when the
+  // registration has been created.
   void Fetch(const String& developer_id,
              Vector<WebServiceWorkerRequest> requests,
-             const BackgroundFetchOptions&,
+             mojom::blink::BackgroundFetchOptionsPtr,
              const SkBitmap& icon,
              RegistrationCallback);
 

third_party/WebKit/Source/modules/background_fetch/BackgroundFetchIconLoader.cpp

@@ -33,17 +33,18 @@ BackgroundFetchIconLoader::~BackgroundFetchIconLoader() {
 
 // TODO(nator): Add functionality to select which icon to load.
 void BackgroundFetchIconLoader::Start(ExecutionContext* execution_context,
-                                      const HeapVector<IconDefinition>& icons,
+                                      HeapVector<IconDefinition> icons,
                                       IconCallback icon_callback) {
   DCHECK(!stopped_);
   DCHECK_GE(icons.size(), 1u);
+  icons_ = std::move(icons);
 
-  if (!icons[0].hasSrc()) {
+  if (!icons_[0].hasSrc()) {
     std::move(icon_callback).Run(SkBitmap());
     return;
   }
 
-  KURL first_icon_url = execution_context->CompleteURL(icons[0].src());
+  KURL first_icon_url = execution_context->CompleteURL(icons_[0].src());
   if (!first_icon_url.IsValid() || first_icon_url.IsEmpty()) {
     std::move(icon_callback).Run(SkBitmap());
     return;

third_party/WebKit/Source/modules/background_fetch/BackgroundFetchIconLoader.h

@@ -10,6 +10,7 @@
 #include "core/loader/ThreadableLoader.h"
 #include "core/loader/ThreadableLoaderClient.h"
 #include "modules/ModulesExport.h"
+#include "modules/background_fetch/BackgroundFetchTypeConverters.h"
 #include "platform/SharedBuffer.h"
 #include "third_party/skia/include/core/SkBitmap.h"
 
@@ -34,9 +35,7 @@ class MODULES_EXPORT BackgroundFetchIconLoader final
 
   // Asynchronously download an icon from the given url, decodes the loaded
   // data, and passes the bitmap to the given callback.
-  void Start(ExecutionContext*,
-             const HeapVector<IconDefinition>&,
-             IconCallback);
+  void Start(ExecutionContext*, HeapVector<IconDefinition>, IconCallback);
 
   // Cancels the pending load, if there is one. The |icon_callback_| will not
   // be run.
@@ -49,7 +48,10 @@ class MODULES_EXPORT BackgroundFetchIconLoader final
   void DidFail(const ResourceError&) override;
   void DidFailRedirectCheck() override;
 
-  void Trace(blink::Visitor* visitor) { visitor->Trace(threadable_loader_); }
+  void Trace(blink::Visitor* visitor) {
+    visitor->Trace(threadable_loader_);
+    visitor->Trace(icons_);
+  }
 
  private:
   void RunCallbackWithEmptyBitmap();
@@ -57,6 +59,7 @@ class MODULES_EXPORT BackgroundFetchIconLoader final
   bool stopped_ = false;
   scoped_refptr<SharedBuffer> data_;
   IconCallback icon_callback_;
+  HeapVector<IconDefinition> icons_;
   Member<ThreadableLoader> threadable_loader_;
 };
 

third_party/WebKit/Source/modules/background_fetch/BackgroundFetchManager.cpp

@@ -18,6 +18,7 @@
 #include "modules/background_fetch/BackgroundFetchIconLoader.h"
 #include "modules/background_fetch/BackgroundFetchOptions.h"
 #include "modules/background_fetch/BackgroundFetchRegistration.h"
+#include "modules/background_fetch/BackgroundFetchTypeConverters.h"
 #include "modules/serviceworkers/ServiceWorkerRegistration.h"
 #include "platform/bindings/ScriptState.h"
 #include "platform/bindings/V8ThrowException.h"
@@ -256,27 +257,29 @@ ScriptPromise BackgroundFetchManager::fetch(
 
   // Load Icons. Right now, we just load the first icon. Lack of icons or
   // inability to load them should not be fatal to the fetch.
+  mojom::blink::BackgroundFetchOptionsPtr options_ptr =
+      mojom::blink::BackgroundFetchOptions::From(options);
   if (options.icons().size()) {
     loader_->Start(
         execution_context, options.icons(),
         WTF::Bind(&BackgroundFetchManager::DidLoadIcons, WrapPersistent(this),
-                  id, WTF::Passed(std::move(web_requests)), options,
-                  WrapPersistent(resolver)));
+                  id, WTF::Passed(std::move(web_requests)),
+                  std::move(options_ptr), WrapPersistent(resolver)));
     return promise;
   }
 
-  DidLoadIcons(id, std::move(web_requests), options, WrapPersistent(resolver),
-               SkBitmap());
+  DidLoadIcons(id, std::move(web_requests), std::move(options_ptr),
+               WrapPersistent(resolver), SkBitmap());
   return promise;
 }
 
 void BackgroundFetchManager::DidLoadIcons(
     const String& id,
     Vector<WebServiceWorkerRequest> web_requests,
-    const BackgroundFetchOptions& options,
+    mojom::blink::BackgroundFetchOptionsPtr options,
     ScriptPromiseResolver* resolver,
     const SkBitmap& icon) {
-  bridge_->Fetch(id, std::move(web_requests), options, icon,
+  bridge_->Fetch(id, std::move(web_requests), std::move(options), icon,
                  WTF::Bind(&BackgroundFetchManager::DidFetch,
                            WrapPersistent(this), WrapPersistent(resolver)));
 }

third_party/WebKit/Source/modules/background_fetch/BackgroundFetchManager.h

@@ -73,7 +73,7 @@ class MODULES_EXPORT BackgroundFetchManager final
 
   void DidLoadIcons(const String&,
                     Vector<WebServiceWorkerRequest>,
-                    const BackgroundFetchOptions&,
+                    mojom::blink::BackgroundFetchOptionsPtr,
                     ScriptPromiseResolver*,
                     const SkBitmap&);
   void DidFetch(ScriptPromiseResolver*,