media: Reject pending CDM promise during destruction.

media/base/cdm_callback_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 "media/base/cdm_callback_promise.h"
 
+#include "base/callback_helpers.h"
 #include "base/logging.h"
 
 namespace media {
 
 template <typename... T>
 CdmCallbackPromise<T...>::CdmCallbackPromise(
     const base::Callback<void(const T&...)>& resolve_cb,
     const PromiseRejectedCB& reject_cb)
     : resolve_cb_(resolve_cb), reject_cb_(reject_cb) {
   DCHECK(!resolve_cb_.is_null());
   DCHECK(!reject_cb_.is_null());
 }
 
 template <typename... T>
 CdmCallbackPromise<T...>::~CdmCallbackPromise() {
+  if (resolve_cb_.is_null() || reject_cb_.is_null())
+    return;

[ddorwin, 2016/02/25 19:14:12]

// Promise has already been settled.
?

Or just add and call IsPromiseSettled(), which would serve as the comment, as
discussed below. This might be less fragile than assuming the CBs are reset in
all implementations (see comment below).

+
+  std::string message =
+      "Unfulfilled promise rejected automatically during destruction.";
+  DVLOG(1) << message;
+  reject(MediaKeys::INVALID_STATE_ERROR, 0, message);
 }
 
 template <typename... T>
 void CdmCallbackPromise<T...>::resolve(const T&... result) {
   MarkPromiseSettled();

[jrummell, 2016/02/25 02:30:03]

This call sets a flag to indicate that the promise is settled. There is the
DCHECK in CdmPromiseTemplate() to make sure it is settled upon destruction. Can
you not simply make the change there, so it's only in one place?

[xhwang, 2016/02/25 07:49:21]

In CdmPromiseTemplate's destructor, I can't call reject() because it's virtual.
Basically the subclass will be destroyed before the base class, so when we are
at the base class's destructor, it's too late to ask the subclass to do
anything. As a result, e.g. the reject_cb_ here will never be called.

[ddorwin, 2016/02/25 19:14:12]

Is there still a possibility that specifications/children will not *reset*
(breaking the check above)?

-  resolve_cb_.Run(result...);
+  base::ResetAndReturn(&resolve_cb_).Run(result...);
 }
 
 template <typename... T>
 void CdmCallbackPromise<T...>::reject(MediaKeys::Exception exception_code,
                                       uint32_t system_code,
                                       const std::string& error_message) {
   MarkPromiseSettled();
-  reject_cb_.Run(exception_code, system_code, error_message);
+  base::ResetAndReturn(&reject_cb_)
+      .Run(exception_code, system_code, error_message);
 }
 
 // Explicit template instantiation for the Promises needed.
 template class MEDIA_EXPORT CdmCallbackPromise<>;
 template class MEDIA_EXPORT CdmCallbackPromise<std::string>;
 
 }  // namespace media