Skip to content

GitLab

T
tangled
Commit 61e613b1 authored by skyostil@chromium.org's avatar skyostil@chromium.org
Browse files
Options

Only unregister touch action handler conditionally 

When a RenderObject with a non-default touch action is about to be
destroyed, we remove the fake event handler that implements the touch
action from the event handler registry. However the handler may have
already been removed previously by the host Document (e.g., from
Document::open()), so this code can cause the same handler to get
removed twice.

This patch fixes the issue by only removing the handler conditionally
if it is still there.

BUG=419420

Review URL: https://codereview.chromium.org/647503002

git-svn-id: svn://svn.chromium.org/blink/trunk@183743 bbb929c8-8fbe-4397-9dbb-9b2b20218538
parent 9f781570
Hide whitespace changes
Inline Side-by-side
Showing with 38 additions and 6 deletions
third_party/WebKit/LayoutTests/fast/events/touch/touch-action-double-remove.html 0 → 100644
View file @ 61e613b1
<!DOCTYPE html>
<style>
br {
touch-action: none;
}
</style>
<!--
This is a regression test against a double unregistration of a touch action
event handler on a <br> element. When the document finishes loading and is
attached to the view, all of its RenderObjects are destroyed and recreated. If
a RenderObject had a touch action associated with it, it will need to remove
the fake event handler it registered for it in the EventHandlerRegistry.
However, when the document gets detached, all of the event handlers within it
are automatically unregistered. When the RenderObject tries to unregister its
own handler after the fact, it is no longer there and we hit an assertion
failure.
Note that this test doesn't use js-test.js because it also inadvertedly
triggers the same bug by appending <br> elements to the document.
The test passes if it doesn't trigger an assertion failure in a debug build.
-->
<script>
if (window.testRunner)
testRunner.dumpAsText();
</script>
PASS<br>
third_party/WebKit/Source/core/rendering/RenderObject.cpp
View file @ 61e613b1
...@@ -2331,12 +2331,15 @@ void RenderObject::willBeDestroyed() ...@@ -2331,12 +2331,15 @@ void RenderObject::willBeDestroyed()
if (hasCounterNodeMap()) if (hasCounterNodeMap())
RenderCounter::destroyCounterNodes(*this); RenderCounter::destroyCounterNodes(*this);
// Remove the handler if node had touch-action set. Don't call when // Remove the handler if node had touch-action set. Handlers are not added
// document is being destroyed as all handlers will have been cleared // for text nodes so don't try removing for one too. Need to check if
// previously. Handlers are not added for text nodes so don't try removing // m_style is null in cases of partial construction. Any handler we added
// for one too. Need to check if m_style is null in cases of partial construction. // previously may have already been removed by the Document independently.
if (!documentBeingDestroyed() && node() && !node()->isTextNode() && m_style && m_style->touchAction() != TouchActionAuto) if (node() && !node()->isTextNode() && m_style && m_style->touchAction() != TouchActionAuto) {
document().frameHost()->eventHandlerRegistry().didRemoveEventHandler(*node(), EventHandlerRegistry::TouchEvent); EventHandlerRegistry& registry = document().frameHost()->eventHandlerRegistry();
if (registry.eventHandlerTargets(EventHandlerRegistry::TouchEvent)->contains(node()))
registry.didRemoveEventHandler(*node(), EventHandlerRegistry::TouchEvent);
}
setAncestorLineBoxDirty(false); setAncestorLineBoxDirty(false);
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment