- Revision
- 247563
- Author
- carlo...@webkit.org
- Date
- 2019-07-18 07:42:48 -0700 (Thu, 18 Jul 2019)
Log Message
[GTK] Crash in webkitWebViewBaseRenderHostFileDescriptor
https://bugs.webkit.org/show_bug.cgi?id=199402
Reviewed by Michael Catanzaro.
There are two problems here:
- We need to ensure that the checks we do in HardwareAccelerationManager to disable AC mode are the same
as the ones done in AcceleratedBackingStore create() methods. This is not the case for WPE renderer.
- Some of the places where accelerateBackingStore is used, can be called even if AC mode is disabled, so we
need to null-check there before using the backing store.
* UIProcess/API/gtk/WebKitWebViewBase.cpp:
(webkitWebViewBaseDraw): Add an assert to ensure accelerateBackingStore is not nullptr here.
(webkitWebViewBaseEnterAcceleratedCompositingMode): Ditto.
(webkitWebViewBaseUpdateAcceleratedCompositingMode): Ditto.
(webkitWebViewBaseExitAcceleratedCompositingMode): Ditto.
(webkitWebViewBaseMakeGLContextCurrent): Ditto.
(webkitWebViewBaseDidRelaunchWebProcess): Null-check accelerateBackingStore before using it.
(webkitWebViewBasePageClosed): Ditto.
(webkitWebViewBaseRenderHostFileDescriptor): Ditto.
* UIProcess/gtk/AcceleratedBackingStore.cpp:
(WebKit::AcceleratedBackingStore::checkRequirements): Call AcceleratedBackingStoreWayland::checkRequirements()
or AcceleratedBackingStoreX11::checkRequirements() depending on the current display.
(WebKit::AcceleratedBackingStore::create): Return early if AC mode is disabled in HardwareAccelerationManager.
* UIProcess/gtk/AcceleratedBackingStore.h:
* UIProcess/gtk/AcceleratedBackingStoreWayland.cpp:
(WebKit::AcceleratedBackingStoreWayland::checkRequirements): Check requirements for hardware acceleration in Wayland.
(WebKit::AcceleratedBackingStoreWayland::create): Assert that requirements are present.
* UIProcess/gtk/AcceleratedBackingStoreWayland.h:
* UIProcess/gtk/AcceleratedBackingStoreX11.cpp:
(WebKit::AcceleratedBackingStoreX11::checkRequirements): Check requirements for hardware acceleration in X11.
(WebKit::AcceleratedBackingStoreX11::create): Assert that requirements are present.
* UIProcess/gtk/AcceleratedBackingStoreX11.h:
* UIProcess/gtk/HardwareAccelerationManager.cpp:
(WebKit::HardwareAccelerationManager::HardwareAccelerationManager): Use
AcceleratedBackingStore::checkRequirements() to decide whether to disable AC mode.
Modified Paths
Diff
Modified: trunk/Source/WebKit/ChangeLog (247562 => 247563)
--- trunk/Source/WebKit/ChangeLog 2019-07-18 13:37:07 UTC (rev 247562)
+++ trunk/Source/WebKit/ChangeLog 2019-07-18 14:42:48 UTC (rev 247563)
@@ -1,3 +1,43 @@
+2019-07-18 Carlos Garcia Campos <cgar...@igalia.com>
+
+ [GTK] Crash in webkitWebViewBaseRenderHostFileDescriptor
+ https://bugs.webkit.org/show_bug.cgi?id=199402
+
+ Reviewed by Michael Catanzaro.
+
+ There are two problems here:
+
+ - We need to ensure that the checks we do in HardwareAccelerationManager to disable AC mode are the same
+ as the ones done in AcceleratedBackingStore create() methods. This is not the case for WPE renderer.
+ - Some of the places where accelerateBackingStore is used, can be called even if AC mode is disabled, so we
+ need to null-check there before using the backing store.
+
+ * UIProcess/API/gtk/WebKitWebViewBase.cpp:
+ (webkitWebViewBaseDraw): Add an assert to ensure accelerateBackingStore is not nullptr here.
+ (webkitWebViewBaseEnterAcceleratedCompositingMode): Ditto.
+ (webkitWebViewBaseUpdateAcceleratedCompositingMode): Ditto.
+ (webkitWebViewBaseExitAcceleratedCompositingMode): Ditto.
+ (webkitWebViewBaseMakeGLContextCurrent): Ditto.
+ (webkitWebViewBaseDidRelaunchWebProcess): Null-check accelerateBackingStore before using it.
+ (webkitWebViewBasePageClosed): Ditto.
+ (webkitWebViewBaseRenderHostFileDescriptor): Ditto.
+ * UIProcess/gtk/AcceleratedBackingStore.cpp:
+ (WebKit::AcceleratedBackingStore::checkRequirements): Call AcceleratedBackingStoreWayland::checkRequirements()
+ or AcceleratedBackingStoreX11::checkRequirements() depending on the current display.
+ (WebKit::AcceleratedBackingStore::create): Return early if AC mode is disabled in HardwareAccelerationManager.
+ * UIProcess/gtk/AcceleratedBackingStore.h:
+ * UIProcess/gtk/AcceleratedBackingStoreWayland.cpp:
+ (WebKit::AcceleratedBackingStoreWayland::checkRequirements): Check requirements for hardware acceleration in Wayland.
+ (WebKit::AcceleratedBackingStoreWayland::create): Assert that requirements are present.
+ * UIProcess/gtk/AcceleratedBackingStoreWayland.h:
+ * UIProcess/gtk/AcceleratedBackingStoreX11.cpp:
+ (WebKit::AcceleratedBackingStoreX11::checkRequirements): Check requirements for hardware acceleration in X11.
+ (WebKit::AcceleratedBackingStoreX11::create): Assert that requirements are present.
+ * UIProcess/gtk/AcceleratedBackingStoreX11.h:
+ * UIProcess/gtk/HardwareAccelerationManager.cpp:
+ (WebKit::HardwareAccelerationManager::HardwareAccelerationManager): Use
+ AcceleratedBackingStore::checkRequirements() to decide whether to disable AC mode.
+
2019-07-17 Megan Gardner <megan_gard...@apple.com>
Early Out of positionInfomation check if possible
Modified: trunk/Source/WebKit/UIProcess/API/gtk/WebKitWebViewBase.cpp (247562 => 247563)
--- trunk/Source/WebKit/UIProcess/API/gtk/WebKitWebViewBase.cpp 2019-07-18 13:37:07 UTC (rev 247562)
+++ trunk/Source/WebKit/UIProcess/API/gtk/WebKitWebViewBase.cpp 2019-07-18 14:42:48 UTC (rev 247563)
@@ -585,9 +585,10 @@
if (showingNavigationSnapshot)
cairo_push_group(cr);
- if (drawingArea->isInAcceleratedCompositingMode())
+ if (drawingArea->isInAcceleratedCompositingMode()) {
+ ASSERT(webViewBase->priv->acceleratedBackingStore);
webViewBase->priv->acceleratedBackingStore->paint(cr, clipRect);
- else {
+ } else {
WebCore::Region unpaintedRegion; // This is simply unused.
drawingArea->paint(cr, clipRect, unpaintedRegion);
}
@@ -1663,21 +1664,25 @@
void webkitWebViewBaseEnterAcceleratedCompositingMode(WebKitWebViewBase* webkitWebViewBase, const LayerTreeContext& layerTreeContext)
{
+ ASSERT(webkitWebViewBase->priv->acceleratedBackingStore);
webkitWebViewBase->priv->acceleratedBackingStore->update(layerTreeContext);
}
void webkitWebViewBaseUpdateAcceleratedCompositingMode(WebKitWebViewBase* webkitWebViewBase, const LayerTreeContext& layerTreeContext)
{
+ ASSERT(webkitWebViewBase->priv->acceleratedBackingStore);
webkitWebViewBase->priv->acceleratedBackingStore->update(layerTreeContext);
}
void webkitWebViewBaseExitAcceleratedCompositingMode(WebKitWebViewBase* webkitWebViewBase)
{
+ ASSERT(webkitWebViewBase->priv->acceleratedBackingStore);
webkitWebViewBase->priv->acceleratedBackingStore->update(LayerTreeContext());
}
bool webkitWebViewBaseMakeGLContextCurrent(WebKitWebViewBase* webkitWebViewBase)
{
+ ASSERT(webkitWebViewBase->priv->acceleratedBackingStore);
return webkitWebViewBase->priv->acceleratedBackingStore->makeContextCurrent();
}
@@ -1700,8 +1705,10 @@
gtk_widget_queue_resize_no_redraw(GTK_WIDGET(webkitWebViewBase));
WebKitWebViewBasePrivate* priv = webkitWebViewBase->priv;
- auto* drawingArea = static_cast<DrawingAreaProxyCoordinatedGraphics*>(priv->pageProxy->drawingArea());
- webkitWebViewBaseUpdateAcceleratedCompositingMode(webkitWebViewBase, drawingArea->layerTreeContext());
+ if (priv->acceleratedBackingStore) {
+ auto* drawingArea = static_cast<DrawingAreaProxyCoordinatedGraphics*>(priv->pageProxy->drawingArea());
+ priv->acceleratedBackingStore->update(drawingArea->layerTreeContext());
+ }
if (priv->viewGestureController)
priv->viewGestureController->connectToProcess();
@@ -1713,7 +1720,8 @@
void webkitWebViewBasePageClosed(WebKitWebViewBase* webkitWebViewBase)
{
- webkitWebViewBase->priv->acceleratedBackingStore->update(LayerTreeContext());
+ if (webkitWebViewBase->priv->acceleratedBackingStore)
+ webkitWebViewBase->priv->acceleratedBackingStore->update({ });
}
RefPtr<WebKit::ViewSnapshot> webkitWebViewBaseTakeViewSnapshot(WebKitWebViewBase* webkitWebViewBase)
@@ -1817,6 +1825,8 @@
#if USE(WPE_RENDERER)
int webkitWebViewBaseRenderHostFileDescriptor(WebKitWebViewBase* webkitWebViewBase)
{
+ if (!webkitWebViewBase->priv->acceleratedBackingStore)
+ return -1;
return webkitWebViewBase->priv->acceleratedBackingStore->renderHostFileDescriptor();
}
#endif
Modified: trunk/Source/WebKit/UIProcess/gtk/AcceleratedBackingStore.cpp (247562 => 247563)
--- trunk/Source/WebKit/UIProcess/gtk/AcceleratedBackingStore.cpp 2019-07-18 13:37:07 UTC (rev 247562)
+++ trunk/Source/WebKit/UIProcess/gtk/AcceleratedBackingStore.cpp 2019-07-18 14:42:48 UTC (rev 247563)
@@ -26,6 +26,7 @@
#include "config.h"
#include "AcceleratedBackingStore.h"
+#include "HardwareAccelerationManager.h"
#include "WebPageProxy.h"
#include <WebCore/CairoUtilities.h>
#include <WebCore/PlatformDisplay.h>
@@ -41,8 +42,26 @@
namespace WebKit {
using namespace WebCore;
+bool AcceleratedBackingStore::checkRequirements()
+{
+#if PLATFORM(WAYLAND) && USE(EGL)
+ if (PlatformDisplay::sharedDisplay().type() == PlatformDisplay::Type::Wayland)
+ return AcceleratedBackingStoreWayland::checkRequirements();
+#endif
+#if PLATFORM(X11)
+ if (PlatformDisplay::sharedDisplay().type() == PlatformDisplay::Type::X11)
+ return AcceleratedBackingStoreX11::checkRequirements();
+#endif
+
+ RELEASE_ASSERT_NOT_REACHED();
+ return false;
+}
+
std::unique_ptr<AcceleratedBackingStore> AcceleratedBackingStore::create(WebPageProxy& webPage)
{
+ if (!HardwareAccelerationManager::singleton().canUseHardwareAcceleration())
+ return nullptr;
+
#if PLATFORM(WAYLAND) && USE(EGL)
if (PlatformDisplay::sharedDisplay().type() == PlatformDisplay::Type::Wayland)
return AcceleratedBackingStoreWayland::create(webPage);
Modified: trunk/Source/WebKit/UIProcess/gtk/AcceleratedBackingStore.h (247562 => 247563)
--- trunk/Source/WebKit/UIProcess/gtk/AcceleratedBackingStore.h 2019-07-18 13:37:07 UTC (rev 247562)
+++ trunk/Source/WebKit/UIProcess/gtk/AcceleratedBackingStore.h 2019-07-18 14:42:48 UTC (rev 247563)
@@ -41,6 +41,7 @@
class AcceleratedBackingStore {
WTF_MAKE_NONCOPYABLE(AcceleratedBackingStore); WTF_MAKE_FAST_ALLOCATED;
public:
+ static bool checkRequirements();
static std::unique_ptr<AcceleratedBackingStore> create(WebPageProxy&);
virtual ~AcceleratedBackingStore() = default;
Modified: trunk/Source/WebKit/UIProcess/gtk/AcceleratedBackingStoreWayland.cpp (247562 => 247563)
--- trunk/Source/WebKit/UIProcess/gtk/AcceleratedBackingStoreWayland.cpp 2019-07-18 13:37:07 UTC (rev 247562)
+++ trunk/Source/WebKit/UIProcess/gtk/AcceleratedBackingStoreWayland.cpp 2019-07-18 14:42:48 UTC (rev 247563)
@@ -64,19 +64,19 @@
namespace WebKit {
using namespace WebCore;
-std::unique_ptr<AcceleratedBackingStoreWayland> AcceleratedBackingStoreWayland::create(WebPageProxy& webPage)
+bool AcceleratedBackingStoreWayland::checkRequirements()
{
#if USE(WPE_RENDERER)
if (!glImageTargetTexture2D) {
if (!wpe_fdo_initialize_for_egl_display(PlatformDisplay::sharedDisplay().eglDisplay()))
- return nullptr;
+ return false;
std::unique_ptr<WebCore::GLContext> eglContext = GLContext::createOffscreenContext();
if (!eglContext)
- return nullptr;
+ return false;
if (!eglContext->makeContextCurrent())
- return nullptr;
+ return false;
#if USE(OPENGL_ES)
std::unique_ptr<Extensions3DOpenGLES> glExtensions = std::make_unique<Extensions3DOpenGLES>(nullptr, false);
@@ -89,12 +89,18 @@
if (!glImageTargetTexture2D) {
WTFLogAlways("AcceleratedBackingStoreWPE requires glEGLImageTargetTexture2D.");
- return nullptr;
+ return false;
}
+
+ return true;
#else
- if (!WaylandCompositor::singleton().isRunning())
- return nullptr;
+ return WaylandCompositor::singleton().isRunning();
#endif
+}
+
+std::unique_ptr<AcceleratedBackingStoreWayland> AcceleratedBackingStoreWayland::create(WebPageProxy& webPage)
+{
+ ASSERT(checkRequirements());
return std::unique_ptr<AcceleratedBackingStoreWayland>(new AcceleratedBackingStoreWayland(webPage));
}
Modified: trunk/Source/WebKit/UIProcess/gtk/AcceleratedBackingStoreWayland.h (247562 => 247563)
--- trunk/Source/WebKit/UIProcess/gtk/AcceleratedBackingStoreWayland.h 2019-07-18 13:37:07 UTC (rev 247562)
+++ trunk/Source/WebKit/UIProcess/gtk/AcceleratedBackingStoreWayland.h 2019-07-18 14:42:48 UTC (rev 247563)
@@ -52,6 +52,7 @@
class AcceleratedBackingStoreWayland final : public AcceleratedBackingStore {
WTF_MAKE_NONCOPYABLE(AcceleratedBackingStoreWayland); WTF_MAKE_FAST_ALLOCATED;
public:
+ static bool checkRequirements();
static std::unique_ptr<AcceleratedBackingStoreWayland> create(WebPageProxy&);
~AcceleratedBackingStoreWayland();
Modified: trunk/Source/WebKit/UIProcess/gtk/AcceleratedBackingStoreX11.cpp (247562 => 247563)
--- trunk/Source/WebKit/UIProcess/gtk/AcceleratedBackingStoreX11.cpp 2019-07-18 13:37:07 UTC (rev 247562)
+++ trunk/Source/WebKit/UIProcess/gtk/AcceleratedBackingStoreX11.cpp 2019-07-18 14:42:48 UTC (rev 247563)
@@ -103,11 +103,15 @@
HashMap<Damage, WTF::Function<void()>> m_notifyFunctions;
};
+bool AcceleratedBackingStoreX11::checkRequirements()
+{
+ auto& display = downcast<PlatformDisplayX11>(PlatformDisplay::sharedDisplay());
+ return display.supportsXComposite() && display.supportsXDamage(s_damageEventBase, s_damageErrorBase);
+}
+
std::unique_ptr<AcceleratedBackingStoreX11> AcceleratedBackingStoreX11::create(WebPageProxy& webPage)
{
- auto& display = downcast<PlatformDisplayX11>(PlatformDisplay::sharedDisplay());
- if (!display.supportsXComposite() || !display.supportsXDamage(s_damageEventBase, s_damageErrorBase))
- return nullptr;
+ ASSERT(checkRequirements());
return std::unique_ptr<AcceleratedBackingStoreX11>(new AcceleratedBackingStoreX11(webPage));
}
Modified: trunk/Source/WebKit/UIProcess/gtk/AcceleratedBackingStoreX11.h (247562 => 247563)
--- trunk/Source/WebKit/UIProcess/gtk/AcceleratedBackingStoreX11.h 2019-07-18 13:37:07 UTC (rev 247562)
+++ trunk/Source/WebKit/UIProcess/gtk/AcceleratedBackingStoreX11.h 2019-07-18 14:42:48 UTC (rev 247563)
@@ -39,6 +39,7 @@
class AcceleratedBackingStoreX11 final : public AcceleratedBackingStore {
WTF_MAKE_NONCOPYABLE(AcceleratedBackingStoreX11); WTF_MAKE_FAST_ALLOCATED;
public:
+ static bool checkRequirements();
static std::unique_ptr<AcceleratedBackingStoreX11> create(WebPageProxy&);
~AcceleratedBackingStoreX11();
Modified: trunk/Source/WebKit/UIProcess/gtk/HardwareAccelerationManager.cpp (247562 => 247563)
--- trunk/Source/WebKit/UIProcess/gtk/HardwareAccelerationManager.cpp 2019-07-18 13:37:07 UTC (rev 247562)
+++ trunk/Source/WebKit/UIProcess/gtk/HardwareAccelerationManager.cpp 2019-07-18 14:42:48 UTC (rev 247563)
@@ -26,21 +26,8 @@
#include "config.h"
#include "HardwareAccelerationManager.h"
-#include <WebCore/NotImplemented.h>
-#include <WebCore/PlatformDisplay.h>
+#include "AcceleratedBackingStore.h"
-#if PLATFORM(X11)
-#include <WebCore/PlatformDisplayX11.h>
-#endif
-
-#if PLATFORM(WAYLAND)
-#if USE(WPE_RENDERER)
-#include <wpe/fdo-egl.h>
-#else
-#include "WaylandCompositor.h"
-#endif
-#endif
-
namespace WebKit {
using namespace WebCore;
@@ -65,33 +52,11 @@
return;
}
-#if PLATFORM(X11)
- if (PlatformDisplay::sharedDisplay().type() == PlatformDisplay::Type::X11) {
- auto& display = downcast<PlatformDisplayX11>(PlatformDisplay::sharedDisplay());
- Optional<int> damageBase, errorBase;
- if (!display.supportsXComposite() || !display.supportsXDamage(damageBase, errorBase)) {
- m_canUseHardwareAcceleration = false;
- return;
- }
+ if (!AcceleratedBackingStore::checkRequirements()) {
+ m_canUseHardwareAcceleration = false;
+ return;
}
-#endif
-#if PLATFORM(WAYLAND) && USE(EGL)
- if (PlatformDisplay::sharedDisplay().type() == PlatformDisplay::Type::Wayland) {
-#if USE(WPE_RENDERER)
- if (!wpe_fdo_initialize_for_egl_display(PlatformDisplay::sharedDisplay().eglDisplay())) {
- m_canUseHardwareAcceleration = false;
- return;
- }
-#else
- if (!WaylandCompositor::singleton().isRunning()) {
- m_canUseHardwareAcceleration = false;
- return;
- }
-#endif
- }
-#endif
-
const char* forceCompositing = getenv("WEBKIT_FORCE_COMPOSITING_MODE");
if (forceCompositing && strcmp(forceCompositing, "0"))
m_forceHardwareAcceleration = true;
