ChromeOS media: remove unnecessary FD duplication.
The recent refactoring CL crrev.com/c/1613297 introduced
unnecessary duplication of file descriptors. Worse, those
descriptors could leak, eventually leading to resource
exhaustion. As an added bonus, several legacy uses of
SharedMemory and SharedMemoryHandle are updated to the new
API.
See crbug.com/965455 for a motivating example of leaky
file descriptors.
(cherry picked from commit 842e2a8cb87738c74ca6f95c7173961d0bf5bfe4)
Bug: 849207, 971575
Change-Id: I8b6c06689d99a12c97af19bd3eabdb0918b1ba22
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1645301
Commit-Queue: Matthew Cary (CET) <mattcary@chromium.org>
Reviewed-by: Wei Lee <wtlee@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#666626}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1657841
Reviewed-by: Matthew Cary (CET) <mattcary@chromium.org>
Cr-Commit-Position: refs/branch-heads/3809@{#287}
Cr-Branched-From: d82dec1a818f378c464ba307ddd9c92133eac355-refs/heads/master@{#665002}
diff --git a/components/chromeos_camera/jpeg_encode_accelerator_unittest.cc b/components/chromeos_camera/jpeg_encode_accelerator_unittest.cc
index e3b5e10..24ad2b01 100644
--- a/components/chromeos_camera/jpeg_encode_accelerator_unittest.cc
+++ b/components/chromeos_camera/jpeg_encode_accelerator_unittest.cc
@@ -587,10 +587,9 @@
encoder_->GetMaxCodedBufferSize(test_image->visible_size);
PrepareMemory(bitstream_buffer_id);
- base::SharedMemoryHandle dup_handle =
- base::SharedMemoryHandle(hw_out_shm_->handle());
+ // media::BitstreamBuffer will duplicate hw_out_shm_->handle().
encoded_buffer_ =
- media::BitstreamBuffer(bitstream_buffer_id, dup_handle,
+ media::BitstreamBuffer(bitstream_buffer_id, hw_out_shm_->handle(),
false /* read_only */, test_image->output_size);
scoped_refptr<media::VideoFrame> input_frame_ =
media::VideoFrame::WrapExternalSharedMemory(
diff --git a/components/chromeos_camera/mojo_jpeg_encode_accelerator_service.cc b/components/chromeos_camera/mojo_jpeg_encode_accelerator_service.cc
index 4324d5c..bcad935 100644
--- a/components/chromeos_camera/mojo_jpeg_encode_accelerator_service.cc
+++ b/components/chromeos_camera/mojo_jpeg_encode_accelerator_service.cc
@@ -199,21 +199,28 @@
}
base::UnguessableToken input_guid = base::UnguessableToken::Create();
- base::UnguessableToken exif_guid = base::UnguessableToken::Create();
- base::UnguessableToken output_guid = base::UnguessableToken::Create();
base::SharedMemoryHandle input_shm_handle(
base::FileDescriptor(input_fd, true), input_buffer_size, input_guid);
- base::SharedMemoryHandle exif_shm_handle(base::FileDescriptor(exif_fd, true),
- exif_buffer_size, exif_guid);
- base::SharedMemoryHandle output_shm_handle(
- base::FileDescriptor(output_fd, true), output_buffer_size, output_guid);
- media::BitstreamBuffer output_buffer(
- task_id, output_shm_handle, false /* read_only */, output_buffer_size);
+ base::subtle::PlatformSharedMemoryRegion output_shm_region =
+ base::subtle::PlatformSharedMemoryRegion::Take(
+ base::subtle::ScopedFDPair(base::ScopedFD(output_fd),
+ base::ScopedFD()),
+ base::subtle::PlatformSharedMemoryRegion::Mode::kUnsafe,
+ output_buffer_size, base::UnguessableToken::Create());
+
+ media::BitstreamBuffer output_buffer(task_id, std::move(output_shm_region),
+ output_buffer_size);
std::unique_ptr<media::BitstreamBuffer> exif_buffer;
if (exif_buffer_size > 0) {
+ base::subtle::PlatformSharedMemoryRegion exif_shm_region =
+ base::subtle::PlatformSharedMemoryRegion::Take(
+ base::subtle::ScopedFDPair(base::ScopedFD(exif_fd),
+ base::ScopedFD()),
+ base::subtle::PlatformSharedMemoryRegion::Mode::kUnsafe,
+ exif_buffer_size, base::UnguessableToken::Create());
exif_buffer = std::make_unique<media::BitstreamBuffer>(
- task_id, exif_shm_handle, false /* read_only */, exif_buffer_size);
+ task_id, std::move(exif_shm_region), exif_buffer_size);
}
gfx::Size coded_size(coded_size_width, coded_size_height);
@@ -314,15 +321,18 @@
0, ::chromeos_camera::JpegEncodeAccelerator::Status::PLATFORM_FAILURE);
return;
}
- base::UnguessableToken exif_guid = base::UnguessableToken::Create();
- base::SharedMemoryHandle exif_shm_handle(base::FileDescriptor(exif_fd, true),
- exif_buffer_size, exif_guid);
std::unique_ptr<media::BitstreamBuffer> exif_buffer;
if (exif_buffer_size > 0) {
// Currently we use our zero-based |task_id| as id of |exif_buffer| to track
// the encode task process from both Chrome OS and Chrome side.
+ base::subtle::PlatformSharedMemoryRegion exif_shm_region =
+ base::subtle::PlatformSharedMemoryRegion::Take(
+ base::subtle::ScopedFDPair(base::ScopedFD(exif_fd),
+ base::ScopedFD()),
+ base::subtle::PlatformSharedMemoryRegion::Mode::kUnsafe,
+ exif_buffer_size, base::UnguessableToken::Create());
exif_buffer = std::make_unique<media::BitstreamBuffer>(
- task_id, exif_shm_handle, false /* read_only */, exif_buffer_size);
+ task_id, std::move(exif_shm_region), exif_buffer_size);
}
encode_cb_map_.emplace(task_id, std::move(callback));
diff --git a/components/chromeos_camera/mojo_mjpeg_decode_accelerator_service.cc b/components/chromeos_camera/mojo_mjpeg_decode_accelerator_service.cc
index 93ed4ba..1e33ee4 100644
--- a/components/chromeos_camera/mojo_mjpeg_decode_accelerator_service.cc
+++ b/components/chromeos_camera/mojo_mjpeg_decode_accelerator_service.cc
@@ -208,16 +208,19 @@
return;
}
- base::UnguessableToken guid = base::UnguessableToken::Create();
- base::SharedMemoryHandle input_shm_handle(
- base::FileDescriptor(input_fd, true), input_buffer_size, guid);
- base::SharedMemoryHandle output_shm_handle(
- base::FileDescriptor(output_fd, true), output_buffer_size, guid);
-
- media::BitstreamBuffer in_buffer(buffer_id, input_shm_handle,
- false /* read_only */, input_buffer_size);
+ base::subtle::PlatformSharedMemoryRegion input_shm_region =
+ base::subtle::PlatformSharedMemoryRegion::Take(
+ base::subtle::ScopedFDPair(base::ScopedFD(input_fd),
+ base::ScopedFD()),
+ base::subtle::PlatformSharedMemoryRegion::Mode::kUnsafe,
+ input_buffer_size, base::UnguessableToken::Create());
+ media::BitstreamBuffer in_buffer(buffer_id, std::move(input_shm_region),
+ input_buffer_size);
gfx::Size coded_size(coded_size_width, coded_size_height);
+ base::SharedMemoryHandle output_shm_handle(
+ base::FileDescriptor(output_fd, true), output_buffer_size,
+ base::UnguessableToken::Create());
mojo::ScopedSharedBufferHandle output_scoped_handle =
mojo::WrapSharedMemoryHandle(
output_shm_handle, output_buffer_size,
diff --git a/components/chromeos_camera/mojo_mjpeg_decode_accelerator_service_unittest.cc b/components/chromeos_camera/mojo_mjpeg_decode_accelerator_service_unittest.cc
index 7d5324e..ca2e74f 100644
--- a/components/chromeos_camera/mojo_mjpeg_decode_accelerator_service_unittest.cc
+++ b/components/chromeos_camera/mojo_mjpeg_decode_accelerator_service_unittest.cc
@@ -6,6 +6,7 @@
#include "base/bind.h"
#include "base/command_line.h"
+#include "base/memory/platform_shared_memory_region.h"
#include "base/run_loop.h"
#include "base/test/scoped_task_environment.h"
#include "base/threading/thread.h"
@@ -68,16 +69,16 @@
subsamples.push_back(media::SubsampleEntry(15, 7));
base::RunLoop run_loop2;
- base::SharedMemory shm;
- ASSERT_TRUE(shm.CreateAndMapAnonymous(kInputBufferSizeInBytes));
+ base::subtle::PlatformSharedMemoryRegion shm_region =
+ base::subtle::PlatformSharedMemoryRegion::CreateUnsafe(
+ kInputBufferSizeInBytes);
mojo::ScopedSharedBufferHandle output_frame_handle =
mojo::SharedBufferHandle::Create(kOutputFrameSizeInBytes);
- media::BitstreamBuffer bitstream_buffer(
- kArbitraryBitstreamBufferId,
- base::SharedMemory::DuplicateHandle(shm.handle()), false /* read_only */,
- kInputBufferSizeInBytes);
+ media::BitstreamBuffer bitstream_buffer(kArbitraryBitstreamBufferId,
+ std::move(shm_region),
+ kInputBufferSizeInBytes);
bitstream_buffer.SetDecryptionSettings(kKeyId, kIv, subsamples);
jpeg_decoder->Decode(