Changeset 268902 in webkit

Timestamp:

Oct 22, 2020, 6:38:25 PM (6 years ago)

Author:

Simon Fraser

Message:

REGRESSION(r268476): [ macOS ] tiled-drawing/scrolling/non-fast-region/handlers-in-iframes.html is a flaky failure
​https://bugs.webkit.org/show_bug.cgi?id=218031
<rdar://problem/70532268>

Reviewed by Tim Horton.
Source/WebCore:

The test exercises wheel event regions in non-composited iframes. These were not reliably updated
when all wheel event handlers were removed, because Frame::invalidateContentEventRegionsIfNeeded()
early-returned if there were no handlers (but we need to update the regions in this case). Clean
up the logic here, and pass in a "reason" so we know that we should do work when there are no
handlers.

Document::wheelEventHandlersChanged() also needs to invalidate style, since flags in style
are used for wheel event region building, and it needs to call invalidateContentEventRegionsIfNeeded().

• dom/Document.cpp:

(WebCore::Document::wheelEventHandlersChanged):
(WebCore::Document::didAddWheelEventHandler):
(WebCore::Document::didRemoveWheelEventHandler):

• dom/Document.h:
• page/Frame.cpp:

(WebCore::Frame::invalidateContentEventRegionsIfNeeded):

• page/Frame.h:
• page/FrameView.cpp:

(WebCore::FrameView::didLayout):

LayoutTests:

• platform/mac/TestExpectations:

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/platform/mac/TestExpectations (modified) (1 diff)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/dom/Document.cpp (modified) (4 diffs)
• Source/WebCore/dom/Document.h (modified) (1 diff)
• Source/WebCore/page/Frame.cpp (modified) (1 diff)
• Source/WebCore/page/Frame.h (modified) (1 diff)
• Source/WebCore/page/FrameView.cpp (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2020-10-22  Simon Fraser  <simon.fraser@apple.com>
+
+        REGRESSION(r268476): [ macOS ] tiled-drawing/scrolling/non-fast-region/handlers-in-iframes.html is a flaky failure
+        https://bugs.webkit.org/show_bug.cgi?id=218031
+        <rdar://problem/70532268>
+
+        Reviewed by Tim Horton.
+        
+        * platform/mac/TestExpectations:
+
 2020-10-22  Aditya Keerthi  <akeerthi@apple.com>
 

trunk/LayoutTests/platform/mac/TestExpectations

@@ -2257 +2257 @@
 webkit.org/b/217994 imported/w3c/web-platform-tests/webaudio/the-audio-api/the-audioworklet-interface/audioworkletnode-output-channel-count.https.html [ Pass Failure ]
 
-webkit.org/b/218031 tiled-drawing/scrolling/non-fast-region/handlers-in-iframes.html [ Pass Failure ]
-
 # rdar://70546330 REGRESSION (r267531): [ Big Sur iOS 14 ] imported/w3c/web-platform-tests/mathml/relations/css-styling/padding-border-margin/border-002.html is a constant failure
 [ BigSur+ ] imported/w3c/web-platform-tests/mathml/relations/css-styling/padding-border-margin/border-002.html [ Failure ]

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2020-10-22  Simon Fraser  <simon.fraser@apple.com>
+
+        REGRESSION(r268476): [ macOS ] tiled-drawing/scrolling/non-fast-region/handlers-in-iframes.html is a flaky failure
+        https://bugs.webkit.org/show_bug.cgi?id=218031
+        <rdar://problem/70532268>
+
+        Reviewed by Tim Horton.
+
+        The test exercises wheel event regions in non-composited iframes. These were not reliably updated
+        when all wheel event handlers were removed, because Frame::invalidateContentEventRegionsIfNeeded()
+        early-returned if there were no handlers (but we need to update the regions in this case). Clean
+        up the logic here, and pass in a "reason" so we know that we should do work when there are no
+        handlers.
+
+        Document::wheelEventHandlersChanged() also needs to invalidate style, since flags in style
+        are used for wheel event region building, and it needs to call invalidateContentEventRegionsIfNeeded().
+
+        * dom/Document.cpp:
+        (WebCore::Document::wheelEventHandlersChanged):
+        (WebCore::Document::didAddWheelEventHandler):
+        (WebCore::Document::didRemoveWheelEventHandler):
+        * dom/Document.h:
+        * page/Frame.cpp:
+        (WebCore::Frame::invalidateContentEventRegionsIfNeeded):
+        * page/Frame.h:
+        * page/FrameView.cpp:
+        (WebCore::FrameView::didLayout):
+
 2020-10-22  Aditya Keerthi  <akeerthi@apple.com>
 

trunk/Source/WebCore/dom/Document.cpp

@@ -6644 +6644 @@
 }
 
-void Document::wheelEventHandlersChanged()
+void Document::wheelEventHandlersChanged(Node* node)
 {
     Page* page = this->page();
@@ -6655 +6655 @@
     }
 
+#if ENABLE(WHEEL_EVENT_REGIONS)
+    if (is<Element>(node)) {
+        // Style is affected via eventListenerRegionTypes().
+        downcast<Element>(*node).invalidateStyle();
+    }
+
+    m_frame->invalidateContentEventRegionsIfNeeded(Frame::InvalidateContentEventRegionsReason::EventHandlerChange);
+#else
+    UNUSED_PARAM(node);
+#endif
+
     bool haveHandlers = m_wheelEventTargets && !m_wheelEventTargets->isEmpty();
     page->chrome().client().wheelEventHandlersChanged(haveHandlers);
@@ -6665 +6676 @@
 
     m_wheelEventTargets->add(&node);
-
-    wheelEventHandlersChanged();
+    wheelEventHandlersChanged(&node);
 
     if (Frame* frame = this->frame())
@@ -6700 +6710 @@
         return;
 
-    wheelEventHandlersChanged();
+    wheelEventHandlersChanged(&node);
 
     if (Frame* frame = this->frame())

trunk/Source/WebCore/dom/Document.h

@@ -1675 +1675 @@
     void didAssociateFormControlsTimerFired();
 
-    void wheelEventHandlersChanged();
+    void wheelEventHandlersChanged(Node* = nullptr);
 
     HttpEquivPolicy httpEquivPolicy() const;

trunk/Source/WebCore/page/Frame.cpp

@@ -301 +301 @@
 }
 
-void Frame::invalidateContentEventRegionsIfNeeded()
+void Frame::invalidateContentEventRegionsIfNeeded(InvalidateContentEventRegionsReason reason)
 {
     if (!m_page || !m_doc || !m_doc->renderView())
         return;
-    bool hasWheelEventHandlers = false;
-    bool hasTouchActionElements = false;
-    bool hasEditableElements = false;
+
+    bool needsUpdateForWheelEventHandlers = false;
+    bool needsUpdateForTouchActionElements = false;
+    bool needsUpdateForEditableElements = false;
 #if ENABLE(WHEEL_EVENT_REGIONS)
-    hasWheelEventHandlers = m_doc->hasWheelEventHandlers();
+    needsUpdateForWheelEventHandlers = m_doc->hasWheelEventHandlers() || reason == InvalidateContentEventRegionsReason::EventHandlerChange;
+#else
+    UNUSED_PARAM(reason);
 #endif
 #if ENABLE(TOUCH_ACTION_REGIONS)
-    hasTouchActionElements = m_doc->mayHaveElementsWithNonAutoTouchAction();
+    // Document::mayHaveElementsWithNonAutoTouchAction never changes from true to false currently.
+    needsUpdateForTouchActionElements = m_doc->mayHaveElementsWithNonAutoTouchAction();
 #endif
 #if ENABLE(EDITABLE_REGION)
-    hasEditableElements = m_doc->mayHaveEditableElements() && m_page->shouldBuildEditableRegion();
+    // Document::mayHaveEditableElements never changes from true to false currently.
+    needsUpdateForEditableElements = m_doc->mayHaveEditableElements() && m_page->shouldBuildEditableRegion();
 #endif
-    if (!hasTouchActionElements && !hasEditableElements && !hasWheelEventHandlers)
-        return;
+    if (!needsUpdateForTouchActionElements && !needsUpdateForEditableElements && !needsUpdateForWheelEventHandlers)
+        return;
+
     if (!m_doc->renderView()->compositor().viewNeedsToInvalidateEventRegionOfEnclosingCompositingLayerForRepaint())
         return;
+
     if (m_ownerElement)
         m_ownerElement->document().invalidateEventRegionsForFrame(*m_ownerElement);

trunk/Source/WebCore/page/Frame.h

@@ -308 +308 @@
     bool mayPrewarmLocalStorage() const;
 
-    void invalidateContentEventRegionsIfNeeded();
+    enum class InvalidateContentEventRegionsReason { Layout, EventHandlerChange };
+    void invalidateContentEventRegionsIfNeeded(InvalidateContentEventRegionsReason);
 
     WEBCORE_EXPORT FloatSize screenSize() const;
     void setOverrideScreenSize(FloatSize&&);
-
-// ========
 
     void selfOnlyRef();

trunk/Source/WebCore/page/FrameView.cpp

@@ -1272 +1272 @@
 #endif
 
-    frame().invalidateContentEventRegionsIfNeeded();
+    frame().invalidateContentEventRegionsIfNeeded(Frame::InvalidateContentEventRegionsReason::Layout);
     document->invalidateRenderingDependentRegions();