Commit 7028377e authored by Guido Urdaneta's avatar Guido Urdaneta Committed by Commit Bot

[MediaStreams] Check the context state in MediaStreamTrack::Ended()

Prior to this CL, the state of the execution context (destroyed or not)
was stored in a boolean field updated when the execution context was
destroyed.

After UserMediaClient/UserMediaProcessor was moved to Blink, the timing
for the stopping of sources changed slightly. Instead of clearing state
upon starting a new navigation, it is now cleared when the execution
context is destroyed.

If the dying context notifies UMC/UMP before notifying the
MediaStreamTrack about the destruction it can happen that
MediaStreamTrack tries to fire an event on the destroyed context
without knowing that the context has already been destroyed.

To prevent this from happening, instead of keeping the state of the
context in a separate variable updated via ContextLifecycleObserver,
we now track the context using a WeakMember.

Bug: 996051
Change-Id: I0027c267a0d067ba67ffcf8f508c1bc56f36a72a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1768460
Commit-Queue: Guido Urdaneta <guidou@chromium.org>
Reviewed-by: default avatarMarina Ciocea <marinaciocea@chromium.org>
Cr-Commit-Position: refs/heads/master@{#690349}
parent 8a73187b
......@@ -218,17 +218,14 @@ MediaStreamTrack::MediaStreamTrack(ExecutionContext* context,
MediaStreamComponent* component)
: MediaStreamTrack(context,
component,
component->Source()->GetReadyState(),
false /* stopped */) {}
component->Source()->GetReadyState()) {}
MediaStreamTrack::MediaStreamTrack(ExecutionContext* context,
MediaStreamComponent* component,
MediaStreamSource::ReadyState ready_state,
bool stopped)
: ContextLifecycleObserver(context),
ready_state_(ready_state),
stopped_(stopped),
component_(component) {
MediaStreamSource::ReadyState ready_state)
: ready_state_(ready_state),
component_(component),
execution_context_(context) {
component_->Source()->AddObserver(this);
// If the source is already non-live at this point, the observer won't have
......@@ -380,8 +377,7 @@ void MediaStreamTrack::stopTrack(ExecutionContext* execution_context) {
MediaStreamTrack* MediaStreamTrack::clone(ScriptState* script_state) {
MediaStreamComponent* cloned_component = Component()->Clone();
MediaStreamTrack* cloned_track = MakeGarbageCollected<MediaStreamTrack>(
ExecutionContext::From(script_state), cloned_component, ready_state_,
stopped_);
ExecutionContext::From(script_state), cloned_component, ready_state_);
DidCloneMediaStreamTrack(Component(), cloned_component);
return cloned_track;
}
......@@ -705,7 +701,8 @@ void MediaStreamTrack::applyConstraintsImageCapture(
}
bool MediaStreamTrack::Ended() const {
return stopped_ || (ready_state_ == MediaStreamSource::kReadyStateEnded);
return (execution_context_ && execution_context_->IsContextDestroyed()) ||
(ready_state_ == MediaStreamSource::kReadyStateEnded);
}
void MediaStreamTrack::SourceChangedState() {
......@@ -739,10 +736,6 @@ void MediaStreamTrack::PropagateTrackEnded() {
is_iterating_registered_media_streams_ = false;
}
void MediaStreamTrack::ContextDestroyed(ExecutionContext*) {
stopped_ = true;
}
bool MediaStreamTrack::HasPendingActivity() const {
// If 'ended' listeners exist and the object hasn't yet reached
// that state, keep the object alive.
......@@ -785,15 +778,15 @@ const AtomicString& MediaStreamTrack::InterfaceName() const {
}
ExecutionContext* MediaStreamTrack::GetExecutionContext() const {
return ContextLifecycleObserver::GetExecutionContext();
return execution_context_.Get();
}
void MediaStreamTrack::Trace(blink::Visitor* visitor) {
visitor->Trace(registered_media_streams_);
visitor->Trace(component_);
visitor->Trace(image_capture_);
visitor->Trace(execution_context_);
EventTargetWithInlineData::Trace(visitor);
ContextLifecycleObserver::Trace(visitor);
}
} // namespace blink
......@@ -29,7 +29,6 @@
#include <memory>
#include "third_party/blink/renderer/bindings/core/v8/active_script_wrappable.h"
#include "third_party/blink/renderer/bindings/core/v8/script_promise.h"
#include "third_party/blink/renderer/core/execution_context/context_lifecycle_observer.h"
#include "third_party/blink/renderer/modules/event_target_modules.h"
#include "third_party/blink/renderer/modules/modules_export.h"
#include "third_party/blink/renderer/platform/mediastream/media_stream_descriptor.h"
......@@ -50,7 +49,6 @@ class ScriptState;
class MODULES_EXPORT MediaStreamTrack
: public EventTargetWithInlineData,
public ActiveScriptWrappable<MediaStreamTrack>,
public ContextLifecycleObserver,
public MediaStreamSource::Observer {
USING_GARBAGE_COLLECTED_MIXIN(MediaStreamTrack);
DEFINE_WRAPPERTYPEINFO();
......@@ -61,8 +59,7 @@ class MODULES_EXPORT MediaStreamTrack
MediaStreamTrack(ExecutionContext*, MediaStreamComponent*);
MediaStreamTrack(ExecutionContext*,
MediaStreamComponent*,
MediaStreamSource::ReadyState,
bool stopped);
MediaStreamSource::ReadyState);
~MediaStreamTrack() override;
String kind() const;
......@@ -108,9 +105,6 @@ class MODULES_EXPORT MediaStreamTrack
// ScriptWrappable
bool HasPendingActivity() const final;
// ContextLifecycleObserver
void ContextDestroyed(ExecutionContext*) override;
std::unique_ptr<AudioSourceProvider> CreateWebAudioSource(
int context_sample_rate);
......@@ -129,9 +123,9 @@ class MODULES_EXPORT MediaStreamTrack
MediaStreamSource::ReadyState ready_state_;
HeapHashSet<Member<MediaStream>> registered_media_streams_;
bool is_iterating_registered_media_streams_ = false;
bool stopped_;
Member<MediaStreamComponent> component_;
Member<ImageCapture> image_capture_;
WeakMember<ExecutionContext> execution_context_;
};
typedef HeapVector<Member<MediaStreamTrack>> MediaStreamTrackVector;
......
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