Introduce an IsMutable method on VideoFrameBuffer.

Unlike HasOneRef, it can be overridden to always return false in
immutable subclasses.

I'm also investigating overiding it in PooledI420Buffer, to directly
inherit I420Buffer but ignore the reference from the pool. Still
unclear if that will work out.

BUG=webrtc:5682

Committed: https://crrev.com/6bd10f2c1ac912cbe5addd880e559d59274c60e6
Cr-Commit-Position: refs/heads/master@{#12365}

R=magjed@webrtc.org, pbos@webrtc.org, perkj@webrtc.org

Review URL: https://codereview.webrtc.org/1881933004 .

Cr-Commit-Position: refs/heads/master@{#12404}

webrtc/common_video/i420_buffer_pool.cc

@@ -29,10 +29,11 @@ class PooledI420Buffer : public webrtc::VideoFrameBuffer {
   const uint8_t* data(webrtc::PlaneType type) const override {
     return buffer_->data(type);
   }
+  bool IsMutable() { return HasOneRef(); }
   uint8_t* MutableData(webrtc::PlaneType type) override {
     // Make the HasOneRef() check here instead of in |buffer_|, because the pool
     // also has a reference to |buffer_|.
-    RTC_DCHECK(HasOneRef());
+    RTC_DCHECK(IsMutable());
     return const_cast<uint8_t*>(buffer_->data(type));
   }
   int stride(webrtc::PlaneType type) const override {
@@ -80,7 +81,7 @@ rtc::scoped_refptr<VideoFrameBuffer> I420BufferPool::CreateBuffer(int width,
     // CreateBuffer that has not been released yet. If the ref count is 1
     // (HasOneRef), then the list we are looping over holds the only reference
     // and it's safe to reuse.
-    if (buffer->HasOneRef())
+    if (buffer->IsMutable())
       return new rtc::RefCountedObject<PooledI420Buffer>(buffer);
   }
   // Allocate new buffer.

webrtc/common_video/i420_buffer_pool_unittest.cc

@@ -55,7 +55,7 @@ TEST(TestI420BufferPool, ExclusiveOwner) {
   // Check that created buffers are exclusive so that they can be written to.
   I420BufferPool pool;
   rtc::scoped_refptr<VideoFrameBuffer> buffer = pool.CreateBuffer(16, 16);
-  EXPECT_TRUE(buffer->HasOneRef());
+  EXPECT_TRUE(buffer->IsMutable());
 }
 
 TEST(TestI420BufferPool, FrameValidAfterPoolDestruction) {
@@ -64,7 +64,7 @@ TEST(TestI420BufferPool, FrameValidAfterPoolDestruction) {
     I420BufferPool pool;
     buffer = pool.CreateBuffer(16, 16);
   }
-  EXPECT_TRUE(buffer->HasOneRef());
+  EXPECT_TRUE(buffer->IsMutable());
   EXPECT_EQ(16, buffer->width());
   EXPECT_EQ(16, buffer->height());
   // Try to trigger use-after-free errors by writing to y-plane.

webrtc/common_video/include/video_frame_buffer.h

@@ -33,7 +33,16 @@ enum PlaneType {
 // not contain any frame metadata such as rotation, timestamp, pixel_width, etc.
 class VideoFrameBuffer : public rtc::RefCountInterface {
  public:
-  // Returns true if this buffer has a single exclusive owner.
+  // Returns true if the caller is exclusive owner, and allowed to
+  // call MutableData.
+
+  // TODO(nisse): Delete default implementation when subclasses in
+  // Chrome are updated.
+  virtual bool IsMutable() { return false; }
+
+  // Underlying refcount access, used to implement IsMutable.
+  // TODO(nisse): Demote to protected, as soon as Chrome is changed to
+  // use IsMutable.
   virtual bool HasOneRef() const = 0;
 
   // The resolution of the frame in pixels. For formats where some planes are
@@ -76,6 +85,7 @@ class I420Buffer : public VideoFrameBuffer {
   const uint8_t* data(PlaneType type) const override;
   // Non-const data access is only allowed if HasOneRef() is true to protect
   // against unexpected overwrites.
+  bool IsMutable() override;
   uint8_t* MutableData(PlaneType type) override;
   int stride(PlaneType type) const override;
   void* native_handle() const override;
@@ -110,6 +120,7 @@ class NativeHandleBuffer : public VideoFrameBuffer {
   const uint8_t* data(PlaneType type) const override;
   int stride(PlaneType type) const override;
   void* native_handle() const override;
+  bool IsMutable() override;
 
  protected:
   void* native_handle_;
@@ -131,6 +142,8 @@ class WrappedI420Buffer : public webrtc::VideoFrameBuffer {
   int width() const override;
   int height() const override;
 
+  bool IsMutable() override;
+
   const uint8_t* data(PlaneType type) const override;
 
   int stride(PlaneType type) const override;

webrtc/common_video/video_frame.cc

@@ -65,7 +65,7 @@ void VideoFrame::CreateEmptyFrame(int width,
   rotation_ = kVideoRotation_0;
 
   // Check if it's safe to reuse allocation.
-  if (video_frame_buffer_ && video_frame_buffer_->HasOneRef() &&
+  if (video_frame_buffer_ && video_frame_buffer_->IsMutable() &&
       !video_frame_buffer_->native_handle() &&
       width == video_frame_buffer_->width() &&
       height == video_frame_buffer_->height() && stride_y == stride(kYPlane) &&

webrtc/common_video/video_frame_buffer.cc

@@ -89,8 +89,12 @@ const uint8_t* I420Buffer::data(PlaneType type) const {
   }
 }
 
+bool I420Buffer::IsMutable() {
+  return HasOneRef();
+}
+
 uint8_t* I420Buffer::MutableData(PlaneType type) {
-  RTC_DCHECK(HasOneRef());
+  RTC_DCHECK(IsMutable());
   return const_cast<uint8_t*>(
       static_cast<const VideoFrameBuffer*>(this)->data(type));
 }
@@ -144,6 +148,10 @@ NativeHandleBuffer::NativeHandleBuffer(void* native_handle,
   RTC_DCHECK_GT(height, 0);
 }
 
+bool NativeHandleBuffer::IsMutable() {
+  return false;
+}
+
 int NativeHandleBuffer::width() const {
   return width_;
 }
@@ -190,6 +198,11 @@ WrappedI420Buffer::~WrappedI420Buffer() {
   no_longer_used_cb_();
 }
 
+// Data owned by creator; never mutable.
+bool WrappedI420Buffer::IsMutable() {
+  return false;
+}
+
 int WrappedI420Buffer::width() const {
   return width_;
 }

webrtc/media/engine/webrtcvideoframe.cc

@@ -119,7 +119,7 @@ int32_t WebRtcVideoFrame::GetVPitch() const {
 }
 
 bool WebRtcVideoFrame::IsExclusive() const {
-  return video_frame_buffer_->HasOneRef();
+  return video_frame_buffer_->IsMutable();
 }
 
 void* WebRtcVideoFrame::GetNativeHandle() const {