Initial RemoteCommandsInvalidator

chrome/browser/policy/cloud/remote_commands_invalidator_base.h

+// 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 CHROME_BROWSER_POLICY_CLOUD_REMOTE_COMMANDS_INVALIDATOR_BASE_H_
+#define CHROME_BROWSER_POLICY_CLOUD_REMOTE_COMMANDS_INVALIDATOR_BASE_H_
+
+#include "base/macros.h"
+#include "base/memory/ref_counted.h"
+#include "base/memory/weak_ptr.h"
+#include "base/threading/thread_checker.h"
+#include "components/invalidation/invalidation_handler.h"
+#include "google/cacheinvalidation/include/types.h"
+#include "policy/proto/device_management_backend.pb.h"
+
+namespace base {
+class SequencedTaskRunner;
+}  // namespace base
+
+namespace invalidation {
+class InvalidationService;
+}  // namespace invalidation
+
+namespace policy {
+
+// Base class for RemoteCommandsInvalidator, this class doesn't interact with
+// CloudPolicyStore and CloudPolicyCore directly. Instead, several interface
+// functions are provided, to allow subclasses to integrate with cloud policy
+// infrastructure.
+class RemoteCommandsInvalidatorBase : public syncer::InvalidationHandler {
+ public:
+  // |task_runner| will be used to post DoRemoteCommandsFetch() tasks.

[bartfab (slow), 2015/05/15 15:37:12]

1: Nit: Document that this needs to be the current thread's task runner.
2: Remove this dependency and use ThreadTaskRunnerHandle::Get() instead. I feel
strongly about this. It will ensure that one cannot shoot themselves in the foot
by passing a task runner for some other thread by accident.

[binjin, 2015/05/15 15:46:46]

Acknowledged. task_runner was removed.

+  explicit RemoteCommandsInvalidatorBase(
+      const scoped_refptr<base::SequencedTaskRunner>& task_runner);
+  ~RemoteCommandsInvalidatorBase() override;
+
+  // Initialize this invalidator to pair with |invalidation_service|. Must be
+  // called before Start().
+  void Initialize(invalidation::InvalidationService* invalidation_service);
+
+  // Shutdown this invalidator. Will stop the invalidator first, and after
+  // shutting down, the invalidator can't be started anymore.

[bartfab (slow), 2015/05/15 15:37:12]

Well, it can be started by calling Initialize() again.

[binjin, 2015/05/15 15:46:46]

Done.

+  void Shutdown();
+
+  // Starts to process the invalidations.

[bartfab (slow), 2015/05/15 15:37:12]

Nit: s/the //

[binjin, 2015/05/15 15:46:46]

Done.

+  void Start();
+
+  // Stops to process the invalidation. Must be called after Start().

[bartfab (slow), 2015/05/15 15:37:12]

Nit 1: s/the invalidation/invalidations/
Nit 2: "Must be called after Start()" makes it sound like you must always call
this method after starting. How about "May only be called after Start() has been
called?"

[binjin, 2015/05/15 15:46:46]

Done.

+  void Stop();
+
+  // Helpful accessors.
+  invalidation::InvalidationService* invalidation_service() {
+    return invalidation_service_;
+  }
+  bool invalidations_enabled() { return invalidations_enabled_; }
+
+  // syncer::InvalidationHandler:
+  void OnInvalidatorStateChange(syncer::InvalidatorState state) override;
+  void OnIncomingInvalidation(
+      const syncer::ObjectIdInvalidationMap& invalidation_map) override;
+  std::string GetOwnerName() const override;
+
+ protected:
+  // Subclasses should override the following functions, if they want to get

[bartfab (slow), 2015/05/15 15:37:12]

Subclasses actually *must* override these methods, since there are pure virtual
:). I think you can remove the comment.

[binjin, 2015/05/15 15:46:46]

Done.

+  // notified on corresponding events.
+  virtual void OnInitialize() = 0;
+  virtual void OnShutdown() = 0;
+  virtual void OnStart() = 0;
+  virtual void OnStop() = 0;
+
+  // Subclasses must override this method to implements the actual remote

[bartfab (slow), 2015/05/15 15:37:12]

Nit: s/implements/implement/

[binjin, 2015/05/15 15:46:46]

Done.

+  // commands fetch. A callback to this function will be posted to

[bartfab (slow), 2015/05/15 15:37:12]

Nit: s/callback/call/

[binjin, 2015/05/15 15:46:46]

Done.

+  // |task_runner_|.
+  virtual void DoRemoteCommandsFetch() = 0;
+
+  // Subclasses must call this function to set the object id for remote command
+  // invalidations.
+  void ReloadPolicyData(const enterprise_management::PolicyData* policy);
+
+ private:
+  // Registers the given object with the invalidation service.
+  void Register(const invalidation::ObjectId& object_id);
+
+  // Unregisters the current object with the invalidation service.
+  void Unregister();
+
+  // Updates invalidations_enabled_.
+  void UpdateInvalidationsEnabled();
+
+  // The state of the object.
+  enum State {
+    SHUT_DOWN,
+    STOPPED,
+    STARTED,
+  };
+  State state_ = SHUT_DOWN;
+
+  // Schedules delayed tasks.
+  const scoped_refptr<base::SequencedTaskRunner> task_runner_;
+
+  // The invalidation service.
+  invalidation::InvalidationService* invalidation_service_ = nullptr;
+
+  // Whether the invalidator currently has the ability to receive invalidations.
+  // This is true if the invalidation service is enabled and the invalidator
+  // has registered for a remote commands object.
+  bool invalidations_enabled_ = false;
+
+  // Whether the invalidation service is currently enabled.
+  bool invalidation_service_enabled_ = false;
+
+  // Whether this object has registered for remote commands invalidations.
+  bool is_registered_ = false;
+
+  // The object id representing the remote commands in the invalidation service.
+  invalidation::ObjectId object_id_;
+
+  // A thread checker to make sure that callbacks are invoked on the correct
+  // thread.
+  base::ThreadChecker thread_checker_;
+
+  // WeakPtrFactory used to create callbacks to this object.
+  base::WeakPtrFactory<RemoteCommandsInvalidatorBase> weak_factory_;
+
+  DISALLOW_COPY_AND_ASSIGN(RemoteCommandsInvalidatorBase);
+};
+
+}  // namespace policy
+
+#endif  // CHROME_BROWSER_POLICY_CLOUD_REMOTE_COMMANDS_INVALIDATOR_BASE_H_