Commit cf43a3b2 authored by sigbjornf's avatar sigbjornf Committed by Commit bot

Signal no pending activity in destructed contexts.

Various hasPendingActivity() overrides weren't taking the state of the
ExecutionContext into account, only considering if event listeners were
registered. We're not interested in holding onto a script environment
after an execution context has been destroyed, so adjust the predicates
to return false if the ExecutionContext has been destructed.

The V8GCController wrapper visitors already check if hasPendingActivity()
implementations incorrectly return |true| when used inside of destroyed
ExecutionContexts, but that check is not handled by trace wrappers
(ActiveScriptWrappable.)

R=
BUG=

Review-Url: https://codereview.chromium.org/2571193002
Cr-Commit-Position: refs/heads/master@{#438787}
parent 9b57da68
...@@ -652,8 +652,7 @@ bool FontFace::hadBlankText() const { ...@@ -652,8 +652,7 @@ bool FontFace::hadBlankText() const {
} }
bool FontFace::hasPendingActivity() const { bool FontFace::hasPendingActivity() const {
return m_status == Loading && getExecutionContext() && return m_status == Loading && getExecutionContext();
!getExecutionContext()->isContextDestroyed();
} }
} // namespace blink } // namespace blink
...@@ -83,7 +83,8 @@ void MediaQueryList::removeListener(MediaQueryListListener* listener) { ...@@ -83,7 +83,8 @@ void MediaQueryList::removeListener(MediaQueryListListener* listener) {
} }
bool MediaQueryList::hasPendingActivity() const { bool MediaQueryList::hasPendingActivity() const {
return m_listeners.size() || hasEventListeners(EventTypeNames::change); return getExecutionContext() &&
(m_listeners.size() || hasEventListeners(EventTypeNames::change));
} }
void MediaQueryList::contextDestroyed() { void MediaQueryList::contextDestroyed() {
......
...@@ -109,14 +109,14 @@ void BatteryManager::resume() { ...@@ -109,14 +109,14 @@ void BatteryManager::resume() {
void BatteryManager::contextDestroyed() { void BatteryManager::contextDestroyed() {
m_hasEventListener = false; m_hasEventListener = false;
m_batteryProperty.clear(); m_batteryProperty = nullptr;
stopUpdating(); stopUpdating();
} }
bool BatteryManager::hasPendingActivity() const { bool BatteryManager::hasPendingActivity() const {
// Prevent V8 from garbage collecting the wrapper object if there are // Prevent V8 from garbage collecting the wrapper object if there are
// event listeners attached to it. // event listeners attached to it.
return hasEventListeners(); return getExecutionContext() && hasEventListeners();
} }
DEFINE_TRACE(BatteryManager) { DEFINE_TRACE(BatteryManager) {
......
...@@ -87,7 +87,7 @@ ExecutionContext* ImageCapture::getExecutionContext() const { ...@@ -87,7 +87,7 @@ ExecutionContext* ImageCapture::getExecutionContext() const {
} }
bool ImageCapture::hasPendingActivity() const { bool ImageCapture::hasPendingActivity() const {
return hasEventListeners(); return getExecutionContext() && hasEventListeners();
} }
void ImageCapture::contextDestroyed() { void ImageCapture::contextDestroyed() {
......
...@@ -558,7 +558,7 @@ bool IDBDatabase::hasPendingActivity() const { ...@@ -558,7 +558,7 @@ bool IDBDatabase::hasPendingActivity() const {
// The script wrapper must not be collected before the object is closed or // The script wrapper must not be collected before the object is closed or
// we can't fire a "versionchange" event to let script manually close the // we can't fire a "versionchange" event to let script manually close the
// connection. // connection.
return !m_closePending && hasEventListeners() && getExecutionContext(); return !m_closePending && getExecutionContext() && hasEventListeners();
} }
void IDBDatabase::contextDestroyed() { void IDBDatabase::contextDestroyed() {
......
...@@ -101,12 +101,9 @@ void PresentationRequest::addedEventListener( ...@@ -101,12 +101,9 @@ void PresentationRequest::addedEventListener(
} }
bool PresentationRequest::hasPendingActivity() const { bool PresentationRequest::hasPendingActivity() const {
if (!getExecutionContext() || getExecutionContext()->isContextDestroyed())
return false;
// Prevents garbage collecting of this object when not hold by another // Prevents garbage collecting of this object when not hold by another
// object but still has listeners registered. // object but still has listeners registered.
return hasEventListeners(); return getExecutionContext() && hasEventListeners();
} }
ScriptPromise PresentationRequest::start(ScriptState* scriptState) { ScriptPromise PresentationRequest::start(ScriptState* scriptState) {
......
...@@ -132,7 +132,7 @@ bool Sensor::hasPendingActivity() const { ...@@ -132,7 +132,7 @@ bool Sensor::hasPendingActivity() const {
if (m_state == Sensor::SensorState::Idle || if (m_state == Sensor::SensorState::Idle ||
m_state == Sensor::SensorState::Errored) m_state == Sensor::SensorState::Errored)
return false; return false;
return hasEventListeners(); return getExecutionContext() && hasEventListeners();
} }
auto Sensor::createSensorConfig() -> SensorConfigurationPtr { auto Sensor::createSensorConfig() -> SensorConfigurationPtr {
......
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