Changeset 214819 in webkit


Ignore:
Timestamp:
Apr 3, 2017 10:51:29 AM (7 years ago)
Author:
Simon Fraser
Message:

Clean up touch event handler registration when moving nodes between documents
​https://bugs.webkit.org/show_bug.cgi?id=170384
rdar://problem/30816694

Reviewed by Chris Dumez.

Source/WebCore:

Make sure that Node::didMoveToNewDocument() does the correct unregistration on the
old document, and registration on the new document for nodes with touch event listeners,
and gesture event listeners. Touch "handler" nodes (those for overflow and sliders) are
already correctly moved via renderer-related teardown.

Add assertions that fire when removal was not complete.

Use references in more places.

Tests: fast/events/touch/ios/gesture-node-move-between-documents.html

fast/events/touch/ios/overflow-node-move-between-documents.html
fast/events/touch/ios/slider-node-move-between-documents.html
fast/events/touch/ios/touch-node-move-between-documents.html

  • dom/EventNames.h:

(WebCore::EventNames::gestureEventNames):

  • dom/Node.cpp:

(WebCore::Node::willBeDeletedFrom):
(WebCore::Node::didMoveToNewDocument):
(WebCore::tryAddEventListener):
(WebCore::tryRemoveEventListener):

  • html/shadow/SliderThumbElement.cpp:

(WebCore::SliderThumbElement::registerForTouchEvents):
(WebCore::SliderThumbElement::unregisterForTouchEvents):

  • rendering/RenderLayer.cpp:

(WebCore::RenderLayer::registerAsTouchEventListenerForScrolling):
(WebCore::RenderLayer::unregisterAsTouchEventListenerForScrolling):

LayoutTests:

Tests for moving nodes with various listener/handler combinations between documents.

  • fast/events/touch/ios/gesture-node-move-between-documents-expected.txt: Added.
  • fast/events/touch/ios/gesture-node-move-between-documents.html: Added.
  • fast/events/touch/ios/overflow-node-move-between-documents-expected.txt: Added.
  • fast/events/touch/ios/overflow-node-move-between-documents.html: Added.
  • fast/events/touch/ios/slider-node-move-between-documents-expected.txt: Added.
  • fast/events/touch/ios/slider-node-move-between-documents.html: Added.
  • fast/events/touch/ios/touch-node-move-between-documents-expected.txt: Added.
  • fast/events/touch/ios/touch-node-move-between-documents.html: Added.
Location:
trunk
Files:
8 added
6 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r214807 r214819  
     12017-04-01  Simon Fraser  <simon.fraser@apple.com>
     2
     3        Clean up touch event handler registration when moving nodes between documents
     4        https://bugs.webkit.org/show_bug.cgi?id=170384
     5        rdar://problem/30816694
     6
     7        Reviewed by Chris Dumez.
     8
     9        Tests for moving nodes with various listener/handler combinations between documents.
     10
     11        * fast/events/touch/ios/gesture-node-move-between-documents-expected.txt: Added.
     12        * fast/events/touch/ios/gesture-node-move-between-documents.html: Added.
     13        * fast/events/touch/ios/overflow-node-move-between-documents-expected.txt: Added.
     14        * fast/events/touch/ios/overflow-node-move-between-documents.html: Added.
     15        * fast/events/touch/ios/slider-node-move-between-documents-expected.txt: Added.
     16        * fast/events/touch/ios/slider-node-move-between-documents.html: Added.
     17        * fast/events/touch/ios/touch-node-move-between-documents-expected.txt: Added.
     18        * fast/events/touch/ios/touch-node-move-between-documents.html: Added.
     19
    1202017-04-03  Carlos Garcia Campos  <cgarcia@igalia.com>
    221

  • trunk/Source/WebCore/ChangeLog

    r214806 r214819  
     12017-04-01  Simon Fraser  <simon.fraser@apple.com>
     2
     3        Clean up touch event handler registration when moving nodes between documents
     4        https://bugs.webkit.org/show_bug.cgi?id=170384
     5        rdar://problem/30816694
     6
     7        Reviewed by Chris Dumez.
     8
     9        Make sure that Node::didMoveToNewDocument() does the correct unregistration on the
     10        old document, and registration on the new document for nodes with touch event listeners,
     11        and gesture event listeners. Touch "handler" nodes (those for overflow and sliders) are
     12        already correctly moved via renderer-related teardown.
     13
     14        Add assertions that fire when removal was not complete.
     15
     16        Use references in more places.
     17
     18        Tests: fast/events/touch/ios/gesture-node-move-between-documents.html
     19               fast/events/touch/ios/overflow-node-move-between-documents.html
     20               fast/events/touch/ios/slider-node-move-between-documents.html
     21               fast/events/touch/ios/touch-node-move-between-documents.html
     22
     23        * dom/EventNames.h:
     24        (WebCore::EventNames::gestureEventNames):
     25        * dom/Node.cpp:
     26        (WebCore::Node::willBeDeletedFrom):
     27        (WebCore::Node::didMoveToNewDocument):
     28        (WebCore::tryAddEventListener):
     29        (WebCore::tryRemoveEventListener):
     30        * html/shadow/SliderThumbElement.cpp:
     31        (WebCore::SliderThumbElement::registerForTouchEvents):
     32        (WebCore::SliderThumbElement::unregisterForTouchEvents):
     33        * rendering/RenderLayer.cpp:
     34        (WebCore::RenderLayer::registerAsTouchEventListenerForScrolling):
     35        (WebCore::RenderLayer::unregisterAsTouchEventListenerForScrolling):
     36
    1372017-04-03  Youenn Fablet  <youenn@apple.com>
    238

  • trunk/Source/WebCore/dom/EventNames.h

    r214627 r214819  
    320320
    321321    std::array<std::reference_wrapper<const AtomicString>, 5> touchEventNames() const;
     322    std::array<std::reference_wrapper<const AtomicString>, 3> gestureEventNames() const;
    322323
    323324private:
    … …  
    360361}
    361362
     363inline std::array<std::reference_wrapper<const AtomicString>, 3> EventNames::gestureEventNames() const
     364{
     365    return { { gesturestartEvent, gesturechangeEvent, gestureendEvent } };
     366}
     367
    362368#if ENABLE(GAMEPAD)
    363369

  • trunk/Source/WebCore/dom/Node.cpp

    r214702 r214819  
    316316    if (hasEventTargetData()) {
    317317        document.didRemoveWheelEventHandler(*this, EventHandlerRemoval::All);
     318#if ENABLE(TOUCH_EVENTS)
     319#if PLATFORM(IOS)
     320        document.removeTouchEventListener(*this, EventHandlerRemoval::All);
     321#endif
     322        document.didRemoveTouchEventHandler(*this, EventHandlerRemoval::All);
     323#endif
     324    }
     325
    318326#if ENABLE(TOUCH_EVENTS) && PLATFORM(IOS)
    319         document.removeTouchEventListener(this, true);
    320 #else
    321         // FIXME: This should call didRemoveTouchEventHandler().
    322 #endif
    323     }
    324 
    325 #if ENABLE(TOUCH_EVENTS) && PLATFORM(IOS)
    326     document.removeTouchEventHandler(this, true);
     327    document.removeTouchEventHandler(*this, EventHandlerRemoval::All);
    327328#endif
    328329
    … …  
    19201921    }
    19211922
    1922     unsigned numTouchEventHandlers = 0;
     1923    unsigned numTouchEventListeners = 0;
    19231924    for (auto& name : eventNames().touchEventNames())
    1924         numTouchEventHandlers += eventListeners(name).size();
    1925 
    1926     for (unsigned i = 0; i < numTouchEventHandlers; ++i) {
     1925        numTouchEventListeners += eventListeners(name).size();
     1926
     1927    for (unsigned i = 0; i < numTouchEventListeners; ++i) {
    19271928        oldDocument.didRemoveTouchEventHandler(*this);
    19281929        document().didAddTouchEventHandler(*this);
    1929     }
     1930
     1931#if ENABLE(TOUCH_EVENTS) && PLATFORM(IOS)
     1932        oldDocument.removeTouchEventListener(*this);
     1933        document().addTouchEventListener(*this);
     1934#endif
     1935    }
     1936
     1937#if ENABLE(TOUCH_EVENTS) && ENABLE(IOS_GESTURE_EVENTS)
     1938    unsigned numGestureEventListeners = 0;
     1939    for (auto& name : eventNames().gestureEventNames())
     1940        numGestureEventListeners += eventListeners(name).size();
     1941
     1942    for (unsigned i = 0; i < numGestureEventListeners; ++i) {
     1943        oldDocument.removeTouchEventHandler(*this);
     1944        document().addTouchEventHandler(*this);
     1945    }
     1946#endif
    19301947
    19311948    if (auto* registry = mutationObserverRegistry()) {
    … …  
    19381955            document().addMutationObserverTypes(registration->mutationTypes());
    19391956    }
     1957
     1958#if !ASSERT_DISABLED || ENABLE(SECURITY_ASSERTIONS)
     1959#if ENABLE(TOUCH_EVENTS) && PLATFORM(IOS)
     1960    ASSERT_WITH_SECURITY_IMPLICATION(!oldDocument.touchEventListenersContain(*this));
     1961    ASSERT_WITH_SECURITY_IMPLICATION(!oldDocument.touchEventHandlersContain(*this));
     1962#endif
     1963#if ENABLE(TOUCH_EVENTS) && ENABLE(IOS_GESTURE_EVENTS)
     1964    ASSERT_WITH_SECURITY_IMPLICATION(!oldDocument.touchEventTargetsContain(*this));
     1965#endif
     1966#endif
    19401967}
    19411968
    … …  
    19641991#if ENABLE(TOUCH_EVENTS)
    19651992    if (eventNames().isTouchEventType(eventType))
    1966         targetNode->document().addTouchEventListener(targetNode);
     1993        targetNode->document().addTouchEventListener(*targetNode);
    19671994#endif
    19681995#endif // PLATFORM(IOS)
    … …  
    19701997#if ENABLE(IOS_GESTURE_EVENTS) && ENABLE(TOUCH_EVENTS)
    19711998    if (eventNames().isGestureEventType(eventType))
    1972         targetNode->document().addTouchEventHandler(targetNode);
     1999        targetNode->document().addTouchEventHandler(*targetNode);
    19732000#endif
    19742001
    … …  
    20052032#if ENABLE(TOUCH_EVENTS)
    20062033    if (eventNames().isTouchEventType(eventType))
    2007         targetNode->document().removeTouchEventListener(targetNode);
     2034        targetNode->document().removeTouchEventListener(*targetNode);
    20082035#endif
    20092036#endif // PLATFORM(IOS)
    … …  
    20112038#if ENABLE(IOS_GESTURE_EVENTS) && ENABLE(TOUCH_EVENTS)
    20122039    if (eventNames().isGestureEventType(eventType))
    2013         targetNode->document().removeTouchEventHandler(targetNode);
     2040        targetNode->document().removeTouchEventHandler(*targetNode);
    20142041#endif
    20152042

  • trunk/Source/WebCore/html/shadow/SliderThumbElement.cpp

    r214291 r214819  
    550550    ASSERT(shouldAcceptTouchEvents());
    551551
    552     document().addTouchEventHandler(this);
     552    document().addTouchEventHandler(*this);
    553553    m_isRegisteredAsTouchEventListener = true;
    554554}
    … …  
    562562    stopDragging();
    563563
    564     document().removeTouchEventHandler(this);
     564    document().removeTouchEventHandler(*this);
    565565    m_isRegisteredAsTouchEventListener = false;
    566566}

  • trunk/Source/WebCore/rendering/RenderLayer.cpp

    r214640 r214819  
    21952195        return;
    21962196   
    2197     renderer().document().addTouchEventHandler(renderer().element());
     2197    renderer().document().addTouchEventHandler(*renderer().element());
    21982198    m_registeredAsTouchEventListenerForScrolling = true;
    21992199}
    … …  
    22042204        return;
    22052205
    2206     renderer().document().removeTouchEventHandler(renderer().element());
     2206    renderer().document().removeTouchEventHandler(*renderer().element());
    22072207    m_registeredAsTouchEventListenerForScrolling = false;
    22082208}
Note: See TracChangeset for help on using the changeset viewer.