Refactor CdmPromise and related classes

media/base/cdm_promise.h

Index: media/base/cdm_promise.h
diff --git a/media/base/cdm_promise.h b/media/base/cdm_promise.h
index 8e6703d0c7dd63c7074f272725781c73d0804d98..f947add1946d77d8e4086a288a30eaf26a16cf73 100644
--- a/media/base/cdm_promise.h
+++ b/media/base/cdm_promise.h
@@ -8,7 +8,6 @@
 #include <string>
 
 #include "base/basictypes.h"
-#include "base/callback.h"
 #include "media/base/media_export.h"
 #include "media/base/media_keys.h"
 
@@ -18,34 +17,25 @@ namespace media {
 // session actions. These may be called synchronously or asynchronously.
 // The promise must be resolved or rejected exactly once. It is expected that
 // the caller free the promise once it is resolved/rejected.
-//
-// This is only the base class, as parameter to resolve() varies.
+
+// These classes are almost generic, except for the parameters to reject(). If
+// a generic class for promises is available, this could be changed to use the
+// generic class as long as the parameters to reject() can be set appropriately.
+
+// The base class only has a reject() method and GetResolveParameterType() that
+// indicates the type of CdmPromiseTemplate. CdmPromiseTemplate<T> adds the
+// resolve(T) method that is dependent on the type of promise. This base class
+// is specified so that the promises can be easily saved before passing across
+// the pepper interface.
 class MEDIA_EXPORT CdmPromise {
  public:
-  // A superset of media::MediaKeys::Exception for UMA reporting.
-  enum ResultCodeForUMA {
-    SUCCESS,
-    NOT_SUPPORTED_ERROR,
-    INVALID_STATE_ERROR,
-    INVALID_ACCESS_ERROR,
-    QUOTA_EXCEEDED_ERROR,
-    UNKNOWN_ERROR,
-    CLIENT_ERROR,
-    OUTPUT_ERROR,
-    NUM_RESULT_CODES
-  };
-
   enum ResolveParameterType {
     VOID_TYPE,
     STRING_TYPE,
     KEY_IDS_VECTOR_TYPE
   };
 
-  typedef base::Callback<void(MediaKeys::Exception exception_code,
-                              uint32 system_code,
-                              const std::string& error_message)>
-      PromiseRejectedCB;
-
+  CdmPromise();
   virtual ~CdmPromise();
 
   // Used to indicate that the operation failed. |exception_code| must be
@@ -54,76 +44,40 @@ class MEDIA_EXPORT CdmPromise {
   // codes are not supported by the Key System. |error_message| is optional.
   virtual void reject(MediaKeys::Exception exception_code,
                       uint32 system_code,
-                      const std::string& error_message);
-
-  ResolveParameterType GetResolveParameterType() const {
-    return parameter_type_;
-  }
-
- protected:
-  explicit CdmPromise(ResolveParameterType parameter_type);
-  CdmPromise(ResolveParameterType parameter_type, PromiseRejectedCB reject_cb);
-
-  // If constructed with a |uma_name| (which must be the name of a
-  // CdmPromiseResult UMA), CdmPromise will report the promise result (success
-  // or rejection code).
-  CdmPromise(ResolveParameterType parameter_type,
-             PromiseRejectedCB reject_cb,
-             const std::string& uma_name);
-
-  // Called by all resolve()/reject() methods to report the UMA result if
-  // applicable, and update |is_pending_|.
-  void ReportResultToUMA(ResultCodeForUMA result);
-
-  const ResolveParameterType parameter_type_;
-  PromiseRejectedCB reject_cb_;
-
-  // Keep track of whether the promise hasn't been resolved or rejected yet.
-  bool is_pending_;
+                      const std::string& error_message) = 0;
 
-  // UMA name to report result to.
-  std::string uma_name_;
+  // Used to determine the template type of CdmPromiseTemplate<T> so that
+  // saved CdmPromise objects can be cast to the corrent templated version.

[xhwang, 2014/10/03 07:34:21]

s/corrent/correct

[jrummell, 2014/10/03 18:58:30]

Done.

+  virtual ResolveParameterType GetResolveParameterType() const = 0;
 
+ private:
   DISALLOW_COPY_AND_ASSIGN(CdmPromise);
 };
 
-template <typename T>
+// This class adds the resolve(T) method. This class is still an interface, and
+// is used as the type of promise that gets passed around.
+template <typename... T>
 class MEDIA_EXPORT CdmPromiseTemplate : public CdmPromise {
  public:
-  CdmPromiseTemplate(base::Callback<void(const T&)> resolve_cb,
-                     PromiseRejectedCB rejected_cb);
-  CdmPromiseTemplate(base::Callback<void(const T&)> resolve_cb,
-                     PromiseRejectedCB rejected_cb,
-                     const std::string& uma_name);
-  virtual void resolve(const T& result);
-
- protected:
-  // Allow subclasses to completely override the implementation.
   CdmPromiseTemplate();
+  virtual ~CdmPromiseTemplate();
 
- private:
-  base::Callback<void(const T&)> resolve_cb_;
-
-  DISALLOW_COPY_AND_ASSIGN(CdmPromiseTemplate);
-};
+  virtual void resolve(const T&... result) = 0;
 
-// Specialization for no parameter to resolve().
-template <>
-class MEDIA_EXPORT CdmPromiseTemplate<void> : public CdmPromise {
- public:
-  CdmPromiseTemplate(base::Callback<void(void)> resolve_cb,
-                     PromiseRejectedCB rejected_cb);
-  CdmPromiseTemplate(base::Callback<void(void)> resolve_cb,
-                     PromiseRejectedCB rejected_cb,
-                     const std::string& uma_name);
-  virtual void resolve();
+  // CdmPromise implementation.
+  virtual void reject(MediaKeys::Exception exception_code,
+                      uint32 system_code,
+                      const std::string& error_message) = 0;
+  virtual ResolveParameterType GetResolveParameterType() const OVERRIDE;
 
  protected:
-  // Allow subclasses to completely override the implementation.
-  CdmPromiseTemplate();
+  // All implementations must call this method in resolve() and reject() methods
+  // to indicate that the promise has been settled.
+  void PromiseSettled();

[xhwang, 2014/10/03 07:34:22]

Use a verb? SettlePromise() or SetPromiseSettled()?

[jrummell, 2014/10/03 18:58:30]

MarkPromiseSettled() since it doesn't actually settle the promise ... just
indicates that it is settled.

 
  private:
-  base::Callback<void(void)> resolve_cb_;
+  // Keep track of whether the promise hasn't been resolved or rejected yet.
+  bool is_pending_;

[xhwang, 2014/10/03 07:34:22]

is_settled_?

[jrummell, 2014/10/03 18:58:30]

Done.

 
   DISALLOW_COPY_AND_ASSIGN(CdmPromiseTemplate);
 };