Remove unnecessary PageOverlay client function pageOverlayDestroyed
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 20 May 2016 21:17:35 +0000 (21:17 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 20 May 2016 21:17:35 +0000 (21:17 +0000)
https://bugs.webkit.org/show_bug.cgi?id=157388
<rdar://problem/25471523>

Patch by John Wilander <wilander@apple.com> on 2016-05-20
Reviewed by Tim Horton.

Remove dead PageOverlay code. Almost all of these overrides were empty and
never called. In the case of WebPageOverlay it was never called but had a
function body, causing confusion. There was a fear of dangling pointers in
WebPageOverlay's static hash map between PageOverlays and WebPageOverlays.
Only WebPageOverlay's constructor creates its PageOverlay object and adds it
to the hash map. Its client object is kept in a unique pointer member which
is automatically deleted when the WebPageOverlay object itself is deleted.
This explains why PageOverlayClientImpl::pageOverlayDestroyed in
WKBundlePageOverlay can safely be removed. Finally, WebPageOverlay's
destructor clears the hash map entry for its PageOverlay object. Thus, there
is no need to call WebPageOverlay::pageOverlayDestroyed nor a need for
WebPageOverlay's destructor to call pageOverlayDestroyed on its client.

No new tests. I tried to come up with a WebKit API test for this but I
wasn't able to test presence/absence of WebPageOverlay's map entries since
the map is not exposed.

Source/WebCore:

* page/DebugPageOverlays.cpp:
(WebCore::RegionOverlay::pageOverlayDestroyed): Deleted.
* page/PageOverlay.h:
(WebCore::PageOverlay::Client::pageOverlayDestroyed): Deleted.
* page/ResourceUsageOverlay.h:
(WebCore::ResourceUsageOverlay::pageOverlayDestroyed): Deleted.
* page/mac/ServicesOverlayController.h:
* page/mac/ServicesOverlayController.mm:
(WebCore::ServicesOverlayController::pageOverlayDestroyed): Deleted.
* testing/MockPageOverlayClient.cpp:
* testing/MockPageOverlayClient.h:
(WebCore::MockPageOverlayClient::pageOverlayDestroyed): Deleted.

Source/WebKit2:

* WebProcess/InjectedBundle/API/c/WKBundlePageOverlay.cpp:
(WebKit::PageOverlayClientImpl::pageOverlayDestroyed): Deleted.
* WebProcess/Plugins/PDF/PDFPlugin.h:
* WebProcess/Plugins/PDF/PDFPlugin.mm:
(WebKit::PDFPlugin::HUD::pageOverlayDestroyed): Deleted.
* WebProcess/WebCoreSupport/WebInspectorClient.cpp:
* WebProcess/WebCoreSupport/WebInspectorClient.h:
(WebKit::WebInspectorClient::pageOverlayDestroyed): Deleted.
* WebProcess/WebPage/FindController.cpp:
* WebProcess/WebPage/FindController.h:
(WebKit::FindController::pageOverlayDestroyed): Deleted.
* WebProcess/WebPage/WebPageOverlay.cpp:
* WebProcess/WebPage/WebPageOverlay.h:
(WebKit::WebPageOverlay::pageOverlayDestroyed): Deleted.
* WebProcess/WebPage/ios/FindIndicatorOverlayClientIOS.h:
(WebKit::FindIndicatorOverlayClientIOS::pageOverlayDestroyed): Deleted.

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

19 files changed:
Source/WebCore/ChangeLog
Source/WebCore/page/DebugPageOverlays.cpp
Source/WebCore/page/PageOverlay.h
Source/WebCore/page/ResourceUsageOverlay.h
Source/WebCore/page/mac/ServicesOverlayController.h
Source/WebCore/page/mac/ServicesOverlayController.mm
Source/WebCore/testing/MockPageOverlayClient.cpp
Source/WebCore/testing/MockPageOverlayClient.h
Source/WebKit2/ChangeLog
Source/WebKit2/WebProcess/InjectedBundle/API/c/WKBundlePageOverlay.cpp
Source/WebKit2/WebProcess/Plugins/PDF/PDFPlugin.h
Source/WebKit2/WebProcess/Plugins/PDF/PDFPlugin.mm
Source/WebKit2/WebProcess/WebCoreSupport/WebInspectorClient.cpp
Source/WebKit2/WebProcess/WebCoreSupport/WebInspectorClient.h
Source/WebKit2/WebProcess/WebPage/FindController.cpp
Source/WebKit2/WebProcess/WebPage/FindController.h
Source/WebKit2/WebProcess/WebPage/WebPageOverlay.cpp
Source/WebKit2/WebProcess/WebPage/WebPageOverlay.h
Source/WebKit2/WebProcess/WebPage/ios/FindIndicatorOverlayClientIOS.h

index 80d0b4f..38eda89 100644 (file)
@@ -1,3 +1,41 @@
+2016-05-20  John Wilander  <wilander@apple.com>
+
+        Remove unnecessary PageOverlay client function pageOverlayDestroyed
+        https://bugs.webkit.org/show_bug.cgi?id=157388
+        <rdar://problem/25471523>
+
+        Reviewed by Tim Horton.
+
+        Remove dead PageOverlay code. Almost all of these overrides were empty and
+        never called. In the case of WebPageOverlay it was never called but had a
+        function body, causing confusion. There was a fear of dangling pointers in
+        WebPageOverlay's static hash map between PageOverlays and WebPageOverlays.
+        Only WebPageOverlay's constructor creates its PageOverlay object and adds it
+        to the hash map. Its client object is kept in a unique pointer member which
+        is automatically deleted when the WebPageOverlay object itself is deleted.
+        This explains why PageOverlayClientImpl::pageOverlayDestroyed in
+        WKBundlePageOverlay can safely be removed. Finally, WebPageOverlay's
+        destructor clears the hash map entry for its PageOverlay object. Thus, there
+        is no need to call WebPageOverlay::pageOverlayDestroyed nor a need for
+        WebPageOverlay's destructor to call pageOverlayDestroyed on its client.
+
+        No new tests. I tried to come up with a WebKit API test for this but I
+        wasn't able to test presence/absence of WebPageOverlay's map entries since
+        the map is not exposed.
+
+        * page/DebugPageOverlays.cpp:
+        (WebCore::RegionOverlay::pageOverlayDestroyed): Deleted.
+        * page/PageOverlay.h:
+        (WebCore::PageOverlay::Client::pageOverlayDestroyed): Deleted.
+        * page/ResourceUsageOverlay.h:
+        (WebCore::ResourceUsageOverlay::pageOverlayDestroyed): Deleted.
+        * page/mac/ServicesOverlayController.h:
+        * page/mac/ServicesOverlayController.mm:
+        (WebCore::ServicesOverlayController::pageOverlayDestroyed): Deleted.
+        * testing/MockPageOverlayClient.cpp:
+        * testing/MockPageOverlayClient.h:
+        (WebCore::MockPageOverlayClient::pageOverlayDestroyed): Deleted.
+
 2016-05-20  Alex Christensen  <achristensen@webkit.org>
 
         Fix null dereferencing in CSSAnimationTriggerScrollValue::equals
index ae7ab22..1447fc8 100644 (file)
@@ -54,7 +54,6 @@ protected:
     RegionOverlay(MainFrame&, Color);
 
 private:
-    void pageOverlayDestroyed(PageOverlay&) final;
     void willMoveToPage(PageOverlay&, Page*) final;
     void didMoveToPage(PageOverlay&, Page*) final;
     void drawRect(PageOverlay&, GraphicsContext&, const IntRect& dirtyRect) final;
@@ -166,10 +165,6 @@ RegionOverlay::~RegionOverlay()
         m_frame.pageOverlayController().uninstallPageOverlay(m_overlay.get(), PageOverlay::FadeMode::DoNotFade);
 }
 
-void RegionOverlay::pageOverlayDestroyed(PageOverlay&)
-{
-}
-
 void RegionOverlay::willMoveToPage(PageOverlay&, Page* page)
 {
     if (!page)
index 3ce0e46..4feedb5 100644 (file)
@@ -52,7 +52,6 @@ public:
         virtual ~Client() { }
     
     public:
-        virtual void pageOverlayDestroyed(PageOverlay&) = 0;
         virtual void willMoveToPage(PageOverlay&, Page*) = 0;
         virtual void didMoveToPage(PageOverlay&, Page*) = 0;
         virtual void drawRect(PageOverlay&, GraphicsContext&, const IntRect& dirtyRect) = 0;
index e039dee..cc11b57 100644 (file)
@@ -62,7 +62,6 @@ public:
     static const int normalHeight = 160;
 
 private:
-    void pageOverlayDestroyed(PageOverlay&) override { }
     void willMoveToPage(PageOverlay&, Page*) override { }
     void didMoveToPage(PageOverlay&, Page*) override { }
     void drawRect(PageOverlay&, GraphicsContext&, const IntRect&) override { }
index 18f9cb2..2e4661d 100644 (file)
@@ -97,7 +97,6 @@ private:
     };
 
     // PageOverlay::Client
-    void pageOverlayDestroyed(PageOverlay&) override;
     void willMoveToPage(PageOverlay&, Page*) override;
     void didMoveToPage(PageOverlay&, Page*) override;
     void drawRect(PageOverlay&, GraphicsContext&, const IntRect& dirtyRect) override;
index 135940c..bcbb56a 100644 (file)
@@ -221,13 +221,6 @@ ServicesOverlayController::~ServicesOverlayController()
         highlight->invalidate();
 }
 
-void ServicesOverlayController::pageOverlayDestroyed(PageOverlay&)
-{
-    // Before the overlay is destroyed, it should have moved out of the Page,
-    // at which point we already cleared our back pointer.
-    ASSERT(!m_servicesOverlay);
-}
-
 void ServicesOverlayController::willMoveToPage(PageOverlay&, Page* page)
 {
     if (page)
index d2f9ee2..ac4de61 100644 (file)
@@ -73,19 +73,6 @@ String MockPageOverlayClient::layerTreeAsText(MainFrame& mainFrame)
     return "View-relative:\n" + mainFrame.pageOverlayController().viewOverlayRootLayer().layerTreeAsText(LayerTreeAsTextIncludePageOverlayLayers) + "\n\nDocument-relative:\n" + mainFrame.pageOverlayController().documentOverlayRootLayer().layerTreeAsText(LayerTreeAsTextIncludePageOverlayLayers);
 }
 
-void MockPageOverlayClient::pageOverlayDestroyed(PageOverlay& overlay)
-{
-    // FIXME: This is dead code, nothing ever calls this function. It's not clear to me what the intention was,
-    // since MockPageOverlayClient has references to MockPageOverlays, not to PageOverlays.
-    // Also, iterating over a set while modifying it is not good.
-    for (auto& mockOverlay : m_overlays) {
-        if (mockOverlay->overlay() == &overlay) {
-            m_overlays.remove(mockOverlay);
-            return;
-        }
-    }
-}
-
 void MockPageOverlayClient::willMoveToPage(PageOverlay&, Page*)
 {
 }
index cb39f8a..d5eb708 100644 (file)
@@ -49,7 +49,6 @@ public:
     virtual ~MockPageOverlayClient() { }
 
 private:
-    void pageOverlayDestroyed(PageOverlay&) override;
     void willMoveToPage(PageOverlay&, Page*) override;
     void didMoveToPage(PageOverlay&, Page*) override;
     void drawRect(PageOverlay&, GraphicsContext&, const IntRect& dirtyRect) override;
index cc59dc9..f2f76a6 100644 (file)
@@ -1,3 +1,45 @@
+2016-05-20  John Wilander  <wilander@apple.com>
+
+        Remove unnecessary PageOverlay client function pageOverlayDestroyed
+        https://bugs.webkit.org/show_bug.cgi?id=157388
+        <rdar://problem/25471523>
+
+        Reviewed by Tim Horton.
+
+        Remove dead PageOverlay code. Almost all of these overrides were empty and
+        never called. In the case of WebPageOverlay it was never called but had a
+        function body, causing confusion. There was a fear of dangling pointers in
+        WebPageOverlay's static hash map between PageOverlays and WebPageOverlays.
+        Only WebPageOverlay's constructor creates its PageOverlay object and adds it
+        to the hash map. Its client object is kept in a unique pointer member which
+        is automatically deleted when the WebPageOverlay object itself is deleted.
+        This explains why PageOverlayClientImpl::pageOverlayDestroyed in
+        WKBundlePageOverlay can safely be removed. Finally, WebPageOverlay's
+        destructor clears the hash map entry for its PageOverlay object. Thus, there
+        is no need to call WebPageOverlay::pageOverlayDestroyed nor a need for
+        WebPageOverlay's destructor to call pageOverlayDestroyed on its client.
+
+        No new tests. I tried to come up with a WebKit API test for this but I
+        wasn't able to test presence/absence of WebPageOverlay's map entries since
+        the map is not exposed.
+
+        * WebProcess/InjectedBundle/API/c/WKBundlePageOverlay.cpp:
+        (WebKit::PageOverlayClientImpl::pageOverlayDestroyed): Deleted.
+        * WebProcess/Plugins/PDF/PDFPlugin.h:
+        * WebProcess/Plugins/PDF/PDFPlugin.mm:
+        (WebKit::PDFPlugin::HUD::pageOverlayDestroyed): Deleted.
+        * WebProcess/WebCoreSupport/WebInspectorClient.cpp:
+        * WebProcess/WebCoreSupport/WebInspectorClient.h:
+        (WebKit::WebInspectorClient::pageOverlayDestroyed): Deleted.
+        * WebProcess/WebPage/FindController.cpp:
+        * WebProcess/WebPage/FindController.h:
+        (WebKit::FindController::pageOverlayDestroyed): Deleted.
+        * WebProcess/WebPage/WebPageOverlay.cpp:
+        * WebProcess/WebPage/WebPageOverlay.h:
+        (WebKit::WebPageOverlay::pageOverlayDestroyed): Deleted.
+        * WebProcess/WebPage/ios/FindIndicatorOverlayClientIOS.h:
+        (WebKit::FindIndicatorOverlayClientIOS::pageOverlayDestroyed): Deleted.
+
 2016-05-19  Chris Dumez  <cdumez@apple.com>
 
         Improve compile-time assertions in is<>() / downcast<>()
index 499da6e..bfecd33 100644 (file)
@@ -70,11 +70,6 @@ public:
 
 private:
     // WebPageOverlay::Client.
-    void pageOverlayDestroyed(WebPageOverlay&) override
-    {
-        delete this;
-    }
-
     void willMoveToPage(WebPageOverlay& pageOverlay, WebPage* page) override
     {
         if (!m_client.willMoveToPage)
index c2639e8..0c6ab1f 100644 (file)
@@ -262,7 +262,6 @@ private:
         void setVisible(bool, AnimateVisibilityTransition);
 
     private:
-        void pageOverlayDestroyed(WebCore::PageOverlay&) override;
         void willMoveToPage(WebCore::PageOverlay&, WebCore::Page*) override;
         void didMoveToPage(WebCore::PageOverlay&, WebCore::Page*) override;
         void drawRect(WebCore::PageOverlay&, WebCore::GraphicsContext&, const WebCore::IntRect& dirtyRect) override;
index af19f31..13cebb2 100644 (file)
@@ -1648,10 +1648,6 @@ PDFPlugin::HUD::~HUD()
     mainFrame.pageOverlayController().uninstallPageOverlay(m_overlay.ptr(), PageOverlay::FadeMode::DoNotFade);
 }
 
-void PDFPlugin::HUD::pageOverlayDestroyed(PageOverlay&)
-{
-}
-
 void PDFPlugin::HUD::willMoveToPage(PageOverlay&, Page* page)
 {
 }
index 72be167..021abfa 100644 (file)
@@ -198,10 +198,6 @@ void WebInspectorClient::elementSelectionChanged(bool active)
         m_page->inspector()->elementSelectionChanged(active);
 }
 
-void WebInspectorClient::pageOverlayDestroyed(PageOverlay&)
-{
-}
-
 void WebInspectorClient::willMoveToPage(PageOverlay&, Page* page)
 {
     if (page)
index 9d26aeb..b7d3f0c 100644 (file)
@@ -72,7 +72,6 @@ private:
     void showPaintRect(const WebCore::FloatRect&) override;
 
     // PageOverlay::Client
-    void pageOverlayDestroyed(WebCore::PageOverlay&) override;
     void willMoveToPage(WebCore::PageOverlay&, WebCore::Page*) override;
     void didMoveToPage(WebCore::PageOverlay&, WebCore::Page*) override;
     void drawRect(WebCore::PageOverlay&, WebCore::GraphicsContext&, const WebCore::IntRect&) override;
index bf5cd17..9f1193f 100644 (file)
@@ -423,10 +423,6 @@ Vector<IntRect> FindController::rectsForTextMatchesInRect(IntRect clipRect)
     return rects;
 }
 
-void FindController::pageOverlayDestroyed(PageOverlay&)
-{
-}
-
 void FindController::willMoveToPage(PageOverlay&, Page* page)
 {
     if (page)
index 924af53..f30a46a 100644 (file)
@@ -73,7 +73,6 @@ public:
 
 private:
     // PageOverlay::Client.
-    virtual void pageOverlayDestroyed(WebCore::PageOverlay&);
     virtual void willMoveToPage(WebCore::PageOverlay&, WebCore::Page*);
     virtual void didMoveToPage(WebCore::PageOverlay&, WebCore::Page*);
     virtual bool mouseEvent(WebCore::PageOverlay&, const WebCore::PlatformMouseEvent&);
index e802ed3..2f42a07 100644 (file)
@@ -84,16 +84,6 @@ void WebPageOverlay::clear()
     m_overlay->clear();
 }
 
-void WebPageOverlay::pageOverlayDestroyed(PageOverlay&)
-{
-    if (m_overlay) {
-        overlayMap().remove(m_overlay.get());
-        m_overlay = nullptr;
-    }
-
-    m_client->pageOverlayDestroyed(*this);
-}
-
 void WebPageOverlay::willMoveToPage(PageOverlay&, Page* page)
 {
     m_client->willMoveToPage(*this, page ? WebPage::fromCorePage(page) : nullptr);
index 788e625..5b16b49 100644 (file)
@@ -47,7 +47,6 @@ public:
     public:
         virtual ~Client() { }
 
-        virtual void pageOverlayDestroyed(WebPageOverlay&) = 0;
         virtual void willMoveToPage(WebPageOverlay&, WebPage*) = 0;
         virtual void didMoveToPage(WebPageOverlay&, WebPage*) = 0;
         virtual void drawRect(WebPageOverlay&, WebCore::GraphicsContext&, const WebCore::IntRect& dirtyRect) = 0;
@@ -89,7 +88,6 @@ private:
     WebPageOverlay(std::unique_ptr<Client>, WebCore::PageOverlay::OverlayType);
 
     // WebCore::PageOverlay::Client
-    void pageOverlayDestroyed(WebCore::PageOverlay&) override;
     void willMoveToPage(WebCore::PageOverlay&, WebCore::Page*) override;
     void didMoveToPage(WebCore::PageOverlay&, WebCore::Page*) override;
     void drawRect(WebCore::PageOverlay&, WebCore::GraphicsContext&, const WebCore::IntRect& dirtyRect) override;
index 64c453b..0368e21 100644 (file)
@@ -45,7 +45,6 @@ public:
     }
 
 private:
-    void pageOverlayDestroyed(WebCore::PageOverlay&) override { }
     void willMoveToPage(WebCore::PageOverlay&, WebCore::Page*) override { }
     void didMoveToPage(WebCore::PageOverlay&, WebCore::Page*) override { }
     void drawRect(WebCore::PageOverlay&, WebCore::GraphicsContext&, const WebCore::IntRect& dirtyRect) override;