Attachment 359200 Details for Bug 193460 – Patch

[patch] Patch

bug-193460-20190115134322.patch (text/plain), 14.53 KB, created by Simon Fraser (smfr) on 2019-01-15 13:43:23 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Simon Fraser (smfr)

Created: 2019-01-15 13:43:23 PST

Size: 14.53 KB

patch

obsolete

>Subversion Revision: 239990
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index ad34c6dfdd3a5d9bc1f4626d55f9f49871ca405b..6f6bd1e7c4b437c86143e0389bd4a89d7c642a55 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -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 NOBODY (OOPS!).
>+
>+        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  Devin Rousso  <drousso@apple.com>
> 
>         Web Inspector: Audit: provide a way to determine whether a give node has event listeners
>diff --git a/Source/WebCore/dom/Document.cpp b/Source/WebCore/dom/Document.cpp
>index c7b21da1f5d244a5c0ca9bfd2696d03aa027369f..9f19dbfb18c0111d4aa907e920a80db6f47affad 100644
>--- a/Source/WebCore/dom/Document.cpp
>+++ b/Source/WebCore/dom/Document.cpp
>@@ -4099,7 +4099,6 @@ void Document::elementInActiveChainDidDetach(Element* element)
> }
> 
> #if ENABLE(DASHBOARD_SUPPORT)
>-
> const Vector<AnnotatedRegionValue>& Document::annotatedRegions() const
> {
>     return m_annotatedRegions;
>@@ -4111,8 +4110,55 @@ 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();
> #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)
> {
>     RefPtr<Element> newFocusedElement = element;
>diff --git a/Source/WebCore/dom/Document.h b/Source/WebCore/dom/Document.h
>index 1df75ec3e3f8e3e7bcd8b8c77f5ac4b9e0f1da46..eb44551ccaf34a09b89378aa871349771396e319 100644
>--- a/Source/WebCore/dom/Document.h
>+++ b/Source/WebCore/dom/Document.h
>@@ -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;
> 
>diff --git a/Source/WebCore/page/FrameView.cpp b/Source/WebCore/page/FrameView.cpp
>index d25a7e61a7ae34cb257e14f0e525c73982282de9..f067d08a74365c38840de09de5e797dadaa0fb0b 100644
>--- a/Source/WebCore/page/FrameView.cpp
>+++ b/Source/WebCore/page/FrameView.cpp
>@@ -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();
>diff --git a/Source/WebCore/page/FrameView.h b/Source/WebCore/page/FrameView.h
>index 2976b567dbf0a19789b76e609a117e62b511825b..529f6cc39a6eec300cf92575eadc4c800b9fb65d 100644
>--- a/Source/WebCore/page/FrameView.h
>+++ b/Source/WebCore/page/FrameView.h
>@@ -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;
>diff --git a/Source/WebCore/rendering/RenderElement.cpp b/Source/WebCore/rendering/RenderElement.cpp
>index c9b95d82389907d6913bd3c53a5f6f0c70ed55da..3954452586b54ee92e8c530421902a974c94a15d 100644
>--- a/Source/WebCore/rendering/RenderElement.cpp
>+++ b/Source/WebCore/rendering/RenderElement.cpp
>@@ -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.
>diff --git a/Source/WebCore/rendering/RenderLayer.cpp b/Source/WebCore/rendering/RenderLayer.cpp
>index ca9d11815168f1a0d69cce61a97bcac0c572e317..6c2a423b6776fc990f0a7e0f340661bccd0f237d 100644
>--- a/Source/WebCore/rendering/RenderLayer.cpp
>+++ b/Source/WebCore/rendering/RenderLayer.cpp
>@@ -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
>diff --git a/Source/WebCore/rendering/RenderListBox.cpp b/Source/WebCore/rendering/RenderListBox.cpp
>index 63ade7e23e4367fceabe01f46683db32fe81ad61..c1df80fafc72b3381974c6876ea70bcb48e66537 100644
>--- a/Source/WebCore/rendering/RenderListBox.cpp
>+++ b/Source/WebCore/rendering/RenderListBox.cpp
>@@ -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

Flags:

zalan: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 193460: 359200