Clean up code related to the updating of Dashboard and touch event regions
authorsimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 15 Jan 2019 22:57:27 +0000 (22:57 +0000)
committersimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 15 Jan 2019 22:57:27 +0000 (22:57 +0000)
https://bugs.webkit.org/show_bug.cgi?id=193460

Reviewed by Zalan Bujtas.

In preparation for layout testing that can count the number of event region
updates, move the code related to updating "annotated" (Dashboard) regions, and
touch event regions into bottleneck functions in Document.

Updating these two kinds of regions is generally similar, but there are some code paths
that eagerly update annotated regions.

No behavior change.

* dom/Document.cpp:
(WebCore::Document::updateAnnotatedRegions): Moved from FrameView.
(WebCore::Document::invalidateRenderingDependentRegions):
(WebCore::Document::invalidateScrollbarDependentRegions):
(WebCore::Document::updateZOrderDependentRegions):
* dom/Document.h:
(WebCore::Document::setAnnotatedRegionsDirty):
(WebCore::Document::annotatedRegionsDirty const):
(WebCore::Document::hasAnnotatedRegions const):
* page/FrameView.cpp:
(WebCore::FrameView::didLayout):
(WebCore::FrameView::didPaintContents):
(WebCore::FrameView::updateAnnotatedRegions): Deleted.
* page/FrameView.h:
* rendering/RenderElement.cpp: Drive-by header cleanup.
(WebCore::RenderElement::styleWillChange):
* rendering/RenderLayer.cpp:
(WebCore::RenderLayer::scrollTo):
(WebCore::RenderLayer::setHasHorizontalScrollbar):
(WebCore::RenderLayer::setHasVerticalScrollbar):
(WebCore::RenderLayer::updateScrollbarsAfterLayout):
(WebCore::RenderLayer::calculateClipRects const):
* rendering/RenderListBox.cpp:
(WebCore::RenderListBox::setHasVerticalScrollbar):

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

Source/WebCore/ChangeLog
Source/WebCore/dom/Document.cpp
Source/WebCore/dom/Document.h
Source/WebCore/page/FrameView.cpp
Source/WebCore/page/FrameView.h
Source/WebCore/rendering/RenderElement.cpp
Source/WebCore/rendering/RenderLayer.cpp
Source/WebCore/rendering/RenderListBox.cpp

index 7592e95..670a694 100644 (file)
@@ -1,3 +1,44 @@
+2019-01-15  Simon Fraser  <simon.fraser@apple.com>
+
+        Clean up code related to the updating of Dashboard and touch event regions
+        https://bugs.webkit.org/show_bug.cgi?id=193460
+
+        Reviewed by Zalan Bujtas.
+
+        In preparation for layout testing that can count the number of event region
+        updates, move the code related to updating "annotated" (Dashboard) regions, and
+        touch event regions into bottleneck functions in Document.
+        
+        Updating these two kinds of regions is generally similar, but there are some code paths
+        that eagerly update annotated regions.
+
+        No behavior change.
+
+        * dom/Document.cpp:
+        (WebCore::Document::updateAnnotatedRegions): Moved from FrameView.
+        (WebCore::Document::invalidateRenderingDependentRegions):
+        (WebCore::Document::invalidateScrollbarDependentRegions):
+        (WebCore::Document::updateZOrderDependentRegions):
+        * dom/Document.h:
+        (WebCore::Document::setAnnotatedRegionsDirty):
+        (WebCore::Document::annotatedRegionsDirty const):
+        (WebCore::Document::hasAnnotatedRegions const):
+        * page/FrameView.cpp:
+        (WebCore::FrameView::didLayout):
+        (WebCore::FrameView::didPaintContents):
+        (WebCore::FrameView::updateAnnotatedRegions): Deleted.
+        * page/FrameView.h:
+        * rendering/RenderElement.cpp: Drive-by header cleanup.
+        (WebCore::RenderElement::styleWillChange):
+        * rendering/RenderLayer.cpp:
+        (WebCore::RenderLayer::scrollTo):
+        (WebCore::RenderLayer::setHasHorizontalScrollbar):
+        (WebCore::RenderLayer::setHasVerticalScrollbar):
+        (WebCore::RenderLayer::updateScrollbarsAfterLayout):
+        (WebCore::RenderLayer::calculateClipRects const):
+        * rendering/RenderListBox.cpp:
+        (WebCore::RenderListBox::setHasVerticalScrollbar):
+
 2019-01-15  David Kilzer  <ddkilzer@apple.com>
 
         Let Xcode have its way with the WebCore project
index c7b21da..05c2e14 100644 (file)
@@ -4099,7 +4099,6 @@ void Document::elementInActiveChainDidDetach(Element* element)
 }
 
 #if ENABLE(DASHBOARD_SUPPORT)
-
 const Vector<AnnotatedRegionValue>& Document::annotatedRegions() const
 {
     return m_annotatedRegions;
@@ -4111,7 +4110,56 @@ void Document::setAnnotatedRegions(const Vector<AnnotatedRegionValue>& regions)
     setAnnotatedRegionsDirty(false);
 }
 
+void Document::updateAnnotatedRegions()
+{
+    if (!hasAnnotatedRegions())
+        return;
+
+    Vector<AnnotatedRegionValue> newRegions;
+    renderBox()->collectAnnotatedRegions(newRegions); // FIXME.
+    if (newRegions == annotatedRegions())
+        return;
+
+    setAnnotatedRegions(newRegions);
+    
+    if (Page* page = this->page())
+        page->chrome().client().annotatedRegionsChanged();
+}
+#endif
+
+void Document::invalidateRenderingDependentRegions(AnnotationsAction annotationsAction)
+{
+#if ENABLE(DASHBOARD_SUPPORT)
+    // FIXME: we don't have a good invalidation/update policy for Dashboard regions. They get eagerly updated
+    // on forced layouts, and don't need to be.
+    if (annotationsAction == AnnotationsAction::Update)
+        updateAnnotatedRegions();
+    else
+        setAnnotatedRegionsDirty();
+#else
+    UNUSED_PARAM(annotationsAction);
+#endif
+
+#if PLATFORM(IOS_FAMILY) && ENABLE(TOUCH_EVENTS)
+    setTouchEventRegionsNeedUpdate();
+#endif
+}
+
+void Document::invalidateScrollbarDependentRegions()
+{
+#if ENABLE(DASHBOARD_SUPPORT)
+    if (hasAnnotatedRegions())
+        setAnnotatedRegionsDirty();
+#endif
+}
+
+void Document::updateZOrderDependentRegions()
+{
+#if ENABLE(DASHBOARD_SUPPORT)
+    if (annotatedRegionsDirty())
+        updateAnnotatedRegions();
 #endif
+}
 
 bool Document::setFocusedElement(Element* element, FocusDirection direction, FocusRemovalEventsMode eventsMode)
 {
index 1df75ec..eb44551 100644 (file)
@@ -1137,14 +1137,15 @@ public:
     WEBCORE_EXPORT String displayStringModifiedByEncoding(const String&) const;
 
 #if ENABLE(DASHBOARD_SUPPORT)
-    void setAnnotatedRegionsDirty(bool f) { m_annotatedRegionsDirty = f; }
-    bool annotatedRegionsDirty() const { return m_annotatedRegionsDirty; }
-    bool hasAnnotatedRegions () const { return m_hasAnnotatedRegions; }
     void setHasAnnotatedRegions(bool f) { m_hasAnnotatedRegions = f; }
     WEBCORE_EXPORT const Vector<AnnotatedRegionValue>& annotatedRegions() const;
-    void setAnnotatedRegions(const Vector<AnnotatedRegionValue>&);
 #endif
 
+    enum class AnnotationsAction { Invalidate, Update };
+    void invalidateRenderingDependentRegions(AnnotationsAction = AnnotationsAction::Invalidate);
+    void invalidateScrollbarDependentRegions();
+    void updateZOrderDependentRegions();
+
     void removeAllEventListeners() final;
 
     WEBCORE_EXPORT const SVGDocumentExtensions* svgExtensions();
@@ -1634,6 +1635,14 @@ private:
 
     void wheelEventHandlersChanged();
 
+#if ENABLE(DASHBOARD_SUPPORT)
+    void setAnnotatedRegionsDirty(bool f = true) { m_annotatedRegionsDirty = f; }
+    bool annotatedRegionsDirty() const { return m_annotatedRegionsDirty; }
+    bool hasAnnotatedRegions () const { return m_hasAnnotatedRegions; }
+    void setAnnotatedRegions(const Vector<AnnotatedRegionValue>&);
+    void updateAnnotatedRegions();
+#endif
+
     HttpEquivPolicy httpEquivPolicy() const;
     AXObjectCache* existingAXObjectCacheSlow() const;
 
index d25a7e6..f067d08 100644 (file)
@@ -1304,13 +1304,7 @@ void FrameView::didLayout(WeakPtr<RenderElement> layoutRoot)
         cache->postNotification(layoutRoot.get(), AXObjectCache::AXLayoutComplete);
 #endif
 
-#if ENABLE(DASHBOARD_SUPPORT)
-    updateAnnotatedRegions();
-#endif
-
-#if ENABLE(IOS_TOUCH_EVENTS)
-    frame().document()->setTouchEventRegionsNeedUpdate();
-#endif
+    frame().document()->invalidateRenderingDependentRegions(Document::AnnotationsAction::Update);
 
     updateCanBlitOnScrollRecursively();
 
@@ -3882,24 +3876,6 @@ bool FrameView::scrollAnimatorEnabled() const
     return false;
 }
 
-#if ENABLE(DASHBOARD_SUPPORT)
-void FrameView::updateAnnotatedRegions()
-{
-    Document* document = frame().document();
-    if (!document->hasAnnotatedRegions())
-        return;
-    Vector<AnnotatedRegionValue> newRegions;
-    document->renderBox()->collectAnnotatedRegions(newRegions);
-    if (newRegions == document->annotatedRegions())
-        return;
-    document->setAnnotatedRegions(newRegions);
-    Page* page = frame().page();
-    if (!page)
-        return;
-    page->chrome().client().annotatedRegionsChanged();
-}
-#endif
-
 void FrameView::updateScrollCorner()
 {
     RenderElement* renderer = nullptr;
@@ -4177,10 +4153,7 @@ void FrameView::didPaintContents(GraphicsContext& context, const IntRect& dirtyR
     m_lastPaintTime = MonotonicTime::now();
 
     // Regions may have changed as a result of the visibility/z-index of element changing.
-#if ENABLE(DASHBOARD_SUPPORT)
-    if (frame().document()->annotatedRegionsDirty())
-        updateAnnotatedRegions();
-#endif
+    frame().document()->updateZOrderDependentRegions();
 
     if (paintingState.isTopLevelPainter)
         sCurrentPaintTimeStamp = MonotonicTime();
index 2976b56..529f6cc 100644 (file)
@@ -333,9 +333,6 @@ public:
     bool speculativeTilingEnabled() const { return m_speculativeTilingEnabled; }
     void loadProgressingStatusChanged();
 
-#if ENABLE(DASHBOARD_SUPPORT)
-    void updateAnnotatedRegions();
-#endif
     WEBCORE_EXPORT void updateControlTints();
 
     WEBCORE_EXPORT bool wasScrolledByUser() const;
index c9b95d8..3954452 100644 (file)
@@ -48,6 +48,7 @@
 #include "RenderDescendantIterator.h"
 #include "RenderFlexibleBox.h"
 #include "RenderFragmentedFlow.h"
+#include "RenderGrid.h"
 #include "RenderImage.h"
 #include "RenderImageResourceStyleImage.h"
 #include "RenderInline.h"
@@ -79,8 +80,6 @@
 #include <wtf/MathExtras.h>
 #include <wtf/StackStats.h>
 
-#include "RenderGrid.h"
-
 namespace WebCore {
 
 WTF_MAKE_ISO_ALLOCATED_IMPL(RenderElement);
@@ -709,14 +708,10 @@ void RenderElement::styleWillChange(StyleDifference diff, const RenderStyle& new
         bool visibilityChanged = m_style.visibility() != newStyle.visibility()
             || m_style.zIndex() != newStyle.zIndex()
             || m_style.hasAutoZIndex() != newStyle.hasAutoZIndex();
-#if ENABLE(DASHBOARD_SUPPORT)
-        if (visibilityChanged)
-            document().setAnnotatedRegionsDirty(true);
-#endif
-#if PLATFORM(IOS_FAMILY) && ENABLE(TOUCH_EVENTS)
+
         if (visibilityChanged)
-            document().setTouchEventRegionsNeedUpdate();
-#endif
+            document().invalidateRenderingDependentRegions();
+
         if (visibilityChanged) {
             if (AXObjectCache* cache = document().existingAXObjectCache())
                 cache->childrenChanged(parent(), this);
@@ -737,6 +732,7 @@ void RenderElement::styleWillChange(StyleDifference diff, const RenderStyle& new
 
         if (m_parent && (newStyle.outlineSize() < m_style.outlineSize() || shouldRepaintForStyleDifference(diff)))
             repaint();
+
         if (isFloating() && m_style.floating() != newStyle.floating()) {
             // For changes in float styles, we need to conceivably remove ourselves
             // from the floating objects list.
index ca9d118..6c2a423 100644 (file)
@@ -2392,10 +2392,7 @@ void RenderLayer::scrollTo(const ScrollPosition& position)
     if (!view.frameView().layoutContext().isInRenderTreeLayout()) {
         // If we're in the middle of layout, we'll just update layers once layout has finished.
         updateLayerPositionsAfterOverflowScroll();
-        // Update regions, scrolling may change the clip of a particular region.
-#if ENABLE(DASHBOARD_SUPPORT)
-        view.frameView().updateAnnotatedRegions();
-#endif
+
         view.frameView().scheduleUpdateWidgetPositions();
 
         if (!m_updatingMarqueePosition) {
@@ -2411,9 +2408,8 @@ void RenderLayer::scrollTo(const ScrollPosition& position)
             updateCompositingLayersAfterScroll();
         }
 
-#if PLATFORM(IOS_FAMILY) && ENABLE(TOUCH_EVENTS)
-        renderer().document().setTouchEventRegionsNeedUpdate();
-#endif
+        // Update regions, scrolling may change the clip of a particular region.
+        renderer().document().invalidateRenderingDependentRegions(Document::AnnotationsAction::Update);
         DebugPageOverlays::didLayout(renderer().frame());
     }
 
@@ -3193,11 +3189,7 @@ void RenderLayer::setHasHorizontalScrollbar(bool hasScrollbar)
     if (m_vBar)
         m_vBar->styleChanged();
 
-    // Force an update since we know the scrollbars have changed things.
-#if ENABLE(DASHBOARD_SUPPORT)
-    if (renderer().document().hasAnnotatedRegions())
-        renderer().document().setAnnotatedRegionsDirty(true);
-#endif
+    renderer().document().invalidateScrollbarDependentRegions();
 }
 
 void RenderLayer::setHasVerticalScrollbar(bool hasScrollbar)
@@ -3224,11 +3216,7 @@ void RenderLayer::setHasVerticalScrollbar(bool hasScrollbar)
     if (m_vBar)
         m_vBar->styleChanged();
 
-    // Force an update since we know the scrollbars have changed things.
-#if ENABLE(DASHBOARD_SUPPORT)
-    if (renderer().document().hasAnnotatedRegions())
-        renderer().document().setAnnotatedRegionsDirty(true);
-#endif
+    renderer().document().invalidateScrollbarDependentRegions();
 }
 
 ScrollableArea* RenderLayer::enclosingScrollableArea() const
@@ -3494,12 +3482,7 @@ void RenderLayer::updateScrollbarsAfterLayout()
 
         updateSelfPaintingLayer();
 
-        // Force an update since we know the scrollbars have changed things.
-#if ENABLE(DASHBOARD_SUPPORT)
-        if (renderer().document().hasAnnotatedRegions())
-            renderer().document().setAnnotatedRegionsDirty(true);
-#endif
-
+        renderer().document().invalidateScrollbarDependentRegions();
         renderer().repaint();
 
         if (renderer().style().overflowX() == Overflow::Auto || renderer().style().overflowY() == Overflow::Auto) {
@@ -6379,7 +6362,7 @@ void RenderLayer::styleChanged(StyleDifference diff, const RenderStyle* oldStyle
 
 #if PLATFORM(IOS_FAMILY) && ENABLE(TOUCH_EVENTS)
     if (diff == StyleDifference::RecompositeLayer || diff >= StyleDifference::LayoutPositionedMovementOnly)
-        renderer().document().setTouchEventRegionsNeedUpdate();
+        renderer().document().invalidateRenderingDependentRegions();
 #else
     UNUSED_PARAM(diff);
 #endif
index 63ade7e..c1df80f 100644 (file)
@@ -954,11 +954,7 @@ void RenderListBox::setHasVerticalScrollbar(bool hasScrollbar)
     if (m_vBar)
         m_vBar->styleChanged();
 
-    // Force an update since we know the scrollbars have changed things.
-#if ENABLE(DASHBOARD_SUPPORT)
-    if (document().hasAnnotatedRegions())
-        document().setAnnotatedRegionsDirty(true);
-#endif
+    document().invalidateScrollbarDependentRegions();
 }
 
 bool RenderListBox::scrolledToTop() const