Diff
Modified: trunk/Source/WebCore/ChangeLog (201223 => 201224)
--- trunk/Source/WebCore/ChangeLog 2016-05-20 20:43:39 UTC (rev 201223)
+++ trunk/Source/WebCore/ChangeLog 2016-05-20 21:17:35 UTC (rev 201224)
@@ -1,3 +1,41 @@
+2016-05-20 John Wilander <[email protected]>
+
+ 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 <[email protected]>
Fix null dereferencing in CSSAnimationTriggerScrollValue::equals
Modified: trunk/Source/WebCore/page/DebugPageOverlays.cpp (201223 => 201224)
--- trunk/Source/WebCore/page/DebugPageOverlays.cpp 2016-05-20 20:43:39 UTC (rev 201223)
+++ trunk/Source/WebCore/page/DebugPageOverlays.cpp 2016-05-20 21:17:35 UTC (rev 201224)
@@ -54,7 +54,6 @@
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 @@
m_frame.pageOverlayController().uninstallPageOverlay(m_overlay.get(), PageOverlay::FadeMode::DoNotFade);
}
-void RegionOverlay::pageOverlayDestroyed(PageOverlay&)
-{
-}
-
void RegionOverlay::willMoveToPage(PageOverlay&, Page* page)
{
if (!page)
Modified: trunk/Source/WebCore/page/PageOverlay.h (201223 => 201224)
--- trunk/Source/WebCore/page/PageOverlay.h 2016-05-20 20:43:39 UTC (rev 201223)
+++ trunk/Source/WebCore/page/PageOverlay.h 2016-05-20 21:17:35 UTC (rev 201224)
@@ -52,7 +52,6 @@
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;
Modified: trunk/Source/WebCore/page/ResourceUsageOverlay.h (201223 => 201224)
--- trunk/Source/WebCore/page/ResourceUsageOverlay.h 2016-05-20 20:43:39 UTC (rev 201223)
+++ trunk/Source/WebCore/page/ResourceUsageOverlay.h 2016-05-20 21:17:35 UTC (rev 201224)
@@ -62,7 +62,6 @@
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 { }
Modified: trunk/Source/WebCore/page/mac/ServicesOverlayController.h (201223 => 201224)
--- trunk/Source/WebCore/page/mac/ServicesOverlayController.h 2016-05-20 20:43:39 UTC (rev 201223)
+++ trunk/Source/WebCore/page/mac/ServicesOverlayController.h 2016-05-20 21:17:35 UTC (rev 201224)
@@ -97,7 +97,6 @@
};
// PageOverlay::Client
- void pageOverlayDestroyed(PageOverlay&) override;
void willMoveToPage(PageOverlay&, Page*) override;
void didMoveToPage(PageOverlay&, Page*) override;
void drawRect(PageOverlay&, GraphicsContext&, const IntRect& dirtyRect) override;
Modified: trunk/Source/WebCore/page/mac/ServicesOverlayController.mm (201223 => 201224)
--- trunk/Source/WebCore/page/mac/ServicesOverlayController.mm 2016-05-20 20:43:39 UTC (rev 201223)
+++ trunk/Source/WebCore/page/mac/ServicesOverlayController.mm 2016-05-20 21:17:35 UTC (rev 201224)
@@ -221,13 +221,6 @@
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)
Modified: trunk/Source/WebCore/testing/MockPageOverlayClient.cpp (201223 => 201224)
--- trunk/Source/WebCore/testing/MockPageOverlayClient.cpp 2016-05-20 20:43:39 UTC (rev 201223)
+++ trunk/Source/WebCore/testing/MockPageOverlayClient.cpp 2016-05-20 21:17:35 UTC (rev 201224)
@@ -73,19 +73,6 @@
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*)
{
}
Modified: trunk/Source/WebCore/testing/MockPageOverlayClient.h (201223 => 201224)
--- trunk/Source/WebCore/testing/MockPageOverlayClient.h 2016-05-20 20:43:39 UTC (rev 201223)
+++ trunk/Source/WebCore/testing/MockPageOverlayClient.h 2016-05-20 21:17:35 UTC (rev 201224)
@@ -49,7 +49,6 @@
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;
Modified: trunk/Source/WebKit2/ChangeLog (201223 => 201224)
--- trunk/Source/WebKit2/ChangeLog 2016-05-20 20:43:39 UTC (rev 201223)
+++ trunk/Source/WebKit2/ChangeLog 2016-05-20 21:17:35 UTC (rev 201224)
@@ -1,3 +1,45 @@
+2016-05-20 John Wilander <[email protected]>
+
+ 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 <[email protected]>
Improve compile-time assertions in is<>() / downcast<>()
Modified: trunk/Source/WebKit2/WebProcess/InjectedBundle/API/c/WKBundlePageOverlay.cpp (201223 => 201224)
--- trunk/Source/WebKit2/WebProcess/InjectedBundle/API/c/WKBundlePageOverlay.cpp 2016-05-20 20:43:39 UTC (rev 201223)
+++ trunk/Source/WebKit2/WebProcess/InjectedBundle/API/c/WKBundlePageOverlay.cpp 2016-05-20 21:17:35 UTC (rev 201224)
@@ -70,11 +70,6 @@
private:
// WebPageOverlay::Client.
- void pageOverlayDestroyed(WebPageOverlay&) override
- {
- delete this;
- }
-
void willMoveToPage(WebPageOverlay& pageOverlay, WebPage* page) override
{
if (!m_client.willMoveToPage)
Modified: trunk/Source/WebKit2/WebProcess/Plugins/PDF/PDFPlugin.h (201223 => 201224)
--- trunk/Source/WebKit2/WebProcess/Plugins/PDF/PDFPlugin.h 2016-05-20 20:43:39 UTC (rev 201223)
+++ trunk/Source/WebKit2/WebProcess/Plugins/PDF/PDFPlugin.h 2016-05-20 21:17:35 UTC (rev 201224)
@@ -262,7 +262,6 @@
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;
Modified: trunk/Source/WebKit2/WebProcess/Plugins/PDF/PDFPlugin.mm (201223 => 201224)
--- trunk/Source/WebKit2/WebProcess/Plugins/PDF/PDFPlugin.mm 2016-05-20 20:43:39 UTC (rev 201223)
+++ trunk/Source/WebKit2/WebProcess/Plugins/PDF/PDFPlugin.mm 2016-05-20 21:17:35 UTC (rev 201224)
@@ -1648,10 +1648,6 @@
mainFrame.pageOverlayController().uninstallPageOverlay(m_overlay.ptr(), PageOverlay::FadeMode::DoNotFade);
}
-void PDFPlugin::HUD::pageOverlayDestroyed(PageOverlay&)
-{
-}
-
void PDFPlugin::HUD::willMoveToPage(PageOverlay&, Page* page)
{
}
Modified: trunk/Source/WebKit2/WebProcess/WebCoreSupport/WebInspectorClient.cpp (201223 => 201224)
--- trunk/Source/WebKit2/WebProcess/WebCoreSupport/WebInspectorClient.cpp 2016-05-20 20:43:39 UTC (rev 201223)
+++ trunk/Source/WebKit2/WebProcess/WebCoreSupport/WebInspectorClient.cpp 2016-05-20 21:17:35 UTC (rev 201224)
@@ -198,10 +198,6 @@
m_page->inspector()->elementSelectionChanged(active);
}
-void WebInspectorClient::pageOverlayDestroyed(PageOverlay&)
-{
-}
-
void WebInspectorClient::willMoveToPage(PageOverlay&, Page* page)
{
if (page)
Modified: trunk/Source/WebKit2/WebProcess/WebCoreSupport/WebInspectorClient.h (201223 => 201224)
--- trunk/Source/WebKit2/WebProcess/WebCoreSupport/WebInspectorClient.h 2016-05-20 20:43:39 UTC (rev 201223)
+++ trunk/Source/WebKit2/WebProcess/WebCoreSupport/WebInspectorClient.h 2016-05-20 21:17:35 UTC (rev 201224)
@@ -72,7 +72,6 @@
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;
Modified: trunk/Source/WebKit2/WebProcess/WebPage/FindController.cpp (201223 => 201224)
--- trunk/Source/WebKit2/WebProcess/WebPage/FindController.cpp 2016-05-20 20:43:39 UTC (rev 201223)
+++ trunk/Source/WebKit2/WebProcess/WebPage/FindController.cpp 2016-05-20 21:17:35 UTC (rev 201224)
@@ -423,10 +423,6 @@
return rects;
}
-void FindController::pageOverlayDestroyed(PageOverlay&)
-{
-}
-
void FindController::willMoveToPage(PageOverlay&, Page* page)
{
if (page)
Modified: trunk/Source/WebKit2/WebProcess/WebPage/FindController.h (201223 => 201224)
--- trunk/Source/WebKit2/WebProcess/WebPage/FindController.h 2016-05-20 20:43:39 UTC (rev 201223)
+++ trunk/Source/WebKit2/WebProcess/WebPage/FindController.h 2016-05-20 21:17:35 UTC (rev 201224)
@@ -73,7 +73,6 @@
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&);
Modified: trunk/Source/WebKit2/WebProcess/WebPage/WebPageOverlay.cpp (201223 => 201224)
--- trunk/Source/WebKit2/WebProcess/WebPage/WebPageOverlay.cpp 2016-05-20 20:43:39 UTC (rev 201223)
+++ trunk/Source/WebKit2/WebProcess/WebPage/WebPageOverlay.cpp 2016-05-20 21:17:35 UTC (rev 201224)
@@ -84,16 +84,6 @@
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);
Modified: trunk/Source/WebKit2/WebProcess/WebPage/WebPageOverlay.h (201223 => 201224)
--- trunk/Source/WebKit2/WebProcess/WebPage/WebPageOverlay.h 2016-05-20 20:43:39 UTC (rev 201223)
+++ trunk/Source/WebKit2/WebProcess/WebPage/WebPageOverlay.h 2016-05-20 21:17:35 UTC (rev 201224)
@@ -47,7 +47,6 @@
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 @@
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;
Modified: trunk/Source/WebKit2/WebProcess/WebPage/ios/FindIndicatorOverlayClientIOS.h (201223 => 201224)
--- trunk/Source/WebKit2/WebProcess/WebPage/ios/FindIndicatorOverlayClientIOS.h 2016-05-20 20:43:39 UTC (rev 201223)
+++ trunk/Source/WebKit2/WebProcess/WebPage/ios/FindIndicatorOverlayClientIOS.h 2016-05-20 21:17:35 UTC (rev 201224)
@@ -45,7 +45,6 @@
}
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;
