Commit 5054f846 authored by Eugene Zemtsov's avatar Eugene Zemtsov Committed by Commit Bot

[webcodecs] Prevent premature destructions of VideoEncoder instances

Before this change instances with outstanding flush() promises might
have been collected and destroyed by JS GC.

Bug: 1148159, 1146170
Change-Id: I4a49deab447ad79e4d2c4022968af6d5e6d4b5d5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2546170
Commit-Queue: Eugene Zemtsov <eugene@chromium.org>
Reviewed-by: default avatarDan Sanders <sandersd@chromium.org>
Cr-Commit-Position: refs/heads/master@{#829027}
parent 3a747541
......@@ -458,8 +458,7 @@ ScriptPromise VideoEncoder::flush(ExceptionState& exception_state) {
return ScriptPromise();
Request* request = MakeGarbageCollected<Request>();
request->resolver =
MakeGarbageCollected<ScriptPromiseResolver>(script_state_);
request->resolver = MakePromise();
request->reset_count = reset_count_;
request->type = Request::Type::kFlush;
EnqueueRequest(request);
......@@ -471,9 +470,8 @@ void VideoEncoder::reset(ExceptionState& exception_state) {
if (ThrowIfCodecStateClosed(state_, "reset", exception_state))
return;
ResetInternal();
state_ = V8CodecState(V8CodecState::Enum::kUnconfigured);
ResetInternal();
}
void VideoEncoder::ResetInternal() {
......@@ -482,15 +480,36 @@ void VideoEncoder::ResetInternal() {
while (!requests_.empty()) {
Request* pending_req = requests_.TakeFirst();
DCHECK(pending_req);
if (pending_req->resolver) {
auto* ex = MakeGarbageCollected<DOMException>(
DOMExceptionCode::kOperationError, "reset() was called.");
pending_req->resolver.Release()->Reject(ex);
}
RejectPromise(pending_req);
}
stall_request_processing_ = false;
}
ScriptPromiseResolver* VideoEncoder::MakePromise() {
outstanding_promises_++;
return MakeGarbageCollected<ScriptPromiseResolver>(script_state_);
}
void VideoEncoder::ResolvePromise(Request* req) {
if (!req || !req->resolver)
return;
req->resolver.Release()->Resolve();
DCHECK_GT(outstanding_promises_, 0u);
outstanding_promises_--;
}
void VideoEncoder::RejectPromise(Request* req, DOMException* ex) {
if (!req || !req->resolver)
return;
auto* resolver = req->resolver.Release();
if (ex)
resolver->Reject(ex);
else
resolver->Reject();
DCHECK_GT(outstanding_promises_, 0u);
outstanding_promises_--;
}
void VideoEncoder::HandleError(DOMException* ex) {
// Save a temp before we clear the callback.
V8WebCodecsErrorCallback* error_callback = error_callback_.Get();
......@@ -636,18 +655,22 @@ void VideoEncoder::ProcessFlush(Request* request) {
auto done_callback = [](VideoEncoder* self, Request* req,
media::Status status) {
if (!self || self->reset_count_ != req->reset_count)
if (!self)
return;
DCHECK(req->resolver);
DCHECK_CALLED_ON_VALID_SEQUENCE(self->sequence_checker_);
if (self->reset_count_ != req->reset_count) {
self->RejectPromise(req);
return;
}
DCHECK(req->resolver);
if (status.is_ok()) {
req->resolver.Release()->Resolve();
self->ResolvePromise(req);
} else {
std::string error_msg = "Flushing error.";
self->HandleError(error_msg, status);
auto* ex = MakeGarbageCollected<DOMException>(
DOMExceptionCode::kOperationError, error_msg.c_str());
req->resolver.Release()->Reject(ex);
self->RejectPromise(req, ex);
}
self->stall_request_processing_ = false;
self->ProcessRequests();
......@@ -698,6 +721,10 @@ void VideoEncoder::ContextDestroyed() {
parent_media_log_ = nullptr;
}
bool VideoEncoder::HasPendingActivity() const {
return outstanding_promises_ > 0;
}
void VideoEncoder::Trace(Visitor* visitor) const {
visitor->Trace(script_state_);
visitor->Trace(output_callback_);
......
......@@ -13,6 +13,7 @@
#include "media/base/video_codecs.h"
#include "media/base/video_color_space.h"
#include "media/base/video_encoder.h"
#include "third_party/blink/renderer/bindings/core/v8/active_script_wrappable.h"
#include "third_party/blink/renderer/bindings/core/v8/script_promise_resolver.h"
#include "third_party/blink/renderer/bindings/modules/v8/v8_codec_state.h"
#include "third_party/blink/renderer/bindings/modules/v8/v8_video_encoder_output_callback.h"
......@@ -38,6 +39,7 @@ class Visitor;
class MODULES_EXPORT VideoEncoder final
: public ScriptWrappable,
public ActiveScriptWrappable<VideoEncoder>,
public ExecutionContextLifecycleObserver {
DEFINE_WRAPPERTYPEINFO();
......@@ -68,6 +70,9 @@ class MODULES_EXPORT VideoEncoder final
// ExecutionContextLifecycleObserver override.
void ContextDestroyed() override;
// ScriptWrappable override.
bool HasPendingActivity() const override;
// GarbageCollected override.
void Trace(Visitor*) const override;
......@@ -119,6 +124,9 @@ class MODULES_EXPORT VideoEncoder final
void UpdateEncoderLog(std::string encoder_name, bool is_hw_accelerated);
void ResetInternal();
ScriptPromiseResolver* MakePromise();
void ResolvePromise(Request* req);
void RejectPromise(Request* req, DOMException* ex = nullptr);
std::unique_ptr<ParsedConfig> ParseConfig(const VideoEncoderConfig*,
ExceptionState&);
......@@ -151,6 +159,9 @@ class MODULES_EXPORT VideoEncoder final
// an operation and its callback.
uint32_t reset_count_ = 0;
// Number of not resolved/rejected promises created by this VideoEncoder.
uint32_t outstanding_promises_ = 0;
// Some kConfigure and kFlush requests can't be executed in parallel with
// kEncode. This flag stops processing of new requests in the requests_ queue
// till the current requests is finished.
......
......@@ -6,7 +6,8 @@
[
Exposed=Window,
RuntimeEnabled=WebCodecs
RuntimeEnabled=WebCodecs,
ActiveScriptWrappable
] interface VideoEncoder {
[CallWith=ScriptState, RaisesException, MeasureAs=WebCodecsVideoEncoder]
constructor(VideoEncoderInit init);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment