[BackgroundSync] Initial land of the BackgroundSyncManager

content/browser/background_sync/background_sync_manager.h

Index: content/browser/background_sync/background_sync_manager.h
diff --git a/content/browser/background_sync/background_sync_manager.h b/content/browser/background_sync/background_sync_manager.h
new file mode 100644
index 0000000000000000000000000000000000000000..e8375d5e7676f79d33c330c8a66a0973d7e43d43
--- /dev/null
+++ b/content/browser/background_sync/background_sync_manager.h
@@ -0,0 +1,170 @@
+// 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.
+
+#ifndef CONTENT_BROWSER_BACKGROUND_SYNC_BACKGROUND_SYNC_MANAGER_H_
+#define CONTENT_BROWSER_BACKGROUND_SYNC_BACKGROUND_SYNC_MANAGER_H_
+
+#include <list>
+#include <map>
+
+#include "base/callback.h"

[davidben, 2015/03/13 22:05:44]

I think you only need callback_forward.h in this file.

[jkarlin, 2015/03/14 01:17:27]

Done.

+#include "base/memory/scoped_ptr.h"
+#include "base/memory/weak_ptr.h"
+#include "content/browser/service_worker/service_worker_storage.h"
+#include "content/common/content_export.h"
+#include "content/common/service_worker/service_worker_status_code.h"
+#include "url/gurl.h"
+
+namespace content {
+
+class ServiceWorkerCacheScheduler;
+class ServiceWorkerContextWrapper;
+
+// BackgroundSyncManager manages and stores the set of background sync
+// registrations across all registered service workers for a profile.
+// Registrations are stored along with their associated Service Worker
+// registration in ServiceWorkerStorage. If the ServiceWorker is unregistered,
+// the sync registrations are removed. This class expects to be run on the IO
+// thread. The asynchronous methods are executed sequentially.
+
+// TODO(jkarlin): Check permissions when registering, scheduling, and firing
+// background sync. In the meantime, --enable-service-worker-sync is required to
+// fire a sync event.
+// TODO(jkarlin): Unregister syncs when permission is revoked.
+// TODO(jkarlin): Create a background sync scheduler to actually run the
+// registered events.
+// TODO(jkarlin): Keep the browser alive if "Let Google Chrome Run in the
+// Background" is true and a sync is registered.
+// TODO(jkarlin): Unregister syncs when storage for an origin is cleared.
+class CONTENT_EXPORT BackgroundSyncManager {
+ public:
+  enum ErrorType {
+    ErrorTypeOK = 0,
+    ErrorTypeExists,
+    ErrorTypeStorage,
+    ErrorTypeNotFound
+  };
+
+  // TODO(jkarlin): Remove this and use the struct from IPC messages once it
+  // lands.
+  struct BackgroundSyncRegistration {
+    BackgroundSyncRegistration() {}
+    explicit BackgroundSyncRegistration(const std::string& id) : id(id) {}
+    std::string id;
+  };
+
+  using StatusCallback = base::Callback<void(ErrorType)>;
+  using StatusAndRegistrationCallback =
+      base::Callback<void(ErrorType, const BackgroundSyncRegistration&)>;
+
+  static scoped_ptr<BackgroundSyncManager> Create(
+      const scoped_refptr<ServiceWorkerContextWrapper>& service_worker_context);
+  virtual ~BackgroundSyncManager();
+
+  // Stores the given background sync registration and adds it to the scheduling
+  // queue. Will request permission from the user if necessary. Returns
+  // ErrorTypeExists if the registration is already registered. Returns
+  // ErrorTypeOK and the registration on success.

[davidben, 2015/03/13 22:05:44]

"Returns" -> "Calls |callback| with"

[jkarlin, 2015/03/14 01:17:27]

Done.

+  void Register(const GURL& origin,
+                int64 sw_registration_id,
+                const BackgroundSyncRegistration& sync_registratin,

[davidben, 2015/03/13 22:05:44]

sync_registratin -> sync_registration

I don't really understand what a consumer of this API is supposed to do with
this. On success, they get back a new BackgroundSyncRegistration (which is just
a string id). But it wasn't actually new because they passed one in and the
implementation seems to echo the same one back. Did you intend for the class to
generate them internally?

[jkarlin, 2015/03/14 01:17:27]

In the future there is a chance that the Register() function will alter the
registration with user-provided guidance. E.g., "Sure, I'll allow this
registration but no more than once per day". 

[davidben, 2015/03/17 18:47:14]

Ah, okay. Perhaps add a comment to this effect or leave it as just a
StatusCallback for now? It's somewhat confusing reading the header without that
context.

[jkarlin, 2015/03/20 13:50:49]

Done.

+                const StatusAndRegistrationCallback& callback);
+
+  // Removes the given background sync registration from storage and the
+  // scheduling queue. Returns ErrorTypeNotFound if not registered. Returns

[davidben, 2015/03/13 22:05:44]

Returns -> Calls |callback with

[jkarlin, 2015/03/14 01:17:27]

Done.

+  // ErrorTypeOK on success.
+  void Unregister(const GURL& origin,
+                  int64 sw_registration_id,
+                  const BackgroundSyncRegistration& sync_registration,
+                  const StatusCallback& callback);
+
+  // Finds the background sync registration associated with
+  // |sw_registration_id|. Returns ErrorTypeNotFound if it doesn't exist.
+  // Returns ErrorTypeOK on success.

[davidben, 2015/03/13 22:05:44]

Returns -> Calls |callback| with

[jkarlin, 2015/03/14 01:17:27]

Done.

+  void GetRegistration(const GURL& origin,
+                       int64 sw_registration_id,
+                       const std::string sync_registration_id,
+                       const StatusAndRegistrationCallback& callback);

[davidben, 2015/03/13 22:05:43]

I'm assuming the BackgroundSyncRegistration will get other fields later? Right
now it just returns the same thing back.

[jkarlin, 2015/03/14 01:17:27]

Yep! 

+
+ private:
+  using PermissionStatusCallback = base::Callback<void(bool)>;
+  using IdToRegistrationMap =
+      std::map<int64, std::list<BackgroundSyncRegistration>>;
+
+  explicit BackgroundSyncManager(
+      const scoped_refptr<ServiceWorkerContextWrapper>& context);
+
+  bool HasRegistration(int64 sw_registration_id,
+                       const BackgroundSyncRegistration& sync_registration);
+
+  // Store all registrations for a given |sw_registration_id|.
+  void StoreRegistrations(const GURL& origin,
+                          int64 sw_registration_id,
+                          const ServiceWorkerStorage::StatusCallback& callback);
+
+  // If the registration is in the map, removes it. Otherwise returns false.
+  bool RemoveRegistrationFromMap(
+      int64 sw_registration_id,
+      const BackgroundSyncRegistration& sync_registration);
+
+  // If the registration is not in the map, adds it. Otherwise returns false.
+  bool AddRegistrationToMap(
+      int64 sw_registration_id,
+      const BackgroundSyncRegistration& sync_registration);
+
+  // Init must be called before any public member function.
+  void Init();
+  void InitImpl();
+  void InitDidGetUserData(
+      const std::vector<std::pair<int64, std::string>>& user_data,
+      ServiceWorkerStatusCode status);
+
+  // Register callbacks
+  void RegisterImpl(const GURL& origin,
+                    int64 sw_registration_id,
+                    const BackgroundSyncRegistration& sync_registration,
+                    const StatusAndRegistrationCallback& callback);
+  void RegisterDidStore(int64 sw_registration_id,
+                        const BackgroundSyncRegistration& sync_registration,
+                        const StatusAndRegistrationCallback& callback,
+                        ServiceWorkerStatusCode status);
+
+  // Unregister callbacks
+  void UnregisterImpl(const GURL& origin,
+                      int64 sw_registration_id,
+                      const BackgroundSyncRegistration& sync_registration,
+                      const StatusCallback& callback);
+  void UnregisterDidStore(int64 sw_registration_id,
+                          const BackgroundSyncRegistration& sync_registration,
+                          const StatusCallback& callback,
+                          ServiceWorkerStatusCode status);
+
+  // GetRegistration callbacks
+  void GetRegistrationImpl(const GURL& origin,
+                           int64 sw_registration_id,
+                           const std::string sync_registration_id,
+                           const StatusAndRegistrationCallback& callback);
+
+  // Operation Scheduling callbacks
+  void PendingStatusAndRegistrationCallback(
+      const StatusAndRegistrationCallback& callback,
+      ErrorType error,
+      const BackgroundSyncRegistration& sync_registration);
+  void PendingStatusCallback(const StatusCallback& callback, ErrorType error);
+
+  bool initialized_;
+  bool initializing_;

[davidben, 2015/03/13 22:05:44]

These two bits of state are redundant, right? You can probably just have a
single initialized_ since initializing_ is always the opposite. Or perhaps just
drop both; it's not clear the DCHECKs are actually asserting on anything since
they always assert on the pair.

(Perhaps add some DCHECKs on just initialized_ for the private functions called
by the scheduler? Otherwise I'd remove both.)

[jkarlin, 2015/03/14 01:17:27]

Since init() is now called by ::Create() (it wasn't when I added those booleans)
you're right, they're no longer necessary. Good catch, thanks!

+
+  scoped_ptr<IdToRegistrationMap> registration_map_;

[davidben, 2015/03/13 22:05:44]

Why put this in a scoped_ptr? It doesn't look like you ever release it early or
detach it from the object or anything.

[jkarlin, 2015/03/14 01:17:27]

Done.

+  scoped_ptr<ServiceWorkerCacheScheduler> op_scheduler_;

[davidben, 2015/03/13 22:05:44]

(Does this need to be a scoped_ptr? I guess other users do it, but it seems this
object may as well be created inline too.)

[jkarlin, 2015/03/14 01:17:27]

Done.

+  scoped_refptr<ServiceWorkerContextWrapper> service_worker_context_;
+
+  base::WeakPtrFactory<BackgroundSyncManager> weak_ptr_factory_;
+
+  DISALLOW_COPY_AND_ASSIGN(BackgroundSyncManager);
+};
+
+}  // namespace content
+
+#endif  // CONTENT_BROWSER_BACKGROUND_SYNC_BACKGROUND_SYNC_MANAGER_H_