VideoDecodeAccelerator now SupportsWeakPtr instead of being RefCountedThreadSafe.

media/video/video_decode_accelerator.h

 // Copyright (c) 2012 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 MEDIA_VIDEO_VIDEO_DECODE_ACCELERATOR_H_
 #define MEDIA_VIDEO_VIDEO_DECODE_ACCELERATOR_H_
 
 #include <vector>
 
 #include "base/basictypes.h"
+#include "base/memory/weak_ptr.h"
 #include "media/base/bitstream_buffer.h"
 #include "media/base/video_decoder_config.h"
 #include "media/video/picture.h"
 #include "ui/gfx/size.h"
 
 namespace media {
 
 // Video decoder interface.
 // This interface is extended by the various components that ultimately
 // implement the backend of PPB_VideoDecode_Dev.
-//
-// No thread-safety guarantees are implied by the use of RefCountedThreadSafe
-// below.
 class MEDIA_EXPORT VideoDecodeAccelerator
-    : public base::RefCountedThreadSafe<VideoDecodeAccelerator> {
+    : public base::SupportsWeakPtr<VideoDecodeAccelerator> {
  public:
+  virtual ~VideoDecodeAccelerator();
+
   // Enumeration of potential errors generated by the API.
   // Note: Keep these in sync with PP_VideoDecodeError_Dev.
   enum Error {
     // An operation was attempted during an incompatible decoder state.
     ILLEGAL_STATE = 1,
     // Invalid argument was passed to an API method.
     INVALID_ARGUMENT,
     // Encoded input is unreadable.
     UNREADABLE_INPUT,
     // A failure occurred at the browser layer or one of its dependencies.
@@ skipping 83 matching lines @@
   // Resets the decoder: all pending inputs are dropped immediately and the
   // decoder returned to a state ready for further Decode()s, followed by
   // NotifyResetDone() being called on the client.  Can be used to implement
   // "seek".
   virtual void Reset() = 0;
 
   // Destroys the decoder: all pending inputs are dropped immediately and the
   // component is freed.  This call may asynchornously free system resources,
   // but its client-visible effects are synchronous.  After this method returns
   // no more callbacks will be made on the client.
-  virtual void Destroy() = 0;
-
- protected:
-  friend class base::RefCountedThreadSafe<VideoDecodeAccelerator>;
-  virtual ~VideoDecodeAccelerator();
+  // Because the client interface is synchronous but implementations may be
+  // asynchronous, the caller (who may own |this|) can pass ownership of the VDA
+  // *to* the VDA for hidden shenaniganary.
+  virtual void Destroy(bool pass_ownership) = 0;

[piman, 2012/07/17 17:14:28]

So I looked carefully at the implementations. There's only 2 places where
Destroy(false) is called, and that's internally to the Mac and the VAAPI
decoders. It's never called externally.
From what I can tell in both those cases above, Destroy will be called twice,
once with false on error, which does cleanup but doesn't delete the object, and
a second time with true, which doesn't really do any cleanup (because it's
already been done) but then deletes the object.

It sounds to me it would be simpler, and better served everywhere to make the
public API always take ownership (and remove the bool). On Mac and VAAPI
decoders, you can have a private Cleanup function, that's called from
StopOnError and NotifyError respectively, as well as Destroy.

WDYT?

[Ami GONE FROM CHROMIUM, 2012/07/17 17:58:50]

SGTM.  Done.

 };
 
 }  // namespace media
 
 #endif  // MEDIA_VIDEO_VIDEO_DECODE_ACCELERATOR_H_