Implement BackgroundFetchManager.fetch() and struct traits

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

 // Copyright 2017 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "modules/background_fetch/BackgroundFetchBridge.h"
 
 #include <utility>
 #include "modules/background_fetch/BackgroundFetchOptions.h"
 #include "modules/background_fetch/BackgroundFetchRegistration.h"
+#include "modules/background_fetch/BackgroundFetchTypeConverters.h"

[dcheng, 2017/03/21 07:03:20]

Let's use typemapping on the Blink side. It'll save some temporaries and be a
bit more efficient.

For a relatively simple example,
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/platform/mojo/...
should provide some useful pointers. I'm happy to provide detailed help as well
if there's issues getting this to work.

[Peter Beverloo, 2017/03/21 14:05:19]

These types live in //Source/modules/, whereas the mojom lives in
//public/platform/ and are, on the Blink side, linked in as part of
//Source/platform/. The issue is that platform cannot depend on modules.

One approach would be to define another layer of wrapper types in platform that
we provide a typemap for, which the module then uses to create the actual types.
That seems unfortunate as it's more duplication.

Another approach would be to separate out the background fetch mojom (and
potentially other affected mojom files, e.g. NFC) to a new target that still
lives in //public/platform/, but whose Blink bindings will actually be linked in
with modules. That feels a bit icky as it requires the Blink embedder to know
about this distinction by depending on another target.

I'm not sure what the best way forward here is. What's your take?

 #include "modules/background_fetch/IconDefinition.h"
 #include "public/platform/InterfaceProvider.h"
 #include "public/platform/Platform.h"
 #include "public/platform/modules/serviceworker/WebServiceWorkerRegistration.h"
 
 namespace blink {
 
-namespace {
-
-// Creates a new BackgroundFetchRegistration instance given a Service Worker
-// Registration and a Mojo BackgroundFetchRegistrationPtr instance.
-BackgroundFetchRegistration* CreateBackgroundFetchRegistration(
-    ServiceWorkerRegistration* serviceWorkerRegistration,
-    mojom::blink::BackgroundFetchRegistrationPtr registrationPtr) {
-  HeapVector<IconDefinition> icons;
-
-  for (const auto& iconPtr : registrationPtr->icons) {
-    IconDefinition icon;
-    icon.setSrc(iconPtr->src);
-    icon.setSizes(iconPtr->sizes);
-    icon.setType(iconPtr->type);
-
-    icons.push_back(icon);
-  }
-
-  return new BackgroundFetchRegistration(
-      serviceWorkerRegistration, registrationPtr->tag, std::move(icons),
-      registrationPtr->total_download_size, registrationPtr->title);
-}
-
-}  // namespace
-
 // static
 BackgroundFetchBridge* BackgroundFetchBridge::from(
     ServiceWorkerRegistration* serviceWorkerRegistration) {
   DCHECK(serviceWorkerRegistration);
 
   BackgroundFetchBridge* bridge = static_cast<BackgroundFetchBridge*>(
       Supplement<ServiceWorkerRegistration>::from(serviceWorkerRegistration,
                                                   supplementName()));
 
   if (!bridge) {
     bridge = new BackgroundFetchBridge(*serviceWorkerRegistration);
     Supplement<ServiceWorkerRegistration>::provideTo(*serviceWorkerRegistration,
                                                      supplementName(), bridge);
   }
 
   return bridge;
 }
 
 // static
 const char* BackgroundFetchBridge::supplementName() {
   return "BackgroundFetchBridge";
 }
 
 BackgroundFetchBridge::BackgroundFetchBridge(
     ServiceWorkerRegistration& registration)
     : Supplement<ServiceWorkerRegistration>(registration) {}
 
 BackgroundFetchBridge::~BackgroundFetchBridge() = default;
 
+void BackgroundFetchBridge::fetch(
+    const String& tag,
+    const BackgroundFetchOptions& options,
+    std::unique_ptr<RegistrationCallback> callback) {
+  getService()->Fetch(
+      supplementable()->webRegistration()->registrationId(), tag,
+      mojom::blink::BackgroundFetchOptions::From(options),
+      convertToBaseCallback(
+          WTF::bind(&BackgroundFetchBridge::didGetRegistration,
+                    wrapPersistent(this), WTF::passed(std::move(callback)))));
+}
+
 void BackgroundFetchBridge::abort(const String& tag) {
   getService()->Abort(supplementable()->webRegistration()->registrationId(),
                       tag);
 }
 
 void BackgroundFetchBridge::updateUI(
     const String& tag,
     const String& title,
     std::unique_ptr<UpdateUICallback> callback) {
   getService()->UpdateUI(supplementable()->webRegistration()->registrationId(),
                          tag, title,
                          convertToBaseCallback(std::move(callback)));
 }
 
 void BackgroundFetchBridge::getRegistration(
     const String& tag,
-    std::unique_ptr<GetRegistrationCallback> callback) {
+    std::unique_ptr<RegistrationCallback> callback) {
   getService()->GetRegistration(
       supplementable()->webRegistration()->registrationId(), tag,
       convertToBaseCallback(
           WTF::bind(&BackgroundFetchBridge::didGetRegistration,
                     wrapPersistent(this), WTF::passed(std::move(callback)))));
 }
 
 void BackgroundFetchBridge::didGetRegistration(

[harkness, 2017/03/21 11:30:00]

optional: I'm glad that we're reusing the code for both fetch and
getRegistration, but consider renaming didGetRegistration to something more
generic so it doesn't look like a 1:1 mapping to getRegistration.

[Peter Beverloo, 2017/03/21 13:46:27]

Can you suggest a name that you think would be clearer? While I agree that the
overlap with the getRegistration() method is unfortunate, in both cases we did
get a registration.

-    std::unique_ptr<GetRegistrationCallback> callback,
+    std::unique_ptr<RegistrationCallback> callback,
     mojom::blink::BackgroundFetchError error,
     mojom::blink::BackgroundFetchRegistrationPtr registrationPtr) {
-  BackgroundFetchRegistration* registration = nullptr;
+  BackgroundFetchRegistration* registration =
+      registrationPtr.To<BackgroundFetchRegistration*>();
 
-  if (registrationPtr) {
+  if (registration) {
     DCHECK_EQ(error, mojom::blink::BackgroundFetchError::NONE);
-    registration = CreateBackgroundFetchRegistration(
-        supplementable(), std::move(registrationPtr));
+    registration->setServiceWorkerRegistration(supplementable());
   }
 
   (*callback)(error, registration);
 }
 
 void BackgroundFetchBridge::getTags(std::unique_ptr<GetTagsCallback> callback) {
   getService()->GetTags(supplementable()->webRegistration()->registrationId(),
                         convertToBaseCallback(std::move(callback)));
 }
 
 mojom::blink::BackgroundFetchServicePtr& BackgroundFetchBridge::getService() {
   if (!m_backgroundFetchService) {
     Platform::current()->interfaceProvider()->getInterface(
         mojo::MakeRequest(&m_backgroundFetchService));
   }
   return m_backgroundFetchService;
 }
 
 }  // namespace blink