Commit 11bd5034 authored by sigbjornf's avatar sigbjornf Committed by Commit bot

Eagerly dispose of ScheduledActions.

The DOMTimer's ScheduledAction hold on to the script source and
state needed to execute the timer action. Let go of ShceduledAction's
resource early.

Apart from reducing the lifetime of script source, this is a speculative
fix for crashes reported in v8::PersistentValueVector::Clear() during
lazy sweeping of ScheduledAction objects.

R=
BUG=

Review-Url: https://codereview.chromium.org/2552673002
Cr-Commit-Position: refs/heads/master@{#436298}
parent d43cbc46
......@@ -61,7 +61,17 @@ DEFINE_TRACE(ScheduledAction) {
visitor->trace(m_code);
}
ScheduledAction::~ScheduledAction() {}
ScheduledAction::~ScheduledAction() {
// Verify that owning DOMTimer has eagerly disposed.
DCHECK(m_info.IsEmpty());
}
void ScheduledAction::dispose() {
m_code.dispose();
m_info.Clear();
m_function.clear();
m_scriptState.clear();
}
void ScheduledAction::execute(ExecutionContext* context) {
if (context->isDocument()) {
......
......@@ -56,6 +56,8 @@ class ScheduledAction final
static ScheduledAction* create(ScriptState*, const String& handler);
~ScheduledAction();
void dispose();
DECLARE_TRACE();
void execute(ExecutionContext*);
......
......@@ -36,6 +36,13 @@ ScriptSourceCode::ScriptSourceCode(ScriptStreamer* streamer,
ScriptSourceCode::~ScriptSourceCode() {}
void ScriptSourceCode::dispose() {
m_source = String();
m_resource = nullptr;
m_streamer = nullptr;
m_url = KURL();
}
DEFINE_TRACE(ScriptSourceCode) {
visitor->trace(m_resource);
visitor->trace(m_streamer);
......
......@@ -56,6 +56,7 @@ class CORE_EXPORT ScriptSourceCode final {
ScriptSourceCode(ScriptStreamer*, ScriptResource*);
~ScriptSourceCode();
void dispose();
DECLARE_TRACE();
bool isEmpty() const { return m_source.isEmpty(); }
......@@ -65,13 +66,13 @@ class CORE_EXPORT ScriptSourceCode final {
bool isNull() const { return m_source.isNull(); }
const String& source() const { return m_source; }
ScriptResource* resource() const { return m_resource.get(); }
ScriptResource* resource() const { return m_resource; }
const KURL& url() const;
int startLine() const { return m_startPosition.m_line.oneBasedInt(); }
const TextPosition& startPosition() const { return m_startPosition; }
String sourceMapUrl() const;
ScriptStreamer* streamer() const { return m_streamer.get(); }
ScriptStreamer* streamer() const { return m_streamer; }
private:
void treatNullSourceAsEmpty();
......
......@@ -107,7 +107,10 @@ DOMTimer::DOMTimer(ExecutionContext* context,
startRepeating(intervalMilliseconds, BLINK_FROM_HERE);
}
DOMTimer::~DOMTimer() {}
DOMTimer::~DOMTimer() {
if (m_action)
m_action->dispose();
}
void DOMTimer::stop() {
InspectorInstrumentation::asyncTaskCanceled(getExecutionContext(), this);
......@@ -115,6 +118,8 @@ void DOMTimer::stop() {
// Need to release JS objects potentially protected by ScheduledAction
// because they can form circular references back to the ExecutionContext
// which will cause a memory leak.
if (m_action)
m_action->dispose();
m_action = nullptr;
SuspendableTimer::stop();
}
......@@ -172,6 +177,8 @@ void DOMTimer::fired() {
executionContext->timers()->setTimerNestingLevel(0);
// Eagerly unregister as ExecutionContext observer.
clearContext();
// Eagerly clear out |action|'s resources.
action->dispose();
}
WebTaskRunner* DOMTimer::timerTaskRunner() const {
......
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