Skip to content

GitLab

T
tangled
Commit ac5c34a5 authored by zqzhang's avatar zqzhang Committed by Commit bot
Browse files
Options

Fix an issue in fullscreen detector when context is destroyed 

There's a race condition causing the WebMediaPlayer to be destroyed
before MediaCustomControlsFullscreenDetector dispatches the final
change. This CL fixes the issue.

BUG=698034

Review-Url: https://codereview.chromium.org/2743573003
Cr-Commit-Position: refs/heads/master@{#456160}
parent d5fc0020
Hide whitespace changes
Inline Side-by-side
Showing with 21 additions and 18 deletions
third_party/WebKit/Source/core/html/HTMLMediaElement.h
View file @ ac5c34a5
...@@ -363,7 +363,7 @@ class CORE_EXPORT HTMLMediaElement ...@@ -363,7 +363,7 @@ class CORE_EXPORT HTMLMediaElement
bool isInteractiveContent() const final; bool isInteractiveContent() const final;
// SuspendableObject functions. // SuspendableObject functions.
void contextDestroyed(ExecutionContext*) final; void contextDestroyed(ExecutionContext*) override;
virtual void updateDisplayState() {} virtual void updateDisplayState() {}
...@@ -740,6 +740,7 @@ class CORE_EXPORT HTMLMediaElement ...@@ -740,6 +740,7 @@ class CORE_EXPORT HTMLMediaElement
friend class MediaControlsTest; friend class MediaControlsTest;
friend class HTMLMediaElementTest; friend class HTMLMediaElementTest;
friend class HTMLMediaElementEventListenersTest; friend class HTMLMediaElementEventListenersTest;
friend class HTMLVideoElement;
friend class HTMLVideoElementTest; friend class HTMLVideoElementTest;
friend class MediaControlsOrientationLockDelegateTest; friend class MediaControlsOrientationLockDelegateTest;
......
third_party/WebKit/Source/core/html/HTMLMediaElementEventListenersTest.cpp
View file @ ac5c34a5
...@@ -129,9 +129,8 @@ TEST_F(HTMLMediaElementEventListenersTest, ...@@ -129,9 +129,8 @@ TEST_F(HTMLMediaElementEventListenersTest,
EXPECT_EQ(controls(), video()->mediaControls()); EXPECT_EQ(controls(), video()->mediaControls());
} }
// crbug.com/700207
TEST_F(HTMLMediaElementEventListenersTest, TEST_F(HTMLMediaElementEventListenersTest,
DISABLED_FullscreenDetectorTimerCancelledOnContextDestroy) { FullscreenDetectorTimerCancelledOnContextDestroy) {
EXPECT_EQ(video(), nullptr); EXPECT_EQ(video(), nullptr);
document().body()->setInnerHTML("<body><video></video</body>"); document().body()->setInnerHTML("<body><video></video</body>");
video()->setSrc("http://example.com"); video()->setSrc("http://example.com");
......
third_party/WebKit/Source/core/html/HTMLVideoElement.cpp
View file @ ac5c34a5
...@@ -76,6 +76,11 @@ DEFINE_TRACE(HTMLVideoElement) { ...@@ -76,6 +76,11 @@ DEFINE_TRACE(HTMLVideoElement) {
HTMLMediaElement::trace(visitor); HTMLMediaElement::trace(visitor);
} }
void HTMLVideoElement::contextDestroyed(ExecutionContext* context) {
m_customControlsFullscreenDetector->contextDestroyed();
HTMLMediaElement::contextDestroyed(context);
}
bool HTMLVideoElement::layoutObjectIsNeeded(const ComputedStyle& style) { bool HTMLVideoElement::layoutObjectIsNeeded(const ComputedStyle& style) {
return HTMLElement::layoutObjectIsNeeded(style); return HTMLElement::layoutObjectIsNeeded(style);
} }
......
third_party/WebKit/Source/core/html/HTMLVideoElement.h
View file @ ac5c34a5
...@@ -127,6 +127,9 @@ class CORE_EXPORT HTMLVideoElement final : public HTMLMediaElement, ...@@ -127,6 +127,9 @@ class CORE_EXPORT HTMLVideoElement final : public HTMLMediaElement,
HTMLVideoElement(Document&); HTMLVideoElement(Document&);
// SuspendableObject functions.
void contextDestroyed(ExecutionContext*) final;
bool layoutObjectIsNeeded(const ComputedStyle&) override; bool layoutObjectIsNeeded(const ComputedStyle&) override;
LayoutObject* createLayoutObject(const ComputedStyle&) override; LayoutObject* createLayoutObject(const ComputedStyle&) override;
void attachLayoutTree(const AttachContext& = AttachContext()) override; void attachLayoutTree(const AttachContext& = AttachContext()) override;
......
third_party/WebKit/Source/core/html/MediaCustomControlsFullscreenDetector.cpp
View file @ ac5c34a5
...@@ -23,7 +23,6 @@ constexpr float kMostlyFillViewportThresholdOfVisibleProportion = 0.75f; ...@@ -23,7 +23,6 @@ constexpr float kMostlyFillViewportThresholdOfVisibleProportion = 0.75f;
MediaCustomControlsFullscreenDetector::MediaCustomControlsFullscreenDetector( MediaCustomControlsFullscreenDetector::MediaCustomControlsFullscreenDetector(
HTMLVideoElement& video) HTMLVideoElement& video)
: EventListener(CPPEventListenerType), : EventListener(CPPEventListenerType),
ContextLifecycleObserver(nullptr),
m_videoElement(video), m_videoElement(video),
m_checkViewportIntersectionTimer( m_checkViewportIntersectionTimer(
TaskRunnerHelper::get(TaskType::Unthrottled, &video.document()), TaskRunnerHelper::get(TaskType::Unthrottled, &video.document()),
...@@ -44,7 +43,6 @@ bool MediaCustomControlsFullscreenDetector::operator==( ...@@ -44,7 +43,6 @@ bool MediaCustomControlsFullscreenDetector::operator==(
} }
void MediaCustomControlsFullscreenDetector::attach() { void MediaCustomControlsFullscreenDetector::attach() {
setContext(&videoElement().document());
videoElement().document().addEventListener( videoElement().document().addEventListener(
EventTypeNames::webkitfullscreenchange, this, true); EventTypeNames::webkitfullscreenchange, this, true);
videoElement().document().addEventListener(EventTypeNames::fullscreenchange, videoElement().document().addEventListener(EventTypeNames::fullscreenchange,
...@@ -52,12 +50,14 @@ void MediaCustomControlsFullscreenDetector::attach() { ...@@ -52,12 +50,14 @@ void MediaCustomControlsFullscreenDetector::attach() {
} }
void MediaCustomControlsFullscreenDetector::detach() { void MediaCustomControlsFullscreenDetector::detach() {
setContext(nullptr);
videoElement().document().removeEventListener( videoElement().document().removeEventListener(
EventTypeNames::webkitfullscreenchange, this, true); EventTypeNames::webkitfullscreenchange, this, true);
videoElement().document().removeEventListener( videoElement().document().removeEventListener(
EventTypeNames::fullscreenchange, this, true); EventTypeNames::fullscreenchange, this, true);
m_checkViewportIntersectionTimer.stop(); m_checkViewportIntersectionTimer.stop();
if (videoElement().webMediaPlayer())
videoElement().webMediaPlayer()->setIsEffectivelyFullscreen(false);
} }
bool MediaCustomControlsFullscreenDetector::computeIsDominantVideoForTests( bool MediaCustomControlsFullscreenDetector::computeIsDominantVideoForTests(
...@@ -130,11 +130,11 @@ void MediaCustomControlsFullscreenDetector::handleEvent( ...@@ -130,11 +130,11 @@ void MediaCustomControlsFullscreenDetector::handleEvent(
BLINK_FROM_HERE); BLINK_FROM_HERE);
} }
void MediaCustomControlsFullscreenDetector::contextDestroyed( void MediaCustomControlsFullscreenDetector::contextDestroyed() {
ExecutionContext*) { // This method is called by HTMLVideoElement when it observes context destroy.
if (videoElement().webMediaPlayer()) // The reason is that when HTMLMediaElement observes context destroy, it will
videoElement().webMediaPlayer()->setIsEffectivelyFullscreen(false); // destroy webMediaPlayer() thus the final setIsEffectivelyFullscreen(false)
// is not called.
detach(); detach();
} }
...@@ -164,7 +164,6 @@ bool MediaCustomControlsFullscreenDetector::isVideoOrParentFullscreen() { ...@@ -164,7 +164,6 @@ bool MediaCustomControlsFullscreenDetector::isVideoOrParentFullscreen() {
DEFINE_TRACE(MediaCustomControlsFullscreenDetector) { DEFINE_TRACE(MediaCustomControlsFullscreenDetector) {
EventListener::trace(visitor); EventListener::trace(visitor);
ContextLifecycleObserver::trace(visitor);
visitor->trace(m_videoElement); visitor->trace(m_videoElement);
} }
......
third_party/WebKit/Source/core/html/MediaCustomControlsFullscreenDetector.h
View file @ ac5c34a5
...@@ -6,7 +6,6 @@ ...@@ -6,7 +6,6 @@
#define MediaCustomControlsFullscreenDetector_h #define MediaCustomControlsFullscreenDetector_h
#include "core/CoreExport.h" #include "core/CoreExport.h"
#include "core/dom/ContextLifecycleObserver.h"
#include "core/events/EventListener.h" #include "core/events/EventListener.h"
#include "platform/Timer.h" #include "platform/Timer.h"
...@@ -17,9 +16,7 @@ class IntRect; ...@@ -17,9 +16,7 @@ class IntRect;
class TimerBase; class TimerBase;
class CORE_EXPORT MediaCustomControlsFullscreenDetector final class CORE_EXPORT MediaCustomControlsFullscreenDetector final
: public EventListener, : public EventListener {
public ContextLifecycleObserver {
USING_GARBAGE_COLLECTED_MIXIN(MediaCustomControlsFullscreenDetector);
WTF_MAKE_NONCOPYABLE(MediaCustomControlsFullscreenDetector); WTF_MAKE_NONCOPYABLE(MediaCustomControlsFullscreenDetector);
public: public:
...@@ -28,8 +25,7 @@ class CORE_EXPORT MediaCustomControlsFullscreenDetector final ...@@ -28,8 +25,7 @@ class CORE_EXPORT MediaCustomControlsFullscreenDetector final
// EventListener implementation. // EventListener implementation.
bool operator==(const EventListener&) const override; bool operator==(const EventListener&) const override;
// ContextLifecycleObserver implemnetation. void contextDestroyed();
void contextDestroyed(ExecutionContext*);
DECLARE_VIRTUAL_TRACE(); DECLARE_VIRTUAL_TRACE();
......
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