Implement a Mojo service for Background Fetch

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 "modules/background_fetch/BackgroundFetchOptions.h"
+#include "modules/background_fetch/BackgroundFetchRegistration.h"
+#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) {

[dcheng, 2017/03/16 07:14:08]

Is this something that would make sense to typemap in the future? I'm not sure
how feasible it is to typemap Mojo generated structs to IDL generated types...
but it does seem like something we should figure out at some point (there's a
lot of boilerplate code like this for interacting with mojo already)

[Peter Beverloo, 2017/03/16 15:32:55]

There's two questions here -

  (1) Mojo structs to IDL dictionaries (like this)
  (2) Mojo structs to IDL interfaces (like BackgroundFetchRegistration)

It would definitely be nice to enable (1). The current issue is that the Mojo
type lives in //public/platform/ whereas the Blink type that we'd want it to use
lives in //Source/modules/, which is not an allowed dependency. We could work
around that by having the TypeConverter live in this module and doing the
conversion manually, but in this case I'm not (yet) convinced it's worth the
overhead.

I can't really comment on (2) from a garbage collection perspective, but many
interfaces do need additional information in order to work correctly. (E.g. the
ServiceWorkerRegistration in this case.) There's probably not as much to gain
there.

+    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),

[dcheng, 2017/03/16 07:14:08]

Nit: #include <utility> for std::move

[Peter Beverloo, 2017/03/16 15:32:56]

Done.

+      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::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) {
+  getService()->GetRegistration(
+      supplementable()->webRegistration()->registrationId(), tag,
+      convertToBaseCallback(
+          WTF::bind(&BackgroundFetchBridge::didGetRegistration,
+                    wrapPersistent(this), WTF::passed(std::move(callback)))));
+}
+
+void BackgroundFetchBridge::didGetRegistration(
+    std::unique_ptr<GetRegistrationCallback> callback,
+    mojom::blink::BackgroundFetchError error,
+    mojom::blink::BackgroundFetchRegistrationPtr registrationPtr) {
+  BackgroundFetchRegistration* registration = nullptr;
+
+  if (registrationPtr) {
+    DCHECK_EQ(error, mojom::blink::BackgroundFetchError::NONE);
+    registration = CreateBackgroundFetchRegistration(
+        supplementable(), std::move(registrationPtr));
+  }
+
+  // Invoke the |callback| with the |error| and the constructed Background
+  // Fetch registration, which can't be a typemap as it's garbage collected.

[dcheng, 2017/03/16 07:14:08]

Can you help me understand why Oilpan heap objects can't be typemapped?

[Peter Beverloo, 2017/03/16 15:32:55]

This actually is a left-over comment -- the first iteration of the Mojo service
I played with used an optional type through WTF::Optional that asserts the
contained type isn't garbage collected. In retrospect that makes no sense for
where I ended up. Sorry for leaving this in -- amended.

Left some thoughts about type mapping above.

+  (*callback)(error, registration);
+}
+
+void BackgroundFetchBridge::getTags(std::unique_ptr<GetTagsCallback> callback) {
+  getService()->GetTags(supplementable()->webRegistration()->registrationId(),
+                        convertToBaseCallback(std::move(callback)));
+}
+
+const mojom::blink::BackgroundFetchServicePtr&

[dcheng, 2017/03/16 07:14:08]

It seems unusual to mark this const: it kind of makes sense from a logical
perspective, since operator-> is marked const... but it feels weird, as the
methods that are invoked feel very much non-const.

[Peter Beverloo, 2017/03/16 15:32:55]

Done.

+BackgroundFetchBridge::getService() {
+  if (!m_backgroundFetchService.get()) {

[dcheng, 2017/03/16 07:14:08]

Nit: no .get()

[Peter Beverloo, 2017/03/16 15:32:55]

Done.

+    Platform::current()->interfaceProvider()->getInterface(
+        mojo::MakeRequest(&m_backgroundFetchService));
+  }
+  return m_backgroundFetchService;
+}
+
+}  // namespace blink