[webkit-changes] [179586] trunk/Source/WebKit2

Tue, 03 Feb 2015 17:59:00 -0800

Title: [179586] trunk/Source/WebKit2
Revision
179586
Author
[email protected]
Date
2015-02-03 17:58:24 -0800 (Tue, 03 Feb 2015)

Log Message

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.

Modified Paths

Diff

Modified: trunk/Source/WebKit2/ChangeLog (179585 => 179586)


--- trunk/Source/WebKit2/ChangeLog	2015-02-04 01:26:13 UTC (rev 179585)
+++ trunk/Source/WebKit2/ChangeLog	2015-02-04 01:58:24 UTC (rev 179586)
@@ -1,3 +1,34 @@
+2015-02-03  Timothy Horton  <[email protected]>
+
+        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  <[email protected]>
 
         Drop ResourceLoadPriorityUnresolved resource load priority and use Optional<> instead

Modified: trunk/Source/WebKit2/WebProcess/InjectedBundle/API/c/WKBundlePageOverlay.cpp (179585 => 179586)


--- trunk/Source/WebKit2/WebProcess/InjectedBundle/API/c/WKBundlePageOverlay.cpp	2015-02-04 01:26:13 UTC (rev 179585)
+++ trunk/Source/WebKit2/WebProcess/InjectedBundle/API/c/WKBundlePageOverlay.cpp	2015-02-04 01:58:24 UTC (rev 179586)
@@ -224,9 +224,7 @@
 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)

Modified: trunk/Source/WebKit2/WebProcess/WebPage/WebPageOverlay.cpp (179585 => 179586)


--- trunk/Source/WebKit2/WebProcess/WebPage/WebPageOverlay.cpp	2015-02-04 01:26:13 UTC (rev 179585)
+++ trunk/Source/WebKit2/WebProcess/WebPage/WebPageOverlay.cpp	2015-02-04 01:58:24 UTC (rev 179586)
@@ -42,15 +42,16 @@
     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 @@
         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

Modified: trunk/Source/WebKit2/WebProcess/WebPage/WebPageOverlay.h (179585 => 179586)


--- trunk/Source/WebKit2/WebProcess/WebPage/WebPageOverlay.h	2015-02-04 01:26:13 UTC (rev 179585)
+++ trunk/Source/WebKit2/WebProcess/WebPage/WebPageOverlay.h	2015-02-04 01:58:24 UTC (rev 179586)
@@ -44,10 +44,9 @@
 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 @@
         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 @@
     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 @@
 #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 @@
 
 
     RefPtr<WebCore::PageOverlay> m_overlay;
-    Client& m_client;
+    std::unique_ptr<Client> m_client;
 };
 
 } // namespace WebKit

_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to