Title: [234887] trunk/Source
Revision
234887
Author
mcatanz...@igalia.com
Date
2018-08-15 08:17:55 -0700 (Wed, 15 Aug 2018)

Log Message

[WPE][GTK] WaylandCompositor fails to properly remove surface from its page map
https://bugs.webkit.org/show_bug.cgi?id=188520

Reviewed by Alex Christensen.

Source/WebKit:

willDestroySurface overwrites the surface pointer in the map's iterator in an attempt to
change the value of the surface pointer in the map, but it doesn't work because changing
the iterator does not change the map itself. There's no need to fix this function: it's
better to use WeakPtr instead.

* UIProcess/gtk/WaylandCompositor.cpp:
(WebKit::WaylandCompositor::getTexture):
(WebKit::WaylandCompositor::bindSurfaceToWebPage):
(WebKit::WaylandCompositor::unregisterWebPage):
(WebKit::WaylandCompositor::willDestroySurface): Deleted.
* UIProcess/gtk/WaylandCompositor.h:

Source/WTF:

Add a comment pointing to CanMakeWeakPtr, since it's useful and I very nearly missed it.

* wtf/WeakPtr.h:

Modified Paths

Diff

Modified: trunk/Source/WTF/ChangeLog (234886 => 234887)


--- trunk/Source/WTF/ChangeLog	2018-08-15 14:32:06 UTC (rev 234886)
+++ trunk/Source/WTF/ChangeLog	2018-08-15 15:17:55 UTC (rev 234887)
@@ -1,3 +1,14 @@
+2018-08-15  Michael Catanzaro  <mcatanz...@igalia.com>
+
+        [WPE][GTK] WaylandCompositor fails to properly remove surface from its page map
+        https://bugs.webkit.org/show_bug.cgi?id=188520
+
+        Reviewed by Alex Christensen.
+
+        Add a comment pointing to CanMakeWeakPtr, since it's useful and I very nearly missed it.
+
+        * wtf/WeakPtr.h:
+
 2018-08-14  Saam barati  <sbar...@apple.com>
 
         HashMap<Ref<P>, V> asserts when V is not zero for its empty value

Modified: trunk/Source/WTF/wtf/WeakPtr.h (234886 => 234887)


--- trunk/Source/WTF/wtf/WeakPtr.h	2018-08-15 14:32:06 UTC (rev 234886)
+++ trunk/Source/WTF/wtf/WeakPtr.h	2018-08-15 15:17:55 UTC (rev 234887)
@@ -90,6 +90,7 @@
     RefPtr<WeakReference<T>> m_ref;
 };
 
+// Note: you probably want to inherit from CanMakeWeakPtr rather than use this directly.
 template<typename T>
 class WeakPtrFactory {
     WTF_MAKE_NONCOPYABLE(WeakPtrFactory<T>);

Modified: trunk/Source/WebKit/ChangeLog (234886 => 234887)


--- trunk/Source/WebKit/ChangeLog	2018-08-15 14:32:06 UTC (rev 234886)
+++ trunk/Source/WebKit/ChangeLog	2018-08-15 15:17:55 UTC (rev 234887)
@@ -1,3 +1,22 @@
+2018-08-15  Michael Catanzaro  <mcatanz...@igalia.com>
+
+        [WPE][GTK] WaylandCompositor fails to properly remove surface from its page map
+        https://bugs.webkit.org/show_bug.cgi?id=188520
+
+        Reviewed by Alex Christensen.
+
+        willDestroySurface overwrites the surface pointer in the map's iterator in an attempt to
+        change the value of the surface pointer in the map, but it doesn't work because changing
+        the iterator does not change the map itself. There's no need to fix this function: it's
+        better to use WeakPtr instead.
+
+        * UIProcess/gtk/WaylandCompositor.cpp:
+        (WebKit::WaylandCompositor::getTexture):
+        (WebKit::WaylandCompositor::bindSurfaceToWebPage):
+        (WebKit::WaylandCompositor::unregisterWebPage):
+        (WebKit::WaylandCompositor::willDestroySurface): Deleted.
+        * UIProcess/gtk/WaylandCompositor.h:
+
 2018-08-15  Wenson Hsieh  <wenson_hs...@apple.com>
 
         [Attachment SPI] Remove attachment display mode options

Modified: trunk/Source/WebKit/UIProcess/gtk/WaylandCompositor.cpp (234886 => 234887)


--- trunk/Source/WebKit/UIProcess/gtk/WaylandCompositor.cpp	2018-08-15 14:32:06 UTC (rev 234886)
+++ trunk/Source/WebKit/UIProcess/gtk/WaylandCompositor.cpp	2018-08-15 15:17:55 UTC (rev 234887)
@@ -350,7 +350,6 @@
             wl_resource_set_implementation(surfaceResource, &surfaceInterface, new WaylandCompositor::Surface(),
                 [](struct wl_resource* resource) {
                     auto* surface = static_cast<WaylandCompositor::Surface*>(wl_resource_get_user_data(resource));
-                    WaylandCompositor::singleton().willDestroySurface(surface);
                     delete surface;
                 });
         } else
@@ -531,7 +530,7 @@
 
 bool WaylandCompositor::getTexture(WebPageProxy& webPage, unsigned& texture, IntSize& textureSize)
 {
-    if (auto* surface = m_pageMap.get(&webPage))
+    if (WeakPtr<Surface> surface = m_pageMap.get(&webPage))
         return surface->prepareTextureForPainting(texture, textureSize);
     return false;
 }
@@ -549,7 +548,7 @@
         return;
 
     surface->setWebPage(webPage);
-    m_pageMap.set(webPage, surface);
+    m_pageMap.set(webPage, makeWeakPtr(*surface));
 }
 
 void WaylandCompositor::registerWebPage(WebPageProxy& webPage)
@@ -559,20 +558,10 @@
 
 void WaylandCompositor::unregisterWebPage(WebPageProxy& webPage)
 {
-    if (auto* surface = m_pageMap.take(&webPage))
+    if (WeakPtr<Surface> surface = m_pageMap.take(&webPage))
         surface->setWebPage(nullptr);
 }
 
-void WaylandCompositor::willDestroySurface(Surface* surface)
-{
-    for (auto it : m_pageMap) {
-        if (it.value == surface) {
-            it.value = nullptr;
-            return;
-        }
-    }
-}
-
 } // namespace WebKit
 
 #endif // PLATFORM(WAYLAND) && USE(EGL)

Modified: trunk/Source/WebKit/UIProcess/gtk/WaylandCompositor.h (234886 => 234887)


--- trunk/Source/WebKit/UIProcess/gtk/WaylandCompositor.h	2018-08-15 14:32:06 UTC (rev 234886)
+++ trunk/Source/WebKit/UIProcess/gtk/WaylandCompositor.h	2018-08-15 15:17:55 UTC (rev 234887)
@@ -76,7 +76,7 @@
         uint32_t m_busyCount { 0 };
     };
 
-    class Surface {
+    class Surface : public CanMakeWeakPtr<Surface> {
         WTF_MAKE_NONCOPYABLE(Surface); WTF_MAKE_FAST_ALLOCATED;
     public:
         Surface();
@@ -111,7 +111,6 @@
     void bindSurfaceToWebPage(Surface*, uint64_t pageID);
     void registerWebPage(WebPageProxy&);
     void unregisterWebPage(WebPageProxy&);
-    void willDestroySurface(Surface*);
 
     bool getTexture(WebPageProxy&, unsigned&, WebCore::IntSize&);
 
@@ -131,7 +130,7 @@
     WebCore::WlUniquePtr<struct wl_global> m_webkitgtkGlobal;
     GRefPtr<GSource> m_eventSource;
     std::unique_ptr<WebCore::GLContext> m_eglContext;
-    HashMap<WebPageProxy*, Surface*> m_pageMap;
+    HashMap<WebPageProxy*, WeakPtr<Surface>> m_pageMap;
 };
 
 } // namespace WebKit
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to