Attachment 349874 Details for Bug 189656 – Patch

[patch] Patch

bug-189656-20180916174412.patch (text/plain), 8.56 KB, created by Simon Fraser (smfr) on 2018-09-16 17:44:13 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Simon Fraser (smfr)

Created: 2018-09-16 17:44:13 PDT

Size: 8.56 KB

patch

obsolete

>Subversion Revision: 235830
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 099c7eef00549df273bd830789c42db59e9d877b..600fce491091019f1995356644a99427a74bd2e4 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,17 @@
>+2018-09-16  Simon Fraser  <simon.fraser@apple.com>
>+
>+        Add more Fullscreen logging
>+        https://bugs.webkit.org/show_bug.cgi?id=189656
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        No new tests (OOPS!).
>+
>+        * platform/cocoa/VideoFullscreenModelVideoElement.mm:
>+        (VideoFullscreenModelVideoElement::VideoFullscreenModelVideoElement):
>+        (VideoFullscreenModelVideoElement::~VideoFullscreenModelVideoElement):
>+        (VideoFullscreenModelVideoElement::setVideoElement):
>+
> 2018-09-16  Simon Fraser  <simon.fraser@apple.com>
> 
>         Add more ResourceLoading logging, particularly in MemoryCache code
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index a728f3b58251819813d01b577dcdf19ec5bd45e4..6e190801878266a84b17db0783f7a9dbf82b273c 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,26 @@
>+2018-09-16  Simon Fraser  <simon.fraser@apple.com>
>+
>+        Add more Fullscreen logging
>+        https://bugs.webkit.org/show_bug.cgi?id=189656
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Add some fullscreen logging so I can tell whether WebFullScreenManager ever releases
>+        the fullscreen element.
>+
>+        * WebProcess/FullScreen/WebFullScreenManager.cpp:
>+        (WebKit::WebFullScreenManager::WebFullScreenManager):
>+        (WebKit::WebFullScreenManager::videoControlsManagerDidChange):
>+        (WebKit::WebFullScreenManager::setPIPStandbyElement):
>+        (WebKit::WebFullScreenManager::enterFullScreenForElement):
>+        (WebKit::WebFullScreenManager::exitFullScreenForElement):
>+        (WebKit::WebFullScreenManager::willEnterFullScreen):
>+        (WebKit::WebFullScreenManager::didEnterFullScreen):
>+        (WebKit::WebFullScreenManager::willExitFullScreen):
>+        (WebKit::WebFullScreenManager::didExitFullScreen):
>+        (WebKit::WebFullScreenManager::close):
>+        * WebProcess/FullScreen/WebFullScreenManager.h:
>+
> 2018-09-15  Simon Fraser  <simon.fraser@apple.com>
> 
>         Many modern media control tests leak documents in testing
>diff --git a/Source/WebCore/platform/cocoa/VideoFullscreenModelVideoElement.mm b/Source/WebCore/platform/cocoa/VideoFullscreenModelVideoElement.mm
>index c0a4fa5f0eb5af0f87f585dcca5d65706a7f504e..ff39282e0d16f671f261cb5a8e2c755d2f55fd80 100644
>--- a/Source/WebCore/platform/cocoa/VideoFullscreenModelVideoElement.mm
>+++ b/Source/WebCore/platform/cocoa/VideoFullscreenModelVideoElement.mm
>@@ -50,10 +50,12 @@ using namespace WebCore;
> VideoFullscreenModelVideoElement::VideoFullscreenModelVideoElement()
>     : EventListener(EventListener::CPPEventListenerType)
> {
>+    LOG(Fullscreen, "VideoFullscreenModelVideoElement %p ctor", this);
> }
> 
> VideoFullscreenModelVideoElement::~VideoFullscreenModelVideoElement()
> {
>+    LOG(Fullscreen, "VideoFullscreenModelVideoElement %p dtor", this);
> }
> 
> void VideoFullscreenModelVideoElement::setVideoElement(HTMLVideoElement* videoElement)
>@@ -61,6 +63,8 @@ void VideoFullscreenModelVideoElement::setVideoElement(HTMLVideoElement* videoEl
>     if (m_videoElement == videoElement)
>         return;
> 
>+    LOG(Fullscreen, "VideoFullscreenModelVideoElement %p setVideoElement(%p)", this, videoElement);
>+
>     if (m_videoElement && m_videoElement->videoFullscreenLayer())
>         m_videoElement->setVideoFullscreenLayer(nullptr);
> 
>diff --git a/Source/WebKit/WebProcess/FullScreen/WebFullScreenManager.cpp b/Source/WebKit/WebProcess/FullScreen/WebFullScreenManager.cpp
>index 133a6c20d1acc31da43a83a25214c8667a8ee322..472fdf4c042cb3c12e5c9d0dcd18b430745aedc7 100644
>--- a/Source/WebKit/WebProcess/FullScreen/WebFullScreenManager.cpp
>+++ b/Source/WebKit/WebProcess/FullScreen/WebFullScreenManager.cpp
>@@ -28,6 +28,7 @@
> #if ENABLE(FULLSCREEN_API)
> 
> #include "Connection.h"
>+#include "Logging.h"
> #include "WebCoreArgumentCoders.h"
> #include "WebFrame.h"
> #include "WebFullScreenManagerProxyMessages.h"
>@@ -70,8 +71,7 @@ Ref<WebFullScreenManager> WebFullScreenManager::create(WebPage* page)
> }
> 
> WebFullScreenManager::WebFullScreenManager(WebPage* page)
>-    : m_topContentInset(0)
>-    , m_page(page)
>+    : m_page(page)
> {
> }
>     
>@@ -87,6 +87,8 @@ WebCore::Element* WebFullScreenManager::element()
> void WebFullScreenManager::videoControlsManagerDidChange()
> {
> #if PLATFORM(IOS) || (PLATFORM(MAC) && ENABLE(VIDEO_PRESENTATION_MODE))
>+    LOG(Fullscreen, "WebFullScreenManager %p videoControlsManagerDidChange()", this);
>+
>     auto* currentPlaybackControlsElement = m_page->playbackSessionManager().currentPlaybackControlsElement();
>     if (!m_element || !is<HTMLVideoElement>(currentPlaybackControlsElement)) {
>         setPIPStandbyElement(nullptr);
>@@ -102,6 +104,8 @@ void WebFullScreenManager::setPIPStandbyElement(WebCore::HTMLVideoElement* pipSt
>     if (pipStandbyElement == m_pipStandbyElement)
>         return;
> 
>+    LOG(Fullscreen, "WebFullScreenManager %p setPIPStandbyElement() - old element %p, new element %p", this, m_pipStandbyElement.get(), pipStandbyElement);
>+
>     if (m_pipStandbyElement)
>         m_pipStandbyElement->setVideoFullscreenStandby(false);
> 
>@@ -126,6 +130,8 @@ bool WebFullScreenManager::supportsFullScreen(bool withKeyboard)
> 
> void WebFullScreenManager::enterFullScreenForElement(WebCore::Element* element)
> {
>+    LOG(Fullscreen, "WebFullScreenManager %p enterFullScreenForElement(%p)", this, element);
>+
>     ASSERT(element);
>     m_element = element;
>     m_initialFrame = screenRectOfContents(m_element.get());
>@@ -134,12 +140,14 @@ void WebFullScreenManager::enterFullScreenForElement(WebCore::Element* element)
> 
> void WebFullScreenManager::exitFullScreenForElement(WebCore::Element* element)
> {
>+    LOG(Fullscreen, "WebFullScreenManager %p exitFullScreenForElement(%p) - fullscreen element %p", this, element, m_element.get());
>     m_page->injectedBundleFullScreenClient().exitFullScreenForElement(m_page.get(), element);
> }
> 
> void WebFullScreenManager::willEnterFullScreen()
> {
>-    ASSERT(m_element);
>+    LOG(Fullscreen, "WebFullScreenManager %p willEnterFullScreen() - element %p", this, m_element.get());
>+
>     m_element->document().webkitWillEnterFullScreenForElement(m_element.get());
> #if !PLATFORM(IOS)
>     m_page->hidePageBanners();
>@@ -152,7 +160,8 @@ void WebFullScreenManager::willEnterFullScreen()
> 
> void WebFullScreenManager::didEnterFullScreen()
> {
>-    ASSERT(m_element);
>+    LOG(Fullscreen, "WebFullScreenManager %p didEnterFullScreen() - element %p", this, m_element.get());
>+
>     m_element->document().webkitDidEnterFullScreenForElement(m_element.get());
> 
> #if PLATFORM(IOS) || (PLATFORM(MAC) && ENABLE(VIDEO_PRESENTATION_MODE))
>@@ -163,6 +172,7 @@ void WebFullScreenManager::didEnterFullScreen()
> 
> void WebFullScreenManager::willExitFullScreen()
> {
>+    LOG(Fullscreen, "WebFullScreenManager %p willExitFullScreen() - element %p", this, m_element.get());
>     ASSERT(m_element);
> 
> #if ENABLE(VIDEO)
>@@ -179,6 +189,8 @@ void WebFullScreenManager::willExitFullScreen()
> 
> void WebFullScreenManager::didExitFullScreen()
> {
>+    LOG(Fullscreen, "WebFullScreenManager %p didExitFullScreen() - element %p", this, m_element.get());
>+
>     ASSERT(m_element);
>     setFullscreenInsets(FloatBoxExtent());
>     setFullscreenAutoHideDuration(0_s);
>@@ -199,6 +211,7 @@ void WebFullScreenManager::requestExitFullScreen()
> 
> void WebFullScreenManager::close()
> {
>+    LOG(Fullscreen, "WebFullScreenManager %p close()", this);
>     m_page->injectedBundleFullScreenClient().closeFullScreen(m_page.get());
> }
> 
>diff --git a/Source/WebKit/WebProcess/FullScreen/WebFullScreenManager.h b/Source/WebKit/WebProcess/FullScreen/WebFullScreenManager.h
>index 3c8ad024bd1db20cdb0a4565b07d332d63f0193a..9120ac90c77fdab202e267498a02107aa0165865 100644
>--- a/Source/WebKit/WebProcess/FullScreen/WebFullScreenManager.h
>+++ b/Source/WebKit/WebProcess/FullScreen/WebFullScreenManager.h
>@@ -22,8 +22,8 @@
>  * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF
>  * THE POSSIBILITY OF SUCH DAMAGE.
>  */
>-#ifndef WebFullScreenManager_h
>-#define WebFullScreenManager_h
>+
>+#pragma once
> 
> #if ENABLE(FULLSCREEN_API)
> 
>@@ -89,7 +89,7 @@ protected:
>     WebCore::IntRect m_initialFrame;
>     WebCore::IntRect m_finalFrame;
>     WebCore::IntPoint m_scrollPosition;
>-    float m_topContentInset;
>+    float m_topContentInset { 0 };
>     RefPtr<WebPage> m_page;
>     RefPtr<WebCore::Element> m_element;
> #if ENABLE(VIDEO)
>@@ -100,5 +100,3 @@ protected:
> } // namespace WebKit
> 
> #endif // ENABLE(FULLSCREEN_API)
>-
>-#endif // WebFullScreenManager_h

Flags:

jer.noble: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 189656: 349874