Refactor CdmPromise and related classes

content/renderer/media/cdm_result_promise.h

 // Copyright 2014 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_RENDERER_MEDIA_CDM_RESULT_PROMISE_H_
 #define CONTENT_RENDERER_MEDIA_CDM_RESULT_PROMISE_H_
 
 #include <map>
 
 #include "base/basictypes.h"
 #include "media/base/cdm_promise.h"
 #include "third_party/WebKit/public/platform/WebContentDecryptionModuleResult.h"
 
 namespace content {
 
+// A superset of media::MediaKeys::Exception for UMA reporting.
+enum ResultCodeForUMA {
+  SUCCESS,

[xhwang, 2014/09/29 18:32:58]

Assign numbers to these values to make sure they match 

https://code.google.com/p/chromium/codesearch#chromium/src/tools/metrics/hist...

Also add a comment that never change the value of these enums since they are
used for UMA.

[jrummell, 2014/09/29 22:08:38]

Done.

+  NOT_SUPPORTED_ERROR,
+  INVALID_STATE_ERROR,
+  INVALID_ACCESS_ERROR,
+  QUOTA_EXCEEDED_ERROR,
+  UNKNOWN_ERROR,
+  CLIENT_ERROR,
+  OUTPUT_ERROR,
+  NUM_RESULT_CODES
+};
+
 // Used to convert a WebContentDecryptionModuleResult into a CdmPromise so that
 // it can be passed through Chromium. When CdmPromise::resolve(T) is called,
 // OnResolve(T) will be called and will call the appropriate complete...()
 // method on WebContentDecryptionModuleResult. If CdmPromise::reject() is called
 // instead, WebContentDecryptionModuleResult::completeWithError() is called.
 // If constructed with a |uma_name| (which must be the name of a
 // CdmPromiseResult UMA), CdmResultPromise will report the promise result

[xhwang, 2014/09/29 18:32:58]

What is "CdmPromiseResult UMA"?

[jrummell, 2014/09/29 22:08:38]

Removed.

 // (success or rejection code).

[xhwang, 2014/09/29 18:32:58]

s/CdmPromise/CdmPromiseTemplate throughout this comment. e.g. we don't have
CdmPromise::resolve()...

[jrummell, 2014/09/29 22:08:38]

Done.

 template <typename T>
 class CdmResultPromise : public media::CdmPromiseTemplate<T> {
  public:
-  explicit CdmResultPromise(
-      const blink::WebContentDecryptionModuleResult& result);
   CdmResultPromise(const blink::WebContentDecryptionModuleResult& result,
                    const std::string& uma_name);
   virtual ~CdmResultPromise();
 
+  virtual void resolve(const T& result) OVERRIDE;

[xhwang, 2014/09/29 18:32:58]

// CdmPromiseTemplate<T> implementation.

[jrummell, 2014/09/29 22:08:38]

Done.

+  virtual void reject(media::MediaKeys::Exception exception_code,
+                      uint32 system_code,
+                      const std::string& error_message) OVERRIDE;
+
  protected:
-  // OnResolve() is virtual as it may need special handling in derived classes.
-  virtual void OnResolve(const T& result);
-  void OnReject(media::MediaKeys::Exception exception_code,
-                uint32 system_code,
-                const std::string& error_message);
+  // Called by all resolve()/reject() methods to report the UMA result if

[ddorwin, 2014/09/26 22:40:00]

// All implementations must call this method in resolve() and reject() to...

[jrummell, 2014/09/29 22:08:38]

Removed.

+  // applicable, and update |is_pending_|.
+  void ReportResultToUMA(ResultCodeForUMA result);
 
   blink::WebContentDecryptionModuleResult web_cdm_result_;
 
  private:
+  // UMA name to report result to.
+  std::string uma_name_;
+
+  // Keep track of whether the promise hasn't been resolved or rejected yet.
+  bool is_pending_;
+
   DISALLOW_COPY_AND_ASSIGN(CdmResultPromise);
 };
 
 // Specialization for no parameter to resolve().
 template <>
 class CdmResultPromise<void> : public media::CdmPromiseTemplate<void> {
  public:
-  explicit CdmResultPromise(
-      const blink::WebContentDecryptionModuleResult& result);
   CdmResultPromise(const blink::WebContentDecryptionModuleResult& result,
                    const std::string& uma_name);
   virtual ~CdmResultPromise();
 
- protected:
-  virtual void OnResolve();
-  void OnReject(media::MediaKeys::Exception exception_code,
-                uint32 system_code,
-                const std::string& error_message);
+  virtual void resolve() OVERRIDE;

[xhwang, 2014/09/29 18:32:58]

// CdmPromiseTemplate<void> implementation.

[jrummell, 2014/09/29 22:08:38]

Done.

+  virtual void reject(media::MediaKeys::Exception exception_code,
+                      uint32 system_code,
+                      const std::string& error_message) OVERRIDE;
+
+ private:
+  // Called by all resolve()/reject() methods to report the UMA result if

[ddorwin, 2014/09/26 22:40:00]

ditto

[jrummell, 2014/09/29 22:08:38]

Done.

+  // applicable, and update |is_pending_|.
+  void ReportResultToUMA(ResultCodeForUMA result);
 
   blink::WebContentDecryptionModuleResult web_cdm_result_;
 
- private:
+  // UMA name to report result to.

[ddorwin, 2014/09/26 22:40:00]

Kind of sad that we have to implement these members twice. :(

[jrummell, 2014/09/29 22:08:38]

Acknowledged.

+  std::string uma_name_;
+
+  // Keep track of whether the promise hasn't been resolved or rejected yet.
+  bool is_pending_;
+
   DISALLOW_COPY_AND_ASSIGN(CdmResultPromise);
 };
 
 typedef CdmResultPromise<void> SimpleCdmResultPromise;
 
 }  // namespace content
 
 #endif  // CONTENT_RENDERER_MEDIA_CDM_RESULT_PROMISE_H_