- Revision
- 197787
- Author
- [email protected]
- Date
- 2016-03-08 11:32:58 -0800 (Tue, 08 Mar 2016)
Log Message
Fix lifetime issues regarding WebVideoFullscreenInterfaceMac
https://bugs.webkit.org/show_bug.cgi?id=155130
Reviewed by Beth Dakin.
Now that both fullscreen and video controls manager rely on WebVideoFullscreenInterface,
we now keep track of a "client count" for each context ID so we'll only remove it
from the context map after all the "clients" are done with it.
Before this change, every time WebVideoFullscreenManager::setUpVideoControlsManager()
is called, the existing interface is cleared and a new one is created even when there's
an existing interface for that. With this change, we reuse the existing interface for
the video element.
* UIProcess/Cocoa/WebVideoFullscreenManagerProxy.h:
* UIProcess/Cocoa/WebVideoFullscreenManagerProxy.mm:
(WebKit::WebVideoFullscreenManagerProxy::invalidate):
Also clear out m_clientCounts.
(WebKit::WebVideoFullscreenManagerProxy::addClientForContext):
If the context ID is not in m_clientCounts yet, add the count of 1 to
that table for that ID. Otherwise, increment the count by 1.
(WebKit::WebVideoFullscreenManagerProxy::removeClientForContext):
Assert that we have added this context id to m_clientCounts before.
Decrement the count. If it reaches 0, remove this context ID from both
m_clientCounts and m_contextMap.
(WebKit::WebVideoFullscreenManagerProxy::setupFullscreenWithID):
We have started a new fullscreen session using this interface. Call
addClientForContext() to update the client count.
(WebKit::WebVideoFullscreenManagerProxy::setUpVideoControlsManagerWithID):
If the current controls manager set up has the same context ID as the one
passed in, we don't have to do anything. Otherwise, if we have set up
the video controls manager with a different ID before, call removeClientForContext()
on the old ID to decrement its client count. Set m_controlsManagerContextId
to the new value and call addClientForContext() with it.
(WebKit::WebVideoFullscreenManagerProxy::didCleanupFullscreen):
Instead of removing the mapping from m_contextMap directly, reset the fullscreen
mode on the interface and call removeClientForContext(), which removes the mapping
only if there's no more client using the interface.
* WebProcess/cocoa/WebVideoFullscreenManager.h:
* WebProcess/cocoa/WebVideoFullscreenManager.mm:
(WebKit::WebVideoFullscreenManager::~WebVideoFullscreenManager):
Clear m_clientCounts.
(WebKit::WebVideoFullscreenManager::removeContext):
Add a helper method for removing the context.
(WebKit::WebVideoFullscreenManager::addClientForContext):
If the context ID is not in m_clientCounts yet, add the count of 1 to
that table for that ID. Otherwise, increment the count by 1.
(WebKit::WebVideoFullscreenManager::removeClientForContext):
Assert that we have added this context id to m_clientCounts before.
Decrement the count. If it reaches 0, remove this context ID from
m_clientCounts and call removeContext() to clean up this context.
(WebKit::WebVideoFullscreenManager::enterVideoFullscreenForVideoElement):
We have started a new fullscreen session using this interface. Call
addClientForContext() to update the client count. Create a layer hosting
context if it's not there.
(WebKit::WebVideoFullscreenManager::setUpVideoControlsManager):
If we have set up a context for this video element before, see if it's
the one we are currently managing video controls for. If it is, return early.
Otherwise, call removeClientForContext() on the previous m_controlsManagerContextId
and update m_controlsManagerContextId to the context ID of this video element.
If there's no context created for this video element yet, set one up.
Send a SetUpVideoControlsManagerWithID message to the proxy object in the UI process
so it'll update its controls manager context ID. Also, conditionalize all of this
under PLATFORM(MAC) to be consistent with WebVideoFullscreenManagerProxy.
(WebKit::WebVideoFullscreenManager::didCleanupFullscreen):
Just reset the fullscreen mode on the interface and call removeClientForContext() which
only cleans up the context if there's no more client using it.
Modified Paths
Diff
Modified: trunk/Source/WebKit2/ChangeLog (197786 => 197787)
--- trunk/Source/WebKit2/ChangeLog 2016-03-08 19:02:31 UTC (rev 197786)
+++ trunk/Source/WebKit2/ChangeLog 2016-03-08 19:32:58 UTC (rev 197787)
@@ -1,3 +1,73 @@
+2016-03-04 Ada Chan <[email protected]>
+
+ Fix lifetime issues regarding WebVideoFullscreenInterfaceMac
+ https://bugs.webkit.org/show_bug.cgi?id=155130
+
+ Reviewed by Beth Dakin.
+
+ Now that both fullscreen and video controls manager rely on WebVideoFullscreenInterface,
+ we now keep track of a "client count" for each context ID so we'll only remove it
+ from the context map after all the "clients" are done with it.
+
+ Before this change, every time WebVideoFullscreenManager::setUpVideoControlsManager()
+ is called, the existing interface is cleared and a new one is created even when there's
+ an existing interface for that. With this change, we reuse the existing interface for
+ the video element.
+
+ * UIProcess/Cocoa/WebVideoFullscreenManagerProxy.h:
+ * UIProcess/Cocoa/WebVideoFullscreenManagerProxy.mm:
+ (WebKit::WebVideoFullscreenManagerProxy::invalidate):
+ Also clear out m_clientCounts.
+ (WebKit::WebVideoFullscreenManagerProxy::addClientForContext):
+ If the context ID is not in m_clientCounts yet, add the count of 1 to
+ that table for that ID. Otherwise, increment the count by 1.
+ (WebKit::WebVideoFullscreenManagerProxy::removeClientForContext):
+ Assert that we have added this context id to m_clientCounts before.
+ Decrement the count. If it reaches 0, remove this context ID from both
+ m_clientCounts and m_contextMap.
+ (WebKit::WebVideoFullscreenManagerProxy::setupFullscreenWithID):
+ We have started a new fullscreen session using this interface. Call
+ addClientForContext() to update the client count.
+ (WebKit::WebVideoFullscreenManagerProxy::setUpVideoControlsManagerWithID):
+ If the current controls manager set up has the same context ID as the one
+ passed in, we don't have to do anything. Otherwise, if we have set up
+ the video controls manager with a different ID before, call removeClientForContext()
+ on the old ID to decrement its client count. Set m_controlsManagerContextId
+ to the new value and call addClientForContext() with it.
+ (WebKit::WebVideoFullscreenManagerProxy::didCleanupFullscreen):
+ Instead of removing the mapping from m_contextMap directly, reset the fullscreen
+ mode on the interface and call removeClientForContext(), which removes the mapping
+ only if there's no more client using the interface.
+ * WebProcess/cocoa/WebVideoFullscreenManager.h:
+ * WebProcess/cocoa/WebVideoFullscreenManager.mm:
+ (WebKit::WebVideoFullscreenManager::~WebVideoFullscreenManager):
+ Clear m_clientCounts.
+ (WebKit::WebVideoFullscreenManager::removeContext):
+ Add a helper method for removing the context.
+ (WebKit::WebVideoFullscreenManager::addClientForContext):
+ If the context ID is not in m_clientCounts yet, add the count of 1 to
+ that table for that ID. Otherwise, increment the count by 1.
+ (WebKit::WebVideoFullscreenManager::removeClientForContext):
+ Assert that we have added this context id to m_clientCounts before.
+ Decrement the count. If it reaches 0, remove this context ID from
+ m_clientCounts and call removeContext() to clean up this context.
+ (WebKit::WebVideoFullscreenManager::enterVideoFullscreenForVideoElement):
+ We have started a new fullscreen session using this interface. Call
+ addClientForContext() to update the client count. Create a layer hosting
+ context if it's not there.
+ (WebKit::WebVideoFullscreenManager::setUpVideoControlsManager):
+ If we have set up a context for this video element before, see if it's
+ the one we are currently managing video controls for. If it is, return early.
+ Otherwise, call removeClientForContext() on the previous m_controlsManagerContextId
+ and update m_controlsManagerContextId to the context ID of this video element.
+ If there's no context created for this video element yet, set one up.
+ Send a SetUpVideoControlsManagerWithID message to the proxy object in the UI process
+ so it'll update its controls manager context ID. Also, conditionalize all of this
+ under PLATFORM(MAC) to be consistent with WebVideoFullscreenManagerProxy.
+ (WebKit::WebVideoFullscreenManager::didCleanupFullscreen):
+ Just reset the fullscreen mode on the interface and call removeClientForContext() which
+ only cleans up the context if there's no more client using it.
+
2016-03-08 Timothy Hatcher <[email protected]>
Web Inspector: Add automation protocol methods for navigation
Modified: trunk/Source/WebKit2/UIProcess/Cocoa/WebVideoFullscreenManagerProxy.h (197786 => 197787)
--- trunk/Source/WebKit2/UIProcess/Cocoa/WebVideoFullscreenManagerProxy.h 2016-03-08 19:02:31 UTC (rev 197786)
+++ trunk/Source/WebKit2/UIProcess/Cocoa/WebVideoFullscreenManagerProxy.h 2016-03-08 19:32:58 UTC (rev 197787)
@@ -132,6 +132,8 @@
ModelInterfaceTuple& ensureModelAndInterface(uint64_t contextId);
WebVideoFullscreenModelContext& ensureModel(uint64_t contextId);
PlatformWebVideoFullscreenInterface& ensureInterface(uint64_t contextId);
+ void addClientForContext(uint64_t contextId);
+ void removeClientForContext(uint64_t contextId);
// Messages from WebVideoFullscreenManager
void setupFullscreenWithID(uint64_t contextId, uint32_t videoLayerID, const WebCore::IntRect& initialRect, float hostingScaleFactor, WebCore::HTMLMediaElementEnums::VideoFullscreenMode, bool allowsPictureInPicture);
@@ -182,7 +184,7 @@
WebPageProxy* m_page;
HashMap<uint64_t, ModelInterfaceTuple> m_contextMap;
uint64_t m_controlsManagerContextId { 0 };
-
+ HashMap<uint64_t, int> m_clientCounts;
};
} // namespace WebKit
Modified: trunk/Source/WebKit2/UIProcess/Cocoa/WebVideoFullscreenManagerProxy.mm (197786 => 197787)
--- trunk/Source/WebKit2/UIProcess/Cocoa/WebVideoFullscreenManagerProxy.mm 2016-03-08 19:02:31 UTC (rev 197786)
+++ trunk/Source/WebKit2/UIProcess/Cocoa/WebVideoFullscreenManagerProxy.mm 2016-03-08 19:32:58 UTC (rev 197787)
@@ -278,6 +278,7 @@
}
m_contextMap.clear();
+ m_clientCounts.clear();
}
void WebVideoFullscreenManagerProxy::requestHideAndExitFullscreen()
@@ -339,6 +340,30 @@
return *std::get<1>(ensureModelAndInterface(contextId));
}
+void WebVideoFullscreenManagerProxy::addClientForContext(uint64_t contextId)
+{
+ auto addResult = m_clientCounts.add(contextId, 1);
+ if (!addResult.isNewEntry)
+ addResult.iterator->value++;
+}
+
+void WebVideoFullscreenManagerProxy::removeClientForContext(uint64_t contextId)
+{
+ ASSERT(m_clientCounts.contains(contextId));
+
+ int clientCount = m_clientCounts.get(contextId);
+ ASSERT(clientCount > 0);
+ clientCount--;
+
+ if (clientCount <= 0) {
+ m_clientCounts.remove(contextId);
+ m_contextMap.remove(contextId);
+ return;
+ }
+
+ m_clientCounts.set(contextId, clientCount);
+}
+
#pragma mark Messages from WebVideoFullscreenManager
void WebVideoFullscreenManagerProxy::setupFullscreenWithID(uint64_t contextId, uint32_t videoLayerID, const WebCore::IntRect& initialRect, float hostingDeviceScaleFactor, HTMLMediaElementEnums::VideoFullscreenMode videoFullscreenMode, bool allowsPictureInPicture)
@@ -348,6 +373,7 @@
RefPtr<PlatformWebVideoFullscreenInterface> interface;
std::tie(model, interface) = ensureModelAndInterface(contextId);
+ addClientForContext(contextId);
RetainPtr<WKLayerHostView> view = static_cast<WKLayerHostView*>(model->layerHostView());
if (!view) {
@@ -377,16 +403,15 @@
void WebVideoFullscreenManagerProxy::setUpVideoControlsManagerWithID(uint64_t contextId)
{
#if PLATFORM(MAC)
- // If there is an existing controls manager, clean it up.
+ if (m_controlsManagerContextId == contextId)
+ return;
+
if (m_controlsManagerContextId)
- m_contextMap.remove(m_controlsManagerContextId);
+ removeClientForContext(m_controlsManagerContextId);
- RefPtr<WebVideoFullscreenModelContext> model;
- RefPtr<PlatformWebVideoFullscreenInterface> interface;
-
- std::tie(model, interface) = ensureModelAndInterface(contextId);
m_controlsManagerContextId = contextId;
- interface->ensureControlsManager();
+ ensureInterface(m_controlsManagerContextId).ensureControlsManager();
+ addClientForContext(m_controlsManagerContextId);
#else
UNUSED_PARAM(contextId);
#endif
@@ -592,14 +617,18 @@
void WebVideoFullscreenManagerProxy::didCleanupFullscreen(uint64_t contextId)
{
- auto& model = ensureModel(contextId);
+ RefPtr<WebVideoFullscreenModelContext> model;
+ RefPtr<PlatformWebVideoFullscreenInterface> interface;
+ std::tie(model, interface) = ensureModelAndInterface(contextId);
+
[CATransaction flush];
- [model.layerHostView() removeFromSuperview];
- model.setLayerHostView(nullptr);
+ [model->layerHostView() removeFromSuperview];
+ model->setLayerHostView(nullptr);
m_page->send(Messages::WebVideoFullscreenManager::DidCleanupFullscreen(contextId), m_page->pageID());
- m_contextMap.remove(contextId);
+ interface->setMode(HTMLMediaElementEnums::VideoFullscreenModeNone);
+ removeClientForContext(contextId);
}
void WebVideoFullscreenManagerProxy::setVideoLayerFrame(uint64_t contextId, WebCore::FloatRect frame)
Modified: trunk/Source/WebKit2/WebProcess/cocoa/WebVideoFullscreenManager.h (197786 => 197787)
--- trunk/Source/WebKit2/WebProcess/cocoa/WebVideoFullscreenManager.h 2016-03-08 19:02:31 UTC (rev 197786)
+++ trunk/Source/WebKit2/WebProcess/cocoa/WebVideoFullscreenManager.h 2016-03-08 19:32:58 UTC (rev 197787)
@@ -129,6 +129,9 @@
ModelInterfaceTuple& ensureModelAndInterface(uint64_t contextId);
WebCore::WebVideoFullscreenModelVideoElement& ensureModel(uint64_t contextId);
WebVideoFullscreenInterfaceContext& ensureInterface(uint64_t contextId);
+ void removeContext(uint64_t contextId);
+ void addClientForContext(uint64_t contextId);
+ void removeClientForContext(uint64_t contextId);
// Interface to WebVideoFullscreenInterfaceContext
void resetMediaState(uint64_t contextId);
@@ -171,6 +174,7 @@
HashMap<WebCore::HTMLVideoElement*, uint64_t> m_videoElements;
HashMap<uint64_t, ModelInterfaceTuple> m_contextMap;
uint64_t m_controlsManagerContextId { 0 };
+ HashMap<uint64_t, int> m_clientCounts;
};
} // namespace WebKit
Modified: trunk/Source/WebKit2/WebProcess/cocoa/WebVideoFullscreenManager.mm (197786 => 197787)
--- trunk/Source/WebKit2/WebProcess/cocoa/WebVideoFullscreenManager.mm 2016-03-08 19:02:31 UTC (rev 197786)
+++ trunk/Source/WebKit2/WebProcess/cocoa/WebVideoFullscreenManager.mm 2016-03-08 19:32:58 UTC (rev 197787)
@@ -182,6 +182,7 @@
m_contextMap.clear();
m_videoElements.clear();
+ m_clientCounts.clear();
WebProcess::singleton().removeMessageReceiver(Messages::WebVideoFullscreenManager::messageReceiverName(), m_page->pageID());
}
@@ -215,6 +216,44 @@
return *std::get<1>(ensureModelAndInterface(contextId));
}
+void WebVideoFullscreenManager::removeContext(uint64_t contextId)
+{
+ RefPtr<WebVideoFullscreenModelVideoElement> model;
+ RefPtr<WebVideoFullscreenInterfaceContext> interface;
+ std::tie(model, interface) = ensureModelAndInterface(contextId);
+
+ RefPtr<HTMLVideoElement> videoElement = model->videoElement();
+ model->setVideoElement(nullptr);
+ model->setWebVideoFullscreenInterface(nullptr);
+ interface->invalidate();
+ m_videoElements.remove(videoElement.get());
+ m_contextMap.remove(contextId);
+}
+
+void WebVideoFullscreenManager::addClientForContext(uint64_t contextId)
+{
+ auto addResult = m_clientCounts.add(contextId, 1);
+ if (!addResult.isNewEntry)
+ addResult.iterator->value++;
+}
+
+void WebVideoFullscreenManager::removeClientForContext(uint64_t contextId)
+{
+ ASSERT(m_clientCounts.contains(contextId));
+
+ int clientCount = m_clientCounts.get(contextId);
+ ASSERT(clientCount > 0);
+ clientCount--;
+
+ if (clientCount <= 0) {
+ m_clientCounts.remove(contextId);
+ removeContext(contextId);
+ return;
+ }
+
+ m_clientCounts.set(contextId, clientCount);
+}
+
#pragma mark Interface to ChromeClient:
bool WebVideoFullscreenManager::supportsVideoFullscreen(WebCore::HTMLMediaElementEnums::VideoFullscreenMode mode) const
@@ -243,6 +282,9 @@
RefPtr<WebVideoFullscreenModelVideoElement> model;
RefPtr<WebVideoFullscreenInterfaceContext> interface;
std::tie(model, interface) = ensureModelAndInterface(contextId);
+ addClientForContext(contextId);
+ if (!interface->layerHostingContext())
+ interface->setLayerHostingContext(LayerHostingContext::createForExternalHostingProcess());
FloatRect clientRect = clientRectForElement(&videoElement);
FloatRect videoLayerFrame = FloatRect(0, 0, clientRect.width(), clientRect.height());
@@ -281,30 +323,25 @@
void WebVideoFullscreenManager::setUpVideoControlsManager(WebCore::HTMLVideoElement& videoElement)
{
- // If there is an existing controls manager, clean it up.
- if (m_controlsManagerContextId) {
- RefPtr<WebVideoFullscreenModelVideoElement> model;
- RefPtr<WebVideoFullscreenInterfaceContext> interface;
- std::tie(model, interface) = ensureModelAndInterface(m_controlsManagerContextId);
+#if PLATFORM(MAC)
+ if (m_videoElements.contains(&videoElement)) {
+ uint64_t contextId = m_videoElements.get(&videoElement);
+ if (m_controlsManagerContextId == contextId)
+ return;
- RefPtr<HTMLVideoElement> videoElement = model->videoElement();
- model->setVideoElement(nullptr);
- model->setWebVideoFullscreenInterface(nullptr);
- interface->invalidate();
- m_videoElements.remove(videoElement.get());
- m_contextMap.remove(m_controlsManagerContextId);
+ if (m_controlsManagerContextId)
+ removeClientForContext(m_controlsManagerContextId);
+ m_controlsManagerContextId = contextId;
+ } else {
+ auto addResult = m_videoElements.ensure(&videoElement, [&] { return nextContextId(); });
+ auto contextId = addResult.iterator->value;
+ m_controlsManagerContextId = contextId;
+ ensureModel(contextId).setVideoElement(&videoElement);
}
- auto addResult = m_videoElements.ensure(&videoElement, [&] { return nextContextId(); });
- auto contextId = addResult.iterator->value;
- m_controlsManagerContextId = contextId;
-
- RefPtr<WebVideoFullscreenModelVideoElement> model;
- RefPtr<WebVideoFullscreenInterfaceContext> interface;
- std::tie(model, interface) = ensureModelAndInterface(contextId);
- model->setVideoElement(&videoElement);
-
- m_page->send(Messages::WebVideoFullscreenManagerProxy::SetUpVideoControlsManagerWithID(contextId), m_page->pageID());
+ addClientForContext(m_controlsManagerContextId);
+ m_page->send(Messages::WebVideoFullscreenManagerProxy::SetUpVideoControlsManagerWithID(m_controlsManagerContextId), m_page->pageID());
+#endif
}
void WebVideoFullscreenManager::exitVideoFullscreenToModeWithoutAnimation(WebCore::HTMLVideoElement& videoElement, WebCore::HTMLMediaElementEnums::VideoFullscreenMode targetMode)
@@ -558,11 +595,8 @@
model->setVideoFullscreenLayer(nil);
RefPtr<HTMLVideoElement> videoElement = model->videoElement();
- model->setVideoElement(nullptr);
- model->setWebVideoFullscreenInterface(nullptr);
- interface->invalidate();
- m_videoElements.remove(videoElement.get());
- m_contextMap.remove(contextId);
+ interface->setFullscreenMode(HTMLMediaElementEnums::VideoFullscreenModeNone);
+ removeClientForContext(contextId);
if (!videoElement || !targetIsFullscreen)
return;
