Refactor CdmPromise and related classes

content/renderer/media/cdm_result_promise.cc

 // 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.
 
 #include "content/renderer/media/cdm_result_promise.h"
 
-#include "base/bind.h"
 #include "base/logging.h"
+#include "base/metrics/histogram.h"
 #include "third_party/WebKit/public/platform/WebString.h"
 
 namespace content {
 
 static blink::WebContentDecryptionModuleException ConvertException(
     media::MediaKeys::Exception exception_code) {
   switch (exception_code) {
     case media::MediaKeys::NOT_SUPPORTED_ERROR:
       return blink::WebContentDecryptionModuleExceptionNotSupportedError;
     case media::MediaKeys::INVALID_STATE_ERROR:
       return blink::WebContentDecryptionModuleExceptionInvalidStateError;
     case media::MediaKeys::INVALID_ACCESS_ERROR:
       return blink::WebContentDecryptionModuleExceptionInvalidAccessError;
     case media::MediaKeys::QUOTA_EXCEEDED_ERROR:
       return blink::WebContentDecryptionModuleExceptionQuotaExceededError;
     case media::MediaKeys::UNKNOWN_ERROR:
       return blink::WebContentDecryptionModuleExceptionUnknownError;
     case media::MediaKeys::CLIENT_ERROR:
       return blink::WebContentDecryptionModuleExceptionClientError;
     case media::MediaKeys::OUTPUT_ERROR:
       return blink::WebContentDecryptionModuleExceptionOutputError;
-    default:
-      NOTREACHED();
-      return blink::WebContentDecryptionModuleExceptionUnknownError;
   }
+  NOTREACHED();
+  return blink::WebContentDecryptionModuleExceptionUnknownError;
 }
 
-template <typename T>
-CdmResultPromise<T>::CdmResultPromise(
-    const blink::WebContentDecryptionModuleResult& result)
-    : media::CdmPromiseTemplate<T>(
-          base::Bind(&CdmResultPromise::OnResolve, base::Unretained(this)),
-          base::Bind(&CdmResultPromise::OnReject, base::Unretained(this))),
-      web_cdm_result_(result) {
+static ResultCodeForUMA ConvertExceptionToUMAResult(
+    media::MediaKeys::Exception exception_code) {
+  switch (exception_code) {
+    case media::MediaKeys::NOT_SUPPORTED_ERROR:
+      return NOT_SUPPORTED_ERROR;
+    case media::MediaKeys::INVALID_STATE_ERROR:
+      return INVALID_STATE_ERROR;
+    case media::MediaKeys::INVALID_ACCESS_ERROR:
+      return INVALID_ACCESS_ERROR;
+    case media::MediaKeys::QUOTA_EXCEEDED_ERROR:
+      return QUOTA_EXCEEDED_ERROR;
+    case media::MediaKeys::UNKNOWN_ERROR:
+      return UNKNOWN_ERROR;
+    case media::MediaKeys::CLIENT_ERROR:
+      return CLIENT_ERROR;
+    case media::MediaKeys::OUTPUT_ERROR:
+      return OUTPUT_ERROR;
+  }
+  NOTREACHED();
+  return UNKNOWN_ERROR;
 }
 
-template <typename T>
-CdmResultPromise<T>::CdmResultPromise(
+static void GenerateUMA(std::string uma_name, ResultCodeForUMA result) {

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

s/Generate/Report

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

Done.

+  if (uma_name.empty())
+    return;
+
+  base::LinearHistogram::FactoryGet(
+      uma_name,
+      1,
+      NUM_RESULT_CODES,
+      NUM_RESULT_CODES + 1,
+      base::HistogramBase::kUmaTargetedHistogramFlag)->Add(result);
+}
+
+template <typename... T>
+CdmResultPromise<T...>::CdmResultPromise(
     const blink::WebContentDecryptionModuleResult& result,
     const std::string& uma_name)
-    : media::CdmPromiseTemplate<T>(
-          base::Bind(&CdmResultPromise::OnResolve, base::Unretained(this)),
-          base::Bind(&CdmResultPromise::OnReject, base::Unretained(this)),
-          uma_name),
-      web_cdm_result_(result) {
+    : media::CdmPromiseTemplate<T...>(),

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

Is this necessary? 

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

Doesn't look like it. Removed.

+      web_cdm_result_(result),
+      uma_name_(uma_name) {
 }
 
-template <typename T>
-CdmResultPromise<T>::~CdmResultPromise() {
+template <typename... T>
+CdmResultPromise<T...>::~CdmResultPromise() {
+}
+
+template <typename... T>
+void CdmResultPromise<T...>::resolve(const T&... result) {
+  // As there are specializations for std::string and media::KeyIdsVector below,
+  // this should only be used for the 0 argument version.

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

In this case, change this to

template<>
void CdmResultPromise<> or void CdmResultPromise<void>?

Not sure which version is the correct one...

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

Done.

+  DCHECK(sizeof...(T) == 0);

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

DCHECK_EQ

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

Removed now that this is specific for CdmResultPromise<>.

+  this->PromiseSettled();
+  GenerateUMA(uma_name_, SUCCESS);
+  web_cdm_result_.complete();
 }
 
 template <>
-void CdmResultPromise<std::string>::OnResolve(const std::string& result) {
-  // This must be overridden in a subclass.
-  NOTREACHED();
+void CdmResultPromise<std::string>::resolve(const std::string& result) {
+  // This must be overridden in a subclass. However, this method must be called
+  // from any overrides to mark the promise as settled and report to UMA.
+  this->PromiseSettled();
+  GenerateUMA(uma_name_, SUCCESS);

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

Comment why we don't call resolve here. It's hard to understand without looking
at the derived class.

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

Done.

 }
 
 template <>
-void CdmResultPromise<media::KeyIdsVector>::OnResolve(
+void CdmResultPromise<media::KeyIdsVector>::resolve(
     const media::KeyIdsVector& result) {
   // TODO(jrummell): Update blink::WebContentDecryptionModuleResult to
   // handle the set of keys.
-  OnReject(media::MediaKeys::NOT_SUPPORTED_ERROR, 0, "Not implemented.");
+  reject(media::MediaKeys::NOT_SUPPORTED_ERROR, 0, "Not implemented.");
 }
 
-template <typename T>
-void CdmResultPromise<T>::OnReject(media::MediaKeys::Exception exception_code,
-                                   uint32 system_code,
-                                   const std::string& error_message) {
-  web_cdm_result_.completeWithError(ConvertException(exception_code),
-                                    system_code,
-                                    blink::WebString::fromUTF8(error_message));
-}
-
-CdmResultPromise<void>::CdmResultPromise(
-    const blink::WebContentDecryptionModuleResult& result)
-    : media::CdmPromiseTemplate<void>(
-          base::Bind(&CdmResultPromise::OnResolve, base::Unretained(this)),
-          base::Bind(&CdmResultPromise::OnReject, base::Unretained(this))),
-      web_cdm_result_(result) {
-}
-
-CdmResultPromise<void>::CdmResultPromise(
-    const blink::WebContentDecryptionModuleResult& result,
-    const std::string& uma_name)
-    : media::CdmPromiseTemplate<void>(
-          base::Bind(&CdmResultPromise::OnResolve, base::Unretained(this)),
-          base::Bind(&CdmResultPromise::OnReject, base::Unretained(this)),
-          uma_name),
-      web_cdm_result_(result) {
-}
-
-CdmResultPromise<void>::~CdmResultPromise() {
-}
-
-void CdmResultPromise<void>::OnResolve() {
-  web_cdm_result_.complete();
-}
-
-void CdmResultPromise<void>::OnReject(
-    media::MediaKeys::Exception exception_code,
-    uint32 system_code,
-    const std::string& error_message) {
+template <typename... T>
+void CdmResultPromise<T...>::reject(media::MediaKeys::Exception exception_code,
+                                    uint32 system_code,
+                                    const std::string& error_message) {
+  this->PromiseSettled();
+  GenerateUMA(uma_name_, ConvertExceptionToUMAResult(exception_code));
   web_cdm_result_.completeWithError(ConvertException(exception_code),
                                     system_code,
                                     blink::WebString::fromUTF8(error_message));
 }
 
 // Explicit template instantiation for the templates needed.
+template class CdmResultPromise<>;
 template class CdmResultPromise<std::string>;
 template class CdmResultPromise<media::KeyIdsVector>;
 
 }  // namespace content