Every PageOverlayClientImpl leaks

https://bugs.webkit.org/show_bug.cgi?id=141224
<rdar://problem/19652939>

Reviewed by Simon Fraser.

* WebProcess/InjectedBundle/API/c/WKBundlePageOverlay.cpp:
(WKBundlePageOverlayCreate):
* WebProcess/WebPage/WebPageOverlay.cpp:
(WebKit::WebPageOverlay::create):
(WebKit::WebPageOverlay::WebPageOverlay):
(WebKit::WebPageOverlay::pageOverlayDestroyed):
(WebKit::WebPageOverlay::willMoveToPage):
(WebKit::WebPageOverlay::didMoveToPage):
(WebKit::WebPageOverlay::drawRect):
(WebKit::WebPageOverlay::mouseEvent):
(WebKit::WebPageOverlay::didScrollFrame):
(WebKit::WebPageOverlay::actionContextForResultAtPoint):
(WebKit::WebPageOverlay::dataDetectorsDidPresentUI):
(WebKit::WebPageOverlay::dataDetectorsDidChangeUI):
(WebKit::WebPageOverlay::dataDetectorsDidHideUI):
(WebKit::WebPageOverlay::copyAccessibilityAttributeStringValueForPoint):
(WebKit::WebPageOverlay::copyAccessibilityAttributeBoolValueForPoint):
(WebKit::WebPageOverlay::copyAccessibilityAttributeNames):
* WebProcess/WebPage/WebPageOverlay.h:
(WebKit::WebPageOverlay::client):
Keep the PageOverlayClientImpl as a unique_ptr instead of a leaked reference,
ensuring that it's cleaned up when the WebPageOverlay is torn down.

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

diff --git a/Source/WebKit2/ChangeLog b/Source/WebKit2/ChangeLog
index 0c98d1f2c4fd92e8bc53b2c279d4d58a1e1f6c59..807b5b435d82f068c955dd78c64d319773419e31 100644 (file)
--- a/Source/WebKit2/ChangeLog
+++ b/Source/WebKit2/ChangeLog
@@ -1,3 +1,34 @@
+2015-02-03  Timothy Horton  <timothy_horton@apple.com>
+
+        Every PageOverlayClientImpl leaks
+        https://bugs.webkit.org/show_bug.cgi?id=141224
+        <rdar://problem/19652939>
+
+        Reviewed by Simon Fraser.
+
+        * WebProcess/InjectedBundle/API/c/WKBundlePageOverlay.cpp:
+        (WKBundlePageOverlayCreate):
+        * WebProcess/WebPage/WebPageOverlay.cpp:
+        (WebKit::WebPageOverlay::create):
+        (WebKit::WebPageOverlay::WebPageOverlay):
+        (WebKit::WebPageOverlay::pageOverlayDestroyed):
+        (WebKit::WebPageOverlay::willMoveToPage):
+        (WebKit::WebPageOverlay::didMoveToPage):
+        (WebKit::WebPageOverlay::drawRect):
+        (WebKit::WebPageOverlay::mouseEvent):
+        (WebKit::WebPageOverlay::didScrollFrame):
+        (WebKit::WebPageOverlay::actionContextForResultAtPoint):
+        (WebKit::WebPageOverlay::dataDetectorsDidPresentUI):
+        (WebKit::WebPageOverlay::dataDetectorsDidChangeUI):
+        (WebKit::WebPageOverlay::dataDetectorsDidHideUI):
+        (WebKit::WebPageOverlay::copyAccessibilityAttributeStringValueForPoint):
+        (WebKit::WebPageOverlay::copyAccessibilityAttributeBoolValueForPoint):
+        (WebKit::WebPageOverlay::copyAccessibilityAttributeNames):
+        * WebProcess/WebPage/WebPageOverlay.h:
+        (WebKit::WebPageOverlay::client):
+        Keep the PageOverlayClientImpl as a unique_ptr instead of a leaked reference,
+        ensuring that it's cleaned up when the WebPageOverlay is torn down.
+
 2015-02-03  Chris Dumez  <cdumez@apple.com>
 
         Drop ResourceLoadPriorityUnresolved resource load priority and use Optional<> instead

diff --git a/Source/WebKit2/WebProcess/InjectedBundle/API/c/WKBundlePageOverlay.cpp b/Source/WebKit2/WebProcess/InjectedBundle/API/c/WKBundlePageOverlay.cpp
index 57c5954b6f9e26e2d7f56cc30fb8c56ecd7d481d..ae45aaf29dd6113d08852d39eb8013232d473d30 100644 (file)
--- a/Source/WebKit2/WebProcess/InjectedBundle/API/c/WKBundlePageOverlay.cpp
+++ b/Source/WebKit2/WebProcess/InjectedBundle/API/c/WKBundlePageOverlay.cpp
@@ -224,9 +224,7 @@ WKTypeID WKBundlePageOverlayGetTypeID()
 WKBundlePageOverlayRef WKBundlePageOverlayCreate(WKBundlePageOverlayClientBase* wkClient)
 {
     auto clientImpl = std::make_unique<PageOverlayClientImpl>(wkClient);
-
-    // FIXME: Looks like this leaks the clientImpl.
-    return toAPI(WebPageOverlay::create(*clientImpl.release()).leakRef());
+    return toAPI(WebPageOverlay::create(WTF::move(clientImpl)).leakRef());
 }
 
 void WKBundlePageOverlaySetAccessibilityClient(WKBundlePageOverlayRef bundlePageOverlayRef, WKBundlePageOverlayAccessibilityClientBase* client)

diff --git a/Source/WebKit2/WebProcess/WebPage/WebPageOverlay.cpp b/Source/WebKit2/WebProcess/WebPage/WebPageOverlay.cpp
index db9865cb552c609810551cc97a7608dec11b8f44..9e4086e9c232666b2ab1daf11dd35ad6dd9f8d23 100644 (file)
--- a/Source/WebKit2/WebProcess/WebPage/WebPageOverlay.cpp
+++ b/Source/WebKit2/WebProcess/WebPage/WebPageOverlay.cpp
@@ -42,15 +42,16 @@ static HashMap<PageOverlay*, WebPageOverlay*>& overlayMap()
     return map;
 }
 
-PassRefPtr<WebPageOverlay> WebPageOverlay::create(WebPageOverlay::Client& client, PageOverlay::OverlayType overlayType)
+PassRefPtr<WebPageOverlay> WebPageOverlay::create(std::unique_ptr<WebPageOverlay::Client> client, PageOverlay::OverlayType overlayType)
 {
-    return adoptRef(new WebPageOverlay(client, overlayType));
+    return adoptRef(new WebPageOverlay(WTF::move(client), overlayType));
 }
 
-WebPageOverlay::WebPageOverlay(WebPageOverlay::Client& client, PageOverlay::OverlayType overlayType)
+WebPageOverlay::WebPageOverlay(std::unique_ptr<WebPageOverlay::Client> client, PageOverlay::OverlayType overlayType)
     : m_overlay(PageOverlay::create(*this, overlayType))
-    , m_client(client)
+    , m_client(WTF::move(client))
 {
+    ASSERT(m_client);
     overlayMap().add(m_overlay.get(), this);
 }
 
@@ -90,69 +91,69 @@ void WebPageOverlay::pageOverlayDestroyed(PageOverlay&)
         m_overlay = nullptr;
     }
 
-    m_client.pageOverlayDestroyed(*this);
+    m_client->pageOverlayDestroyed(*this);
 }
 
 void WebPageOverlay::willMoveToPage(PageOverlay&, Page* page)
 {
-    m_client.willMoveToPage(*this, page ? WebPage::fromCorePage(page) : nullptr);
+    m_client->willMoveToPage(*this, page ? WebPage::fromCorePage(page) : nullptr);
 }
 
 void WebPageOverlay::didMoveToPage(PageOverlay&, Page* page)
 {
-    m_client.didMoveToPage(*this, page ? WebPage::fromCorePage(page) : nullptr);
+    m_client->didMoveToPage(*this, page ? WebPage::fromCorePage(page) : nullptr);
 }
 
 void WebPageOverlay::drawRect(PageOverlay&, GraphicsContext& context, const IntRect& dirtyRect)
 {
-    m_client.drawRect(*this, context, dirtyRect);
+    m_client->drawRect(*this, context, dirtyRect);
 }
 
 bool WebPageOverlay::mouseEvent(PageOverlay&, const PlatformMouseEvent& event)
 {
-    return m_client.mouseEvent(*this, event);
+    return m_client->mouseEvent(*this, event);
 }
 
 void WebPageOverlay::didScrollFrame(PageOverlay&, Frame& frame)
 {
-    m_client.didScrollFrame(*this, WebFrame::fromCoreFrame(frame));
+    m_client->didScrollFrame(*this, WebFrame::fromCoreFrame(frame));
 }
 
 #if PLATFORM(MAC)
 DDActionContext *WebPageOverlay::actionContextForResultAtPoint(FloatPoint location, RefPtr<WebCore::Range>& rangeHandle)
 {
-    return m_client.actionContextForResultAtPoint(*this, location, rangeHandle);
+    return m_client->actionContextForResultAtPoint(*this, location, rangeHandle);
 }
 
 void WebPageOverlay::dataDetectorsDidPresentUI()
 {
-    m_client.dataDetectorsDidPresentUI(*this);
+    m_client->dataDetectorsDidPresentUI(*this);
 }
 
 void WebPageOverlay::dataDetectorsDidChangeUI()
 {
-    m_client.dataDetectorsDidChangeUI(*this);
+    m_client->dataDetectorsDidChangeUI(*this);
 }
 
 void WebPageOverlay::dataDetectorsDidHideUI()
 {
-    m_client.dataDetectorsDidHideUI(*this);
+    m_client->dataDetectorsDidHideUI(*this);
 }
 #endif // PLATFORM(MAC)
 
 bool WebPageOverlay::copyAccessibilityAttributeStringValueForPoint(PageOverlay&, String attribute, FloatPoint parameter, String& value)
 {
-    return m_client.copyAccessibilityAttributeStringValueForPoint(*this, attribute, parameter, value);
+    return m_client->copyAccessibilityAttributeStringValueForPoint(*this, attribute, parameter, value);
 }
 
 bool WebPageOverlay::copyAccessibilityAttributeBoolValueForPoint(PageOverlay&, String attribute, FloatPoint parameter, bool& value)
 {
-    return m_client.copyAccessibilityAttributeBoolValueForPoint(*this, attribute, parameter, value);
+    return m_client->copyAccessibilityAttributeBoolValueForPoint(*this, attribute, parameter, value);
 }
 
 Vector<String> WebPageOverlay::copyAccessibilityAttributeNames(PageOverlay&, bool parameterizedNames)
 {
-    return m_client.copyAccessibilityAttributeNames(*this, parameterizedNames);
+    return m_client->copyAccessibilityAttributeNames(*this, parameterizedNames);
 }
 
 } // namespace WebKit

diff --git a/Source/WebKit2/WebProcess/WebPage/WebPageOverlay.h b/Source/WebKit2/WebProcess/WebPage/WebPageOverlay.h
index 576f5a9757f735f7bf8753dc4af38aac1566b08e..fbbe62a2a2851f36896f6f0627798a8a6607f1a9 100644 (file)
--- a/Source/WebKit2/WebProcess/WebPage/WebPageOverlay.h
+++ b/Source/WebKit2/WebProcess/WebPage/WebPageOverlay.h
@@ -44,10 +44,9 @@ class WebPage;
 class WebPageOverlay : public API::ObjectImpl<API::Object::Type::BundlePageOverlay>, private WebCore::PageOverlay::Client {
 public:
     class Client {
-    protected:
+    public:
         virtual ~Client() { }
 
-    public:
         virtual void pageOverlayDestroyed(WebPageOverlay&) = 0;
         virtual void willMoveToPage(WebPageOverlay&, WebPage*) = 0;
         virtual void didMoveToPage(WebPageOverlay&, WebPage*) = 0;
@@ -67,7 +66,7 @@ public:
         virtual Vector<String> copyAccessibilityAttributeNames(WebPageOverlay&, bool /* parameterizedNames */) { return Vector<String>(); }
     };
 
-    static PassRefPtr<WebPageOverlay> create(Client&, WebCore::PageOverlay::OverlayType = WebCore::PageOverlay::OverlayType::View);
+    static PassRefPtr<WebPageOverlay> create(std::unique_ptr<Client>, WebCore::PageOverlay::OverlayType = WebCore::PageOverlay::OverlayType::View);
     static WebPageOverlay* fromCoreOverlay(WebCore::PageOverlay&);
     virtual ~WebPageOverlay();
 
@@ -77,7 +76,7 @@ public:
     void clear();
 
     WebCore::PageOverlay* coreOverlay() const { return m_overlay.get(); }
-    Client& client() const { return m_client; }
+    Client& client() const { return *m_client; }
 
 #if PLATFORM(MAC)
     DDActionContext *actionContextForResultAtPoint(WebCore::FloatPoint, RefPtr<WebCore::Range>&);
@@ -87,7 +86,7 @@ public:
 #endif
 
 private:
-    WebPageOverlay(Client&, WebCore::PageOverlay::OverlayType);
+    WebPageOverlay(std::unique_ptr<Client>, WebCore::PageOverlay::OverlayType);
 
     // WebCore::PageOverlay::Client
     virtual void pageOverlayDestroyed(WebCore::PageOverlay&) override;
@@ -103,7 +102,7 @@ private:
 
 
     RefPtr<WebCore::PageOverlay> m_overlay;
-    Client& m_client;
+    std::unique_ptr<Client> m_client;
 };
 
 } // namespace WebKit