Changeset 182349 in webkit


Ignore:
Timestamp:
Apr 4, 2015 5:51:53 PM (9 years ago)
Author:
Simon Fraser
Message:

Crash under Document::absoluteRegionForEventTargets on build.webkit.org/dashboard
​https://bugs.webkit.org/show_bug.cgi?id=143406
rdar://problem/20407080

Reviewed by Ryosuke Niwa.

Source/WebCore:

We failed to remove elements from Document's m_wheelEventTargets HashSet when the
elements were destroyed with wheel handlers still on them. Fix by removing the
node from the set via Node::willBeDeletedFrom().

Tests: platform/mac-wk2/tiled-drawing/scrolling/non-fast-region/destroy-element-with-multiple-handlers-crash.html

platform/mac-wk2/tiled-drawing/scrolling/non-fast-region/destroy-wheel-element-crash.html
platform/mac-wk2/tiled-drawing/scrolling/non-fast-region/destroy-wheel-element-parent-crash.html

  • dom/Document.cpp:

(WebCore::removeHandlerFromSet): Helper to remove one or all handlers on the given node.
(WebCore::Document::didRemoveWheelEventHandler): Add a parameter to specify whether we're
removing all handlers on the given node.
(WebCore::Document::didRemoveTouchEventHandler): Use removeHandlerFromSet().

  • dom/Document.h:
  • dom/Node.cpp:

(WebCore::Node::willBeDeletedFrom): Tell the document we're removing all handlers
for this node.

LayoutTests:

Test configurations of elements with different parenting and event handlers adding orders, and multiple handlers on
the same node.

  • platform/mac-wk2/tiled-drawing/scrolling/non-fast-region/destroy-element-with-multiple-handlers-crash-expected.txt: Added.
  • platform/mac-wk2/tiled-drawing/scrolling/non-fast-region/destroy-element-with-multiple-handlers-crash.html: Added.
  • platform/mac-wk2/tiled-drawing/scrolling/non-fast-region/destroy-wheel-element-crash-expected.txt: Added.
  • platform/mac-wk2/tiled-drawing/scrolling/non-fast-region/destroy-wheel-element-crash.html: Added.
  • platform/mac-wk2/tiled-drawing/scrolling/non-fast-region/destroy-wheel-element-parent-crash-expected.txt: Added.
  • platform/mac-wk2/tiled-drawing/scrolling/non-fast-region/destroy-wheel-element-parent-crash.html: Added.
Location:
trunk
Files:
6 added
5 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r182346 r182349  
     12015-04-04  Simon Fraser  <simon.fraser@apple.com>
     2
     3        Crash under Document::absoluteRegionForEventTargets on build.webkit.org/dashboard
     4        https://bugs.webkit.org/show_bug.cgi?id=143406
     5        rdar://problem/20407080
     6
     7        Reviewed by Ryosuke Niwa.
     8       
     9        Test configurations of elements with different parenting and event handlers adding orders, and multiple handlers on
     10        the same node.
     11
     12        * platform/mac-wk2/tiled-drawing/scrolling/non-fast-region/destroy-element-with-multiple-handlers-crash-expected.txt: Added.
     13        * platform/mac-wk2/tiled-drawing/scrolling/non-fast-region/destroy-element-with-multiple-handlers-crash.html: Added.
     14        * platform/mac-wk2/tiled-drawing/scrolling/non-fast-region/destroy-wheel-element-crash-expected.txt: Added.
     15        * platform/mac-wk2/tiled-drawing/scrolling/non-fast-region/destroy-wheel-element-crash.html: Added.
     16        * platform/mac-wk2/tiled-drawing/scrolling/non-fast-region/destroy-wheel-element-parent-crash-expected.txt: Added.
     17        * platform/mac-wk2/tiled-drawing/scrolling/non-fast-region/destroy-wheel-element-parent-crash.html: Added.
     18
    1192015-04-04  Simon Fraser  <simon.fraser@apple.com>
    220

  • trunk/Source/WebCore/ChangeLog

    r182346 r182349  
     12015-04-04  Simon Fraser  <simon.fraser@apple.com>
     2
     3        Crash under Document::absoluteRegionForEventTargets on build.webkit.org/dashboard
     4        https://bugs.webkit.org/show_bug.cgi?id=143406
     5        rdar://problem/20407080
     6
     7        Reviewed by Ryosuke Niwa.
     8       
     9        We failed to remove elements from Document's m_wheelEventTargets HashSet when the
     10        elements were destroyed with wheel handlers still on them. Fix by removing the
     11        node from the set via Node::willBeDeletedFrom().
     12
     13        Tests: platform/mac-wk2/tiled-drawing/scrolling/non-fast-region/destroy-element-with-multiple-handlers-crash.html
     14               platform/mac-wk2/tiled-drawing/scrolling/non-fast-region/destroy-wheel-element-crash.html
     15               platform/mac-wk2/tiled-drawing/scrolling/non-fast-region/destroy-wheel-element-parent-crash.html
     16
     17        * dom/Document.cpp:
     18        (WebCore::removeHandlerFromSet): Helper to remove one or all handlers on the given node.
     19        (WebCore::Document::didRemoveWheelEventHandler): Add a parameter to specify whether we're
     20        removing all handlers on the given node.
     21        (WebCore::Document::didRemoveTouchEventHandler): Use removeHandlerFromSet().
     22        * dom/Document.h:
     23        * dom/Node.cpp:
     24        (WebCore::Node::willBeDeletedFrom): Tell the document we're removing all handlers
     25        for this node.
     26
    1272015-04-04  Simon Fraser  <simon.fraser@apple.com>
    228

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

    r182271 r182349  
    60046004}
    60056005
    6006 void Document::didRemoveWheelEventHandler(Node& node)
     6006static bool removeHandlerFromSet(EventTargetSet& handlerSet, Node& node, EventHandlerRemoval removal)
     6007{
     6008    switch (removal) {
     6009    case EventHandlerRemoval::One:
     6010        return handlerSet.remove(&node);
     6011    case EventHandlerRemoval::All:
     6012        return handlerSet.removeAll(&node);
     6013    }
     6014    return false;
     6015}
     6016
     6017void Document::didRemoveWheelEventHandler(Node& node, EventHandlerRemoval removal)
    60076018{
    60086019    if (!m_wheelEventTargets)
    60096020        return;
    60106021
    6011     ASSERT(m_wheelEventTargets->contains(&node));
    6012     m_wheelEventTargets->remove(&node);
     6022    if (!removeHandlerFromSet(*m_wheelEventTargets, node, removal))
     6023        return;
    60136024
    60146025    if (Document* parent = parentDocument()) {
    … …  
    60576068}
    60586069
    6059 void Document::didRemoveTouchEventHandler(Node& handler)
     6070void Document::didRemoveTouchEventHandler(Node& handler, EventHandlerRemoval removal)
    60606071{
    60616072#if ENABLE(TOUCH_EVENTS)
    … …  
    60636074        return;
    60646075
    6065     ASSERT(m_touchEventTargets->contains(&handler));
    6066     m_touchEventTargets->remove(&handler);
     6076    removeHandlerFromSet(*m_touchEventTargets, handler, removal))
    60676077
    60686078    if (Document* parent = parentDocument()) {
    … …  
    60856095#else
    60866096    UNUSED_PARAM(handler);
     6097    UNUSED_PARAM(removal);
    60876098#endif
    60886099}
    … …  
    61406151    for (auto it : *targets) {
    61416152        LayoutRect rootRelativeBounds;
    6142        
     6153
    61436154        if (is<Document>(it.key)) {
    61446155            Document* document = downcast<Document>(it.key);

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

    r182242 r182349  
    241241const int numNodeListInvalidationTypes = InvalidateOnAnyAttrChange + 1;
    242242
     243enum class EventHandlerRemoval { One, All };
    243244typedef HashCountedSet<Node*> EventTargetSet;
    244245
    … …  
    11241125
    11251126    void didAddWheelEventHandler(Node&);
    1126     void didRemoveWheelEventHandler(Node&);
     1127    void didRemoveWheelEventHandler(Node&, EventHandlerRemoval = EventHandlerRemoval::One);
    11271128
    11281129    double lastHandledUserGestureTimestamp() const { return m_lastHandledUserGestureTimestamp; }
    … …  
    11401141
    11411142    void didAddTouchEventHandler(Node&);
    1142     void didRemoveTouchEventHandler(Node&);
     1143    void didRemoveTouchEventHandler(Node&, EventHandlerRemoval = EventHandlerRemoval::One);
    11431144
    11441145    void didRemoveEventTargetNode(Node&);

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

    r181514 r182349  
    329329{
    330330    if (hasEventTargetData()) {
     331        document.didRemoveWheelEventHandler(*this, EventHandlerRemoval::All);
    331332#if ENABLE(TOUCH_EVENTS) && PLATFORM(IOS)
    332333        document.removeTouchEventListener(this, true);
     334#else
     335        // FIXME: This should call didRemoveTouchEventHandler().
    333336#endif
    334337        clearEventTargetData();
Note: See TracChangeset for help on using the changeset viewer.