Commit 629360d2 authored by Hongchan Choi's avatar Hongchan Choi Committed by Commit Bot

Check WorkletPendingTasks when BaseAudioContext is going away

This is a speculative fix for the bug associated. The following is the
scenario of potential NULL-deference:

1. BaseAudioContext.audioWorklet.addModule('some.url');
2. BaseAudioContext is destroyed.
3. The callback originated from the module loading arrives, but the
   associated context is already gone.
4. Crash with NULL deference, because the callback touches the
   destination node of the AudioContext.

This CL fixes the issue by keeping track of WorkletPendingTasks until
module loading tasks are resolved.

Bug: 839642
Change-Id: I154bb8c56f0fa95d6708c642047c7b05acf83eb2
Reviewed-on: https://chromium-review.googlesource.com/1062795Reviewed-by: default avatarRaymond Toy <rtoy@chromium.org>
Reviewed-by: default avatarKentaro Hara <haraken@chromium.org>
Reviewed-by: default avatarHiroki Nakagawa <nhiroki@chromium.org>
Commit-Queue: Hongchan Choi <hongchan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#560290}
parent ea955df4
......@@ -29,6 +29,7 @@ Worklet::Worklet(Document* document)
Worklet::~Worklet() {
for (const auto& proxy : proxies_)
proxy->WorkletObjectDestroyed();
DCHECK(!HasPendingTasks());
}
// Implementation of the first half of the "addModule(moduleURL, options)"
......@@ -65,6 +66,9 @@ ScriptPromise Worklet::addModule(ScriptState* script_state,
return promise;
}
WorkletPendingTasks* pending_tasks = new WorkletPendingTasks(this, resolver);
pending_tasks_set_.insert(pending_tasks);
// Step 5: "Return promise, and then continue running this algorithm in
// parallel."
// |kInternalLoading| is used here because this is a part of script module
......@@ -73,7 +77,7 @@ ScriptPromise Worklet::addModule(ScriptState* script_state,
->GetTaskRunner(TaskType::kInternalLoading)
->PostTask(FROM_HERE, WTF::Bind(&Worklet::FetchAndInvokeScript,
WrapPersistent(this), module_url_record,
options, WrapPersistent(resolver)));
options, WrapPersistent(pending_tasks)));
return promise;
}
......@@ -84,6 +88,16 @@ void Worklet::ContextDestroyed(ExecutionContext* execution_context) {
proxy->TerminateWorkletGlobalScope();
}
bool Worklet::HasPendingTasks() const {
return pending_tasks_set_.size() > 0;
}
void Worklet::FinishPendingTasks(WorkletPendingTasks* pending_tasks) {
DCHECK(IsMainThread());
DCHECK(pending_tasks_set_.Contains(pending_tasks));
pending_tasks_set_.erase(pending_tasks);
}
WorkletGlobalScopeProxy* Worklet::FindAvailableGlobalScope() {
DCHECK(IsMainThread());
return proxies_.at(SelectGlobalScope());
......@@ -94,7 +108,7 @@ WorkletGlobalScopeProxy* Worklet::FindAvailableGlobalScope() {
// https://drafts.css-houdini.org/worklets/#dom-worklet-addmodule
void Worklet::FetchAndInvokeScript(const KURL& module_url_record,
const WorkletOptions& options,
ScriptPromiseResolver* resolver) {
WorkletPendingTasks* pending_tasks) {
DCHECK(IsMainThread());
if (!GetExecutionContext())
return;
......@@ -133,8 +147,7 @@ void Worklet::FetchAndInvokeScript(const KURL& module_url_record,
// Step 11: "Let pendingTaskStruct be a new pending tasks struct with counter
// initialized to the length of worklet's WorkletGlobalScopes."
WorkletPendingTasks* pending_tasks =
new WorkletPendingTasks(GetNumberOfGlobalScopes(), resolver);
pending_tasks->InitializeCounter(GetNumberOfGlobalScopes());
// Step 12: "For each workletGlobalScope in the worklet's
// WorkletGlobalScopes, queue a task on the workletGlobalScope to fetch and
......@@ -157,6 +170,7 @@ size_t Worklet::SelectGlobalScope() {
void Worklet::Trace(blink::Visitor* visitor) {
visitor->Trace(proxies_);
visitor->Trace(module_responses_map_);
visitor->Trace(pending_tasks_set_);
ScriptWrappable::Trace(visitor);
ContextLifecycleObserver::Trace(visitor);
}
......
......@@ -18,7 +18,6 @@
namespace blink {
class Document;
class ScriptPromiseResolver;
// This is the base implementation of Worklet interface defined in the spec:
// https://drafts.css-houdini.org/worklets/#worklet
......@@ -44,6 +43,13 @@ class CORE_EXPORT Worklet : public ScriptWrappable,
// ContextLifecycleObserver
void ContextDestroyed(ExecutionContext*) override;
// Returns true if there is ongoing module loading tasks. BaseAudioContext
// uses this check to keep itself alive until pending tasks are resolved.
bool HasPendingTasks() const;
// Called by WorkletPendingTasks to notify the Worklet.
void FinishPendingTasks(WorkletPendingTasks*);
void Trace(blink::Visitor*) override;
protected:
......@@ -61,7 +67,7 @@ class CORE_EXPORT Worklet : public ScriptWrappable,
private:
virtual void FetchAndInvokeScript(const KURL& module_url_record,
const WorkletOptions&,
ScriptPromiseResolver*);
WorkletPendingTasks*);
// Returns true if there are no global scopes or additional global scopes are
// necessary. CreateGlobalScope() will be called in that case. Each worklet
......@@ -87,6 +93,9 @@ class CORE_EXPORT Worklet : public ScriptWrappable,
// https://drafts.css-houdini.org/worklets/#module-responses-map
Member<WorkletModuleResponsesMap> module_responses_map_;
// Keeps track of pending tasks from addModule() call.
HeapHashSet<Member<WorkletPendingTasks>> pending_tasks_set_;
DISALLOW_COPY_AND_ASSIGN(Worklet);
};
......
......@@ -6,16 +6,22 @@
#include "third_party/blink/renderer/core/dom/dom_exception.h"
#include "third_party/blink/renderer/core/dom/exception_code.h"
#include "third_party/blink/renderer/core/workers/worklet.h"
#include "third_party/blink/renderer/platform/wtf/wtf.h"
namespace blink {
WorkletPendingTasks::WorkletPendingTasks(int counter,
WorkletPendingTasks::WorkletPendingTasks(Worklet* worklet,
ScriptPromiseResolver* resolver)
: counter_(counter), resolver_(resolver) {
: resolver_(resolver), worklet_(worklet) {
DCHECK(IsMainThread());
}
void WorkletPendingTasks::InitializeCounter(int counter) {
DCHECK(IsMainThread());
counter_ = counter;
}
void WorkletPendingTasks::Abort() {
DCHECK(IsMainThread());
// Step 3: "If script is null, then queue a task on outsideSettings's
......@@ -25,6 +31,7 @@ void WorkletPendingTasks::Abort() {
// 2: "Reject promise with an "AbortError" DOMException."
if (counter_ != -1) {
counter_ = -1;
worklet_->FinishPendingTasks(this);
resolver_->Reject(DOMException::Create(kAbortError));
}
}
......@@ -39,8 +46,14 @@ void WorkletPendingTasks::DecrementCounter() {
if (counter_ != -1) {
--counter_;
if (counter_ == 0)
worklet_->FinishPendingTasks(this);
resolver_->Resolve();
}
}
void WorkletPendingTasks::Trace(blink::Visitor* visitor) {
visitor->Trace(resolver_);
visitor->Trace(worklet_);
}
} // namespace blink
......@@ -11,6 +11,8 @@
namespace blink {
class Worklet;
// Implementation of the "pending tasks struct":
// https://drafts.css-houdini.org/worklets/#pending-tasks-struct
//
......@@ -22,7 +24,11 @@ namespace blink {
class CORE_EXPORT WorkletPendingTasks final
: public GarbageCollected<WorkletPendingTasks> {
public:
WorkletPendingTasks(int counter, ScriptPromiseResolver*);
WorkletPendingTasks(Worklet*, ScriptPromiseResolver*);
// This must be called after the construction and before decrementing the
// counter.
void InitializeCounter(int counter);
// Sets |counter_| to -1 and rejects the promise.
void Abort();
......@@ -30,7 +36,7 @@ class CORE_EXPORT WorkletPendingTasks final
// Decrements |counter_| and resolves the promise if the counter becomes 0.
void DecrementCounter();
virtual void Trace(blink::Visitor* visitor) { visitor->Trace(resolver_); }
virtual void Trace(blink::Visitor*);
private:
// The number of pending tasks. -1 indicates these tasks are aborted and
......@@ -38,6 +44,8 @@ class CORE_EXPORT WorkletPendingTasks final
int counter_;
Member<ScriptPromiseResolver> resolver_;
Member<Worklet> worklet_;
};
} // namespace blink
......
......@@ -199,6 +199,12 @@ void BaseAudioContext::ContextDestroyed(ExecutionContext*) {
}
bool BaseAudioContext::HasPendingActivity() const {
// As long as AudioWorklet has a pending task from worklet script loading,
// the BaseAudioContext needs to stay.
if (audioWorklet() && audioWorklet()->HasPendingTasks()) {
return true;
}
// There's no pending activity if the audio context has been cleared.
return !is_cleared_;
}
......
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