[background-sync] Remove WebSyncError and SyncCallbacks

third_party/WebKit/Source/modules/background_sync/SyncManager.cpp

 // Copyright 2015 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_sync/SyncManager.h"
 
 #include "bindings/core/v8/CallbackPromiseAdapter.h"
 #include "bindings/core/v8/ScriptPromise.h"
 #include "bindings/core/v8/ScriptPromiseResolver.h"
 #include "bindings/core/v8/ScriptState.h"
 #include "core/dom/DOMException.h"
 #include "core/dom/ExceptionCode.h"
 #include "core/dom/ExecutionContext.h"
 #include "modules/background_sync/BackgroundSyncProvider.h"
-#include "modules/background_sync/SyncCallbacks.h"
 #include "modules/serviceworkers/ServiceWorkerRegistration.h"
 #include "public/platform/Platform.h"
 #include "wtf/PtrUtil.h"
 #include "wtf/ThreadSpecific.h"
 
 namespace blink {
 
 // static
 BackgroundSyncProvider* SyncManager::backgroundSyncProvider() {
   DEFINE_THREAD_SAFE_STATIC_LOCAL(ThreadSpecific<BackgroundSyncProvider>,
@@ skipping 22 matching lines @@
   ScriptPromise promise = resolver->promise();
 
   mojom::blink::SyncRegistrationPtr syncRegistration =
       mojom::blink::SyncRegistration::New();
   syncRegistration->id = SyncManager::kUnregisteredSyncID;
   syncRegistration->tag = tag;
   syncRegistration->network_state =
       blink::mojom::BackgroundSyncNetworkState::ONLINE;
 
   backgroundSyncProvider()->registerBackgroundSync(
-      std::move(syncRegistration), m_registration->webRegistration(),
-      makeUnique<SyncRegistrationCallbacks>(resolver, m_registration));
+      std::move(syncRegistration), m_registration->webRegistration(), resolver);

[iclelland, 2016/11/18 20:53:46]

The old SyncRegistrationCallbacks object held a reference to the service worker
registration -- I think that was so that even if this SyncManager was destroyed,
the service worker registration object would be kept alive long enough that the
provider could handle the success/failure of the register() call, and call the
JS callback function.

I haven't traced it completely through, so I may be wrong, but I'm not sure that
we still have that guarantee here.

[iclelland, 2016/11/18 21:22:36]

Tracing through the git history, and the evolution of that file, it looks like
that reference was originally used so that we could pass an active
SyncRegistration object to the (JS) callback on success. That was pulled out in
crrev.com/1667623004, but we never got rid of the constructor arg.

I think that in this case, the fact that you're making the ScriptPromiseResolver
persistent is enough to make sure that the callback will run correctly, and we
don't actually need to keep the service worker registration object alive
explicitly.

 
   return promise;
 }
 
 ScriptPromise SyncManager::getTags(ScriptState* scriptState) {
   ScriptPromiseResolver* resolver = ScriptPromiseResolver::create(scriptState);
   ScriptPromise promise = resolver->promise();
 
-  backgroundSyncProvider()->getRegistrations(
-      m_registration->webRegistration(),
-      makeUnique<SyncGetRegistrationsCallbacks>(resolver, m_registration));
+  backgroundSyncProvider()->getRegistrations(m_registration->webRegistration(),
+                                             resolver);
 
   return promise;
 }
 
 DEFINE_TRACE(SyncManager) {
   visitor->trace(m_registration);
 }
 
 }  // namespace blink