REGRESSION(r191402?): Safari, Mail crash at com.apple.WebKit: WebKit::WebContextMenuL...
authorantti@apple.com <antti@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 30 May 2017 14:02:48 +0000 (14:02 +0000)
committerantti@apple.com <antti@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 30 May 2017 14:02:48 +0000 (14:02 +0000)
https://bugs.webkit.org/show_bug.cgi?id=172704

Reviewed by Andreas Kling.

r191402 made WebContextMenuProxy non-refcounted. However there are several potential ways for WebContextMenuProxyMac::show()
to re-enter WebPageProxy and delete itself. This patch partially reverts r191402 bringing refcounting back and protects
WebContextMenuProxy during show().

Speculative fix. No test, can't repro the crash.

* UIProcess/PageClient.h:
* UIProcess/WebContextMenuProxy.h:
* UIProcess/WebPageProxy.cpp:
(WebKit::WebPageProxy::internalShowContextMenu):

    Protect WebContextMenuProxy during show().

* UIProcess/WebPageProxy.h:
* UIProcess/gtk/WebContextMenuProxyGtk.h:
(WebKit::WebContextMenuProxyGtk::create):
* UIProcess/ios/PageClientImplIOS.h:
* UIProcess/ios/PageClientImplIOS.mm:
(WebKit::PageClientImpl::createContextMenuProxy):
* UIProcess/mac/PageClientImpl.h:
* UIProcess/mac/PageClientImpl.mm:
(WebKit::PageClientImpl::createContextMenuProxy):
* UIProcess/mac/WebContextMenuProxyMac.h:
(WebKit::WebContextMenuProxyMac::create):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@217552 268f45cc-cd09-0410-ab3c-d52691b4dbfc

15 files changed:
Source/WebKit2/ChangeLog
Source/WebKit2/UIProcess/API/gtk/PageClientImpl.cpp
Source/WebKit2/UIProcess/API/gtk/PageClientImpl.h
Source/WebKit2/UIProcess/API/wpe/PageClientImpl.cpp
Source/WebKit2/UIProcess/API/wpe/PageClientImpl.h
Source/WebKit2/UIProcess/PageClient.h
Source/WebKit2/UIProcess/WebContextMenuProxy.h
Source/WebKit2/UIProcess/WebPageProxy.cpp
Source/WebKit2/UIProcess/WebPageProxy.h
Source/WebKit2/UIProcess/gtk/WebContextMenuProxyGtk.h
Source/WebKit2/UIProcess/ios/PageClientImplIOS.h
Source/WebKit2/UIProcess/ios/PageClientImplIOS.mm
Source/WebKit2/UIProcess/mac/PageClientImpl.h
Source/WebKit2/UIProcess/mac/PageClientImpl.mm
Source/WebKit2/UIProcess/mac/WebContextMenuProxyMac.h

index 716c3ae..a9b0b70 100644 (file)
@@ -1,3 +1,35 @@
+2017-05-30  Antti Koivisto  <antti@apple.com>
+
+        REGRESSION(r191402?): Safari, Mail crash at com.apple.WebKit: WebKit::WebContextMenuListenerProxy::invalidate + 4
+        https://bugs.webkit.org/show_bug.cgi?id=172704
+
+        Reviewed by Andreas Kling.
+
+        r191402 made WebContextMenuProxy non-refcounted. However there are several potential ways for WebContextMenuProxyMac::show()
+        to re-enter WebPageProxy and delete itself. This patch partially reverts r191402 bringing refcounting back and protects
+        WebContextMenuProxy during show().
+
+        Speculative fix. No test, can't repro the crash.
+
+        * UIProcess/PageClient.h:
+        * UIProcess/WebContextMenuProxy.h:
+        * UIProcess/WebPageProxy.cpp:
+        (WebKit::WebPageProxy::internalShowContextMenu):
+
+            Protect WebContextMenuProxy during show().
+
+        * UIProcess/WebPageProxy.h:
+        * UIProcess/gtk/WebContextMenuProxyGtk.h:
+        (WebKit::WebContextMenuProxyGtk::create):
+        * UIProcess/ios/PageClientImplIOS.h:
+        * UIProcess/ios/PageClientImplIOS.mm:
+        (WebKit::PageClientImpl::createContextMenuProxy):
+        * UIProcess/mac/PageClientImpl.h:
+        * UIProcess/mac/PageClientImpl.mm:
+        (WebKit::PageClientImpl::createContextMenuProxy):
+        * UIProcess/mac/WebContextMenuProxyMac.h:
+        (WebKit::WebContextMenuProxyMac::create):
+
 2017-05-30  Zan Dobersek  <zdobersek@igalia.com>
 
         Invalidate the LayerTreeHost when destroying the DrawingAreaWPE object.
index 98e0673..b6fb9b8 100644 (file)
@@ -209,9 +209,9 @@ RefPtr<WebPopupMenuProxy> PageClientImpl::createPopupMenuProxy(WebPageProxy& pag
     return WebPopupMenuProxyGtk::create(m_viewWidget, page);
 }
 
-std::unique_ptr<WebContextMenuProxy> PageClientImpl::createContextMenuProxy(WebPageProxy& page, const ContextMenuContextData& context, const UserData& userData)
+RefPtr<WebContextMenuProxy> PageClientImpl::createContextMenuProxy(WebPageProxy& page, const ContextMenuContextData& context, const UserData& userData)
 {
-    return std::make_unique<WebContextMenuProxyGtk>(m_viewWidget, page, context, userData);
+    return WebContextMenuProxyGtk::create(m_viewWidget, page, context, userData);
 }
 
 RefPtr<WebColorPicker> PageClientImpl::createColorPicker(WebPageProxy* page, const WebCore::Color& color, const WebCore::IntRect& rect)
index a01bf07..88ee6e0 100644 (file)
@@ -81,7 +81,7 @@ private:
     WebCore::IntRect rootViewToScreen(const WebCore::IntRect&) override;
     void doneWithKeyEvent(const NativeWebKeyboardEvent&, bool wasEventHandled) override;
     RefPtr<WebPopupMenuProxy> createPopupMenuProxy(WebPageProxy&) override;
-    std::unique_ptr<WebContextMenuProxy> createContextMenuProxy(WebPageProxy&, const ContextMenuContextData&, const UserData&) override;
+    RefPtr<WebContextMenuProxy> createContextMenuProxy(WebPageProxy&, const ContextMenuContextData&, const UserData&) override;
 #if ENABLE(INPUT_TYPE_COLOR)
     RefPtr<WebColorPicker> createColorPicker(WebPageProxy*, const WebCore::Color& intialColor, const WebCore::IntRect&) override;
 #endif
index 3163949..1f70e36 100644 (file)
@@ -228,7 +228,7 @@ RefPtr<WebPopupMenuProxy> PageClientImpl::createPopupMenuProxy(WebPageProxy&)
 }
 
 #if ENABLE(CONTEXT_MENUS)
-std::unique_ptr<WebContextMenuProxy> PageClientImpl::createContextMenuProxy(WebPageProxy&, const ContextMenuContextData&, const UserData&)
+RefPtr<WebContextMenuProxy> PageClientImpl::createContextMenuProxy(WebPageProxy&, const ContextMenuContextData&, const UserData&)
 {
     return nullptr;
 }
index d1456be..e07d29a 100644 (file)
@@ -83,7 +83,7 @@ private:
 
     RefPtr<WebPopupMenuProxy> createPopupMenuProxy(WebPageProxy&) override;
 #if ENABLE(CONTEXT_MENUS)
-    std::unique_ptr<WebContextMenuProxy> createContextMenuProxy(WebPageProxy&, const ContextMenuContextData&, const UserData&) override;
+    RefPtr<WebContextMenuProxy> createContextMenuProxy(WebPageProxy&, const ContextMenuContextData&, const UserData&) override;
 #endif
 
     void enterAcceleratedCompositingMode(const LayerTreeContext&) override;
index 3db7ebe..ae603b9 100644 (file)
@@ -218,7 +218,7 @@ public:
 
     virtual RefPtr<WebPopupMenuProxy> createPopupMenuProxy(WebPageProxy&) = 0;
 #if ENABLE(CONTEXT_MENUS)
-    virtual std::unique_ptr<WebContextMenuProxy> createContextMenuProxy(WebPageProxy&, const ContextMenuContextData&, const UserData&) = 0;
+    virtual RefPtr<WebContextMenuProxy> createContextMenuProxy(WebPageProxy&, const ContextMenuContextData&, const UserData&) = 0;
 #endif
 
 #if ENABLE(INPUT_TYPE_COLOR)
index f45a504..2415db1 100644 (file)
@@ -23,8 +23,7 @@
  * THE POSSIBILITY OF SUCH DAMAGE.
  */
 
-#ifndef WebContextMenuProxy_h
-#define WebContextMenuProxy_h
+#pragma once
 
 #if ENABLE(CONTEXT_MENUS)
 
@@ -36,7 +35,7 @@ namespace WebKit {
 
 class WebContextMenuItem;
 
-class WebContextMenuProxy {
+class WebContextMenuProxy : public RefCounted<WebContextMenuProxy> {
 public:
     virtual ~WebContextMenuProxy();
 
@@ -54,5 +53,3 @@ protected:
 } // namespace WebKit
 
 #endif
-
-#endif // WebPopupMenuProxy_h
index 1339845..c04d4ee 100644 (file)
@@ -4594,6 +4594,8 @@ void WebPageProxy::internalShowContextMenu(const ContextMenuContextData& context
     // Since showContextMenu() can spin a nested run loop we need to turn off the responsiveness timer.
     m_process->responsivenessTimer().stop();
 
+    // m_activeContextMenu might get cleared if WebPageProxy code is re-entered from the menu runloop or delegates.
+    Ref<WebContextMenuProxy> protector(*m_activeContextMenu);
     m_activeContextMenu->show();
 }
 
index a736914..a8ee3ae 100644 (file)
@@ -1685,7 +1685,7 @@ private:
 
     RefPtr<WebPopupMenuProxy> m_activePopupMenu;
 #if ENABLE(CONTEXT_MENUS)
-    std::unique_ptr<WebContextMenuProxy> m_activeContextMenu;
+    RefPtr<WebContextMenuProxy> m_activeContextMenu;
     ContextMenuContextData m_activeContextMenuContextData;
 #endif
     RefPtr<API::HitTestResult> m_lastMouseMoveHitTestResult;
index 144be98..9f46b5d 100644 (file)
@@ -44,13 +44,17 @@ class WebPageProxy;
 
 class WebContextMenuProxyGtk : public WebContextMenuProxy {
 public:
-    WebContextMenuProxyGtk(GtkWidget*, WebPageProxy&, const ContextMenuContextData&, const UserData&);
+    static auto create(GtkWidget* widget, WebPageProxy& page, const ContextMenuContextData& context, const UserData& userData)
+    {
+        return adoptRef(*new WebContextMenuProxyGtk(widget, page, context, userData));
+    }
     ~WebContextMenuProxyGtk();
 
     void populate(const Vector<WebContextMenuItemGtk>&);
     GtkMenu* gtkMenu() const { return m_menu; }
 
 private:
+    WebContextMenuProxyGtk(GtkWidget*, WebPageProxy&, const ContextMenuContextData&, const UserData&);
     void show() override;
     void showContextMenuWithItems(const Vector<WebContextMenuItemData>&) override;
     void append(GMenu*, const WebContextMenuItemGtk&);
index 7ff0608..6bbea2a 100644 (file)
@@ -96,7 +96,7 @@ private:
 #endif
     RefPtr<WebPopupMenuProxy> createPopupMenuProxy(WebPageProxy&) override;
 #if ENABLE(CONTEXT_MENUS)
-    std::unique_ptr<WebContextMenuProxy> createContextMenuProxy(WebPageProxy&, const ContextMenuContextData&, const UserData&) override;
+    RefPtr<WebContextMenuProxy> createContextMenuProxy(WebPageProxy&, const ContextMenuContextData&, const UserData&) override;
 #endif
     Ref<WebCore::ValidationBubble> createValidationBubble(const String& message, const WebCore::ValidationBubble::Settings&) final;
 
index c005c63..b5f65b6 100644 (file)
@@ -451,7 +451,7 @@ RefPtr<WebPopupMenuProxy> PageClientImpl::createPopupMenuProxy(WebPageProxy&)
 }
 
 #if ENABLE(CONTEXT_MENUS)
-std::unique_ptr<WebContextMenuProxy> PageClientImpl::createContextMenuProxy(WebPageProxy&, const UserData&)
+RefPtr<WebContextMenuProxy> PageClientImpl::createContextMenuProxy(WebPageProxy&, const UserData&)
 {
     return nullptr;
 }
index 0d997c2..4204369 100644 (file)
@@ -125,7 +125,7 @@ private:
 
     RefPtr<WebPopupMenuProxy> createPopupMenuProxy(WebPageProxy&) override;
 #if ENABLE(CONTEXT_MENUS)
-    std::unique_ptr<WebContextMenuProxy> createContextMenuProxy(WebPageProxy&, const ContextMenuContextData&, const UserData&) override;
+    RefPtr<WebContextMenuProxy> createContextMenuProxy(WebPageProxy&, const ContextMenuContextData&, const UserData&) override;
 #endif
 
 #if ENABLE(INPUT_TYPE_COLOR)
index da8505e..1835c95 100644 (file)
@@ -435,9 +435,9 @@ RefPtr<WebPopupMenuProxy> PageClientImpl::createPopupMenuProxy(WebPageProxy& pag
 }
 
 #if ENABLE(CONTEXT_MENUS)
-std::unique_ptr<WebContextMenuProxy> PageClientImpl::createContextMenuProxy(WebPageProxy& page, const ContextMenuContextData& context, const UserData& userData)
+RefPtr<WebContextMenuProxy> PageClientImpl::createContextMenuProxy(WebPageProxy& page, const ContextMenuContextData& context, const UserData& userData)
 {
-    return std::make_unique<WebContextMenuProxyMac>(m_view, page, context, userData);
+    return WebContextMenuProxyMac::create(m_view, page, context, userData);
 }
 #endif
 
index a651d12..77b4f56 100644 (file)
@@ -23,8 +23,7 @@
  * THE POSSIBILITY OF SUCH DAMAGE.
  */
 
-#ifndef WebContextMenuProxyMac_h
-#define WebContextMenuProxyMac_h
+#pragma once
 
 #if PLATFORM(MAC)
 
@@ -48,7 +47,10 @@ class WebPageProxy;
 
 class WebContextMenuProxyMac : public WebContextMenuProxy {
 public:
-    WebContextMenuProxyMac(NSView*, WebPageProxy&, const ContextMenuContextData&, const UserData&);
+    static auto create(NSView* view, WebPageProxy& page, const ContextMenuContextData& context, const UserData& userData)
+    {
+        return adoptRef(*new WebContextMenuProxyMac(view, page, context, userData));
+    }
     ~WebContextMenuProxyMac();
 
     void contextMenuItemSelected(const WebContextMenuItemData&);
@@ -62,6 +64,7 @@ public:
     NSWindow *window() const;
 
 private:
+    WebContextMenuProxyMac(NSView*, WebPageProxy&, const ContextMenuContextData&, const UserData&);
     void show() override;
 
     RefPtr<WebContextMenuListenerProxy> m_contextMenuListener;
@@ -84,5 +87,3 @@ private:
 } // namespace WebKit
 
 #endif // PLATFORM(MAC)
-
-#endif // WebContextMenuProxyMac_h