Signed settings refactoring: Proper caching and more tests.

chrome/browser/chromeos/login/signed_settings_helper.cc

 // Copyright (c) 2011 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 "chrome/browser/chromeos/login/signed_settings_helper.h"
 
 #include <string>
 #include <vector>
 
 #include "base/lazy_instance.h"
@@ skipping 31 matching lines @@
     // Note that the context could be released when op_->Execute() returns.
     // So keep a local copy of delegate and executing flag to use after
     // the call.
     Delegate* delegate = delegate_;
     executing_ = true;
     op_->Execute();
     if (delegate)
       delegate->OnOpStarted(this);
   }
 
-  // Cancels the callback.
-  void CancelCallback() {
-    callback_ = NULL;
-  }
-
   // Cancels the callback and cancels the op if it is not executing.
   void Cancel() {
-    CancelCallback();
-
     if (!executing_)
       OnOpCompleted();
   }
 
   // Accessors.
   SignedSettings* op() const {
     return op_.get();
   }
 
-  SignedSettingsHelper::Callback* callback() const {
-    return callback_;
-  }
-
   void set_delegate(Delegate* delegate) {
     delegate_ = delegate;
   }
 
  protected:
-  OpContext(SignedSettingsHelper::Callback* callback,
-            Delegate* delegate)
+  explicit OpContext(Delegate* delegate)
       : executing_(false),
-        delegate_(delegate),
-        callback_(callback) {
+        delegate_(delegate) {
   }
 
   // Creates the op to execute.
   virtual void CreateOp() = 0;
 
   // Callback on op completion.
   virtual void OnOpCompleted() {
     if (delegate_)
       delegate_->OnOpCompleted(this);
 
     delete this;
   }
 
   bool executing_;
   Delegate* delegate_;
 
   scoped_refptr<SignedSettings> op_;
-  SignedSettingsHelper::Callback* callback_;
 };
 
-class StorePropertyOpContext : public SignedSettings::Delegate<bool>,
-                               public OpContext {
- public:
-  StorePropertyOpContext(const std::string& name,
-                         const base::Value& value,
-                         SignedSettingsHelper::Callback* callback,
-                         OpContext::Delegate* delegate)
-      : OpContext(callback, delegate),
-        name_(name),
-        value_(value.DeepCopy()) {
-  }
-
-  // chromeos::SignedSettings::Delegate implementation
-  virtual void OnSettingsOpCompleted(SignedSettings::ReturnCode code,
-                                     bool unused) OVERRIDE {
-    VLOG(2) << "OnSettingsOpCompleted, code = " << code;
-    if (callback_)
-      callback_->OnStorePropertyCompleted(code, name_, *value_);
-    OnOpCompleted();
-  }
-
- protected:
-  // OpContext implemenetation
-  virtual void CreateOp() OVERRIDE {
-    op_ = SignedSettings::CreateStorePropertyOp(name_, *value_, this);
-  }
-
- private:
-  std::string name_;
-  scoped_ptr<base::Value> value_;
-
-  DISALLOW_COPY_AND_ASSIGN(StorePropertyOpContext);
-};
-
-class RetrievePropertyOpContext
-    : public SignedSettings::Delegate<const base::Value*>,
+class StorePolicyOpContext
+    : public SignedSettings::Delegate<bool>,
       public OpContext {
  public:
-  RetrievePropertyOpContext(const std::string& name,
-                            SignedSettingsHelper::Callback* callback,
-                            OpContext::Delegate* delegate)
-      : OpContext(callback, delegate),
-        name_(name) {
-  }
-
-  // chromeos::SignedSettings::Delegate implementation
-  virtual void OnSettingsOpCompleted(SignedSettings::ReturnCode code,
-                                     const base::Value* value) OVERRIDE {
-    if (callback_)
-      callback_->OnRetrievePropertyCompleted(code, name_, value);
-
-    OnOpCompleted();
-  }
-
- protected:
-  // OpContext implemenetation
-  virtual void CreateOp() OVERRIDE {
-    op_ = SignedSettings::CreateRetrievePropertyOp(name_, this);
-  }
-
- private:
-  std::string name_;
-
-  DISALLOW_COPY_AND_ASSIGN(RetrievePropertyOpContext);
-};
-
-class StorePolicyOpContext : public SignedSettings::Delegate<bool>,
-                             public OpContext {
- public:
   StorePolicyOpContext(const em::PolicyFetchResponse& policy,
-                       SignedSettingsHelper::Callback* callback,
+                       SignedSettingsHelper::StorePolicyCallback callback,
                        OpContext::Delegate* delegate)
-      : OpContext(callback, delegate),
+      : OpContext(delegate),
+        callback_(callback),
         policy_(policy) {
   }
 
   // chromeos::SignedSettings::Delegate implementation
   virtual void OnSettingsOpCompleted(SignedSettings::ReturnCode code,
                                      bool unused) OVERRIDE {
     VLOG(2) << "OnSettingsOpCompleted, code = " << code;
-    if (callback_)
-      callback_->OnStorePolicyCompleted(code);
+    callback_.Run(code);
     OnOpCompleted();
   }
 
  protected:
   // OpContext implementation
   virtual void CreateOp() OVERRIDE {
     op_ = SignedSettings::CreateStorePolicyOp(&policy_, this);
   }
 
  private:
+  SignedSettingsHelper::StorePolicyCallback callback_;
   em::PolicyFetchResponse policy_;
+
   DISALLOW_COPY_AND_ASSIGN(StorePolicyOpContext);
 };
 
 class RetrievePolicyOpContext
     : public SignedSettings::Delegate<const em::PolicyFetchResponse&>,
       public OpContext {
  public:
-  RetrievePolicyOpContext(SignedSettingsHelper::Callback* callback,
+  RetrievePolicyOpContext(SignedSettingsHelper::RetrievePolicyCallback callback,
                           OpContext::Delegate* delegate)
-      : OpContext(callback, delegate) {
+      : OpContext(delegate),
+        callback_(callback){
   }
 
   // chromeos::SignedSettings::Delegate implementation
   virtual void OnSettingsOpCompleted(
       SignedSettings::ReturnCode code,
       const em::PolicyFetchResponse& policy) OVERRIDE {
-    if (callback_)
-      callback_->OnRetrievePolicyCompleted(code, policy);
+    callback_.Run(code, policy);
     OnOpCompleted();
   }
 
  protected:
   // OpContext implementation
   virtual void CreateOp() OVERRIDE {
     op_ = SignedSettings::CreateRetrievePolicyOp(this);
   }
 
  private:
+  SignedSettingsHelper::RetrievePolicyCallback callback_;
+
   DISALLOW_COPY_AND_ASSIGN(RetrievePolicyOpContext);
 };
 
 }  // namespace
 
 
 class SignedSettingsHelperImpl : public SignedSettingsHelper,
                                  public OpContext::Delegate {
  public:
   // SignedSettingsHelper implementation
-  virtual void StartStorePropertyOp(const std::string& name,
-                                    const base::Value& value,
-                                    Callback* callback) OVERRIDE;
-  virtual void StartRetrieveProperty(const std::string& name,
-                                     Callback* callback) OVERRIDE;
   virtual void StartStorePolicyOp(const em::PolicyFetchResponse& policy,
-                                  Callback* callback) OVERRIDE;
-  virtual void StartRetrievePolicyOp(Callback* callback) OVERRIDE;
-  virtual void CancelCallback(Callback* callback) OVERRIDE;
+                                  StorePolicyCallback callback) OVERRIDE;
+  virtual void StartRetrievePolicyOp(RetrievePolicyCallback callback) OVERRIDE;
 
   // OpContext::Delegate implementation
   virtual void OnOpCreated(OpContext* context);
   virtual void OnOpStarted(OpContext* context);
   virtual void OnOpCompleted(OpContext* context);
 
  private:
   SignedSettingsHelperImpl();
-  ~SignedSettingsHelperImpl();
+  virtual ~SignedSettingsHelperImpl();
 
   void AddOpContext(OpContext* context);
   void ClearAll();
 
   std::vector<OpContext*> pending_contexts_;
 
   friend struct base::DefaultLazyInstanceTraits<SignedSettingsHelperImpl>;
   DISALLOW_COPY_AND_ASSIGN(SignedSettingsHelperImpl);
 };
 
 static base::LazyInstance<SignedSettingsHelperImpl>
     g_signed_settings_helper_impl = LAZY_INSTANCE_INITIALIZER;
 
 SignedSettingsHelperImpl::SignedSettingsHelperImpl() {
 }
 
 SignedSettingsHelperImpl::~SignedSettingsHelperImpl() {
   if (!pending_contexts_.empty()) {
     LOG(WARNING) << "SignedSettingsHelperImpl shutdown with pending ops, "
                  << "changes will be lost.";
     ClearAll();
   }
 }
 
-void SignedSettingsHelperImpl::StartStorePropertyOp(
-    const std::string& name,
-    const base::Value& value,
-    SignedSettingsHelper::Callback* callback) {
-  AddOpContext(new StorePropertyOpContext(
-      name,
-      value,
-      callback,
-      this));
-}
-
-void SignedSettingsHelperImpl::StartRetrieveProperty(
-    const std::string& name,
-    SignedSettingsHelper::Callback* callback) {
-  AddOpContext(new RetrievePropertyOpContext(
-      name,
-      callback,
-      this));
-}
-
 void SignedSettingsHelperImpl::StartStorePolicyOp(
     const em::PolicyFetchResponse& policy,
-    SignedSettingsHelper::Callback* callback) {
+    StorePolicyCallback callback) {
   AddOpContext(new StorePolicyOpContext(policy, callback, this));
 }
 
 void SignedSettingsHelperImpl::StartRetrievePolicyOp(
-    SignedSettingsHelper::Callback* callback) {
+    RetrievePolicyCallback callback) {
   AddOpContext(new RetrievePolicyOpContext(callback, this));
 }
 
-void SignedSettingsHelperImpl::CancelCallback(
-    SignedSettingsHelper::Callback* callback) {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-
-  for (size_t i = 0; i < pending_contexts_.size(); ++i) {
-    if (pending_contexts_[i]->callback() == callback) {
-      pending_contexts_[i]->CancelCallback();
-    }
-  }
-}
-
 void SignedSettingsHelperImpl::AddOpContext(OpContext* context) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   CHECK(context);
 
   pending_contexts_.push_back(context);
   if (pending_contexts_.size() == 1)
     context->Execute();
 }
 
 void SignedSettingsHelperImpl::ClearAll() {
@@ skipping 26 matching lines @@
 
   if (test_delegate_)
     test_delegate_->OnOpCompleted(context->op());
 }
 
 SignedSettingsHelper* SignedSettingsHelper::Get() {
   return g_signed_settings_helper_impl.Pointer();
 }
 
 }  // namespace chromeos