Commit 814f82b5 authored by Tommy Steimel's avatar Tommy Steimel Committed by Commit Bot

Fix ResizeObserver leak in TextTrackContainer

This CL adds logic to disconnect TextTrackContainer's ResizeObserver
when the container is removed from the document. This fixes an issue
where video elements were leaked since the ResizeObserver was a root
within the object graph.

TESTED=Patched into M76 and it fixed the leak for StratosMedia

Bug: 971193
Change-Id: Ie4c72ed34009a804d8f5d433c730ef146a9e42a8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1693027Reviewed-by: default avatarJeremy Roman <jbroman@chromium.org>
Reviewed-by: default avatarMounir Lamouri <mlamouri@chromium.org>
Commit-Queue: Tommy Steimel <steimel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#676448}
parent b65e2c8b
......@@ -66,21 +66,42 @@ class VideoElementResizeDelegate final : public ResizeObserver::Delegate {
} // namespace
TextTrackContainer::TextTrackContainer(Document& document)
: HTMLDivElement(document), default_font_size_(0) {}
TextTrackContainer::TextTrackContainer(HTMLMediaElement& media_element)
: TextTrackContainer(media_element.GetDocument()) {
: HTMLDivElement(media_element.GetDocument()),
media_element_(&media_element),
default_font_size_(0) {
SetShadowPseudoId(AtomicString("-webkit-media-text-track-container"));
if (IsHTMLVideoElement(media_element))
ObserveSizeChanges(media_element);
if (IsHTMLVideoElement(*media_element_))
ObserveSizeChanges(*media_element_);
}
void TextTrackContainer::Trace(Visitor* visitor) {
visitor->Trace(media_element_);
visitor->Trace(video_size_observer_);
HTMLDivElement::Trace(visitor);
}
Node::InsertionNotificationRequest TextTrackContainer::InsertedInto(
ContainerNode& root) {
if (!video_size_observer_ && media_element_->isConnected() &&
IsHTMLVideoElement(*media_element_)) {
ObserveSizeChanges(*media_element_);
}
return HTMLDivElement::InsertedInto(root);
}
void TextTrackContainer::RemovedFrom(ContainerNode& insertion_point) {
DCHECK(!media_element_->isConnected());
HTMLDivElement::RemovedFrom(insertion_point);
if (video_size_observer_) {
video_size_observer_->disconnect();
video_size_observer_.Clear();
}
}
LayoutObject* TextTrackContainer::CreateLayoutObject(const ComputedStyle&,
LegacyLayout) {
// TODO(mstensho): Should use LayoutObjectFactory to create the right type of
......
......@@ -39,9 +39,12 @@ class HTMLMediaElement;
class TextTrackContainer final : public HTMLDivElement {
public:
TextTrackContainer(Document&);
TextTrackContainer(HTMLMediaElement&);
// Node override.
Node::InsertionNotificationRequest InsertedInto(ContainerNode&) override;
void RemovedFrom(ContainerNode&) override;
// Runs the "rules for updating the text track rendering". The
// ExposingControls enum is used in the WebVTT processing model to reset the
// layout when the media controls become visible, to avoid overlapping them.
......@@ -60,6 +63,7 @@ class TextTrackContainer final : public HTMLDivElement {
LayoutObject* CreateLayoutObject(const ComputedStyle&, LegacyLayout) override;
Member<HTMLMediaElement> media_element_;
Member<ResizeObserver> video_size_observer_;
float default_font_size_;
};
......
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