- Revision
- 174700
- Author
- [email protected]
- Date
- 2014-10-14 13:59:32 -0700 (Tue, 14 Oct 2014)
Log Message
Crash in WebCore::UserGestureIndicator::processingUserGesture with WebWorkers
https://bugs.webkit.org/show_bug.cgi?id=137676
<rdar://problem/15735049>
Reviewed by Alexey Proskuryakov.
Remove the code I added that tracks the timestamp of the most recent
user gesture from the event handling dispatch, as it was both
a silly place to do it and it originally crashed when events were fired from
Worker threads (although this was fixed in r152238).
It's now recorded by going through UserGestureIndicator, which is good because
it knows when a user has triggered an event. Its constructor now takes
a pointer to Document, and updates the timestamp there if necessary.
Not all UserGestureIndicator instances needed to reset the timestamp; Those did
not have to pass along the Document.
This is untestable due to the fix mentioned above.
* WebCore.exp.in: Change constructor signature.
* accessibility/AccessibilityNodeObject.cpp: Pass a pointer to the Document into the UserGestureIndicator.
(WebCore::AccessibilityNodeObject::increment):
(WebCore::AccessibilityNodeObject::decrement):
* accessibility/AccessibilityObject.cpp: Ditto.
(WebCore::AccessibilityObject::press):
* dom/Document.cpp:
(WebCore::Document::updateLastHandledUserGestureTimestamp): Renamed.
* dom/Document.h:
* dom/EventTarget.cpp: Remove the code to update the timestamp.
(WebCore::EventTarget::fireEventListeners):
* dom/UserGestureIndicator.cpp:
(WebCore::UserGestureIndicator::UserGestureIndicator): If there is a Document and
this is a user gesture, then reset the timestamp.
* dom/UserGestureIndicator.h:
* page/EventHandler.cpp: Pass a pointer to the Document.
(WebCore::EventHandler::handleMousePressEvent):
(WebCore::EventHandler::handleMouseDoubleClickEvent):
(WebCore::EventHandler::handleMouseReleaseEvent):
(WebCore::EventHandler::keyEvent):
(WebCore::EventHandler::handleTouchEvent):
* rendering/HitTestResult.cpp: Ditto.
(WebCore::HitTestResult::toggleMediaFullscreenState):
(WebCore::HitTestResult::enterFullscreenForVideo):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (174699 => 174700)
--- trunk/Source/WebCore/ChangeLog 2014-10-14 20:36:18 UTC (rev 174699)
+++ trunk/Source/WebCore/ChangeLog 2014-10-14 20:59:32 UTC (rev 174700)
@@ -1,3 +1,56 @@
+2014-10-14 Dean Jackson <[email protected]>
+
+ Crash in WebCore::UserGestureIndicator::processingUserGesture with WebWorkers
+ https://bugs.webkit.org/show_bug.cgi?id=137676
+ <rdar://problem/15735049>
+
+ Reviewed by Alexey Proskuryakov.
+
+ Remove the code I added that tracks the timestamp of the most recent
+ user gesture from the event handling dispatch, as it was both
+ a silly place to do it and it originally crashed when events were fired from
+ Worker threads (although this was fixed in r152238).
+
+ It's now recorded by going through UserGestureIndicator, which is good because
+ it knows when a user has triggered an event. Its constructor now takes
+ a pointer to Document, and updates the timestamp there if necessary.
+
+ Not all UserGestureIndicator instances needed to reset the timestamp; Those did
+ not have to pass along the Document.
+
+ This is untestable due to the fix mentioned above.
+
+ * WebCore.exp.in: Change constructor signature.
+
+ * accessibility/AccessibilityNodeObject.cpp: Pass a pointer to the Document into the UserGestureIndicator.
+ (WebCore::AccessibilityNodeObject::increment):
+ (WebCore::AccessibilityNodeObject::decrement):
+ * accessibility/AccessibilityObject.cpp: Ditto.
+ (WebCore::AccessibilityObject::press):
+
+ * dom/Document.cpp:
+ (WebCore::Document::updateLastHandledUserGestureTimestamp): Renamed.
+ * dom/Document.h:
+
+ * dom/EventTarget.cpp: Remove the code to update the timestamp.
+ (WebCore::EventTarget::fireEventListeners):
+
+ * dom/UserGestureIndicator.cpp:
+ (WebCore::UserGestureIndicator::UserGestureIndicator): If there is a Document and
+ this is a user gesture, then reset the timestamp.
+ * dom/UserGestureIndicator.h:
+
+ * page/EventHandler.cpp: Pass a pointer to the Document.
+ (WebCore::EventHandler::handleMousePressEvent):
+ (WebCore::EventHandler::handleMouseDoubleClickEvent):
+ (WebCore::EventHandler::handleMouseReleaseEvent):
+ (WebCore::EventHandler::keyEvent):
+ (WebCore::EventHandler::handleTouchEvent):
+
+ * rendering/HitTestResult.cpp: Ditto.
+ (WebCore::HitTestResult::toggleMediaFullscreenState):
+ (WebCore::HitTestResult::enterFullscreenForVideo):
+
2014-10-14 Brent Fulgham <[email protected]>
[Win] Unreviewed gardening. Ignore Visual Studio *.sdf files.
Modified: trunk/Source/WebCore/WebCore.exp.in (174699 => 174700)
--- trunk/Source/WebCore/WebCore.exp.in 2014-10-14 20:36:18 UTC (rev 174699)
+++ trunk/Source/WebCore/WebCore.exp.in 2014-10-14 20:59:32 UTC (rev 174700)
@@ -890,7 +890,7 @@
__ZN7WebCore20TransformationMatrix5scaleEd
__ZN7WebCore20TransformationMatrix9translateEdd
__ZN7WebCore20UserGestureIndicator7s_stateE
-__ZN7WebCore20UserGestureIndicatorC1ENS_26ProcessingUserGestureStateE
+__ZN7WebCore20UserGestureIndicatorC1ENS_26ProcessingUserGestureStateEPNS_8DocumentE
__ZN7WebCore20UserGestureIndicatorD1Ev
__ZN7WebCore20deleteEmptyDirectoryERKN3WTF6StringE
__ZN7WebCore20looksLikeAbsoluteURLEP8NSString
Modified: trunk/Source/WebCore/accessibility/AccessibilityNodeObject.cpp (174699 => 174700)
--- trunk/Source/WebCore/accessibility/AccessibilityNodeObject.cpp 2014-10-14 20:36:18 UTC (rev 174699)
+++ trunk/Source/WebCore/accessibility/AccessibilityNodeObject.cpp 2014-10-14 20:59:32 UTC (rev 174700)
@@ -1076,13 +1076,13 @@
void AccessibilityNodeObject::increment()
{
- UserGestureIndicator gestureIndicator(DefinitelyProcessingUserGesture);
+ UserGestureIndicator gestureIndicator(DefinitelyProcessingUserGesture, document());
alterSliderValue(true);
}
void AccessibilityNodeObject::decrement()
{
- UserGestureIndicator gestureIndicator(DefinitelyProcessingUserGesture);
+ UserGestureIndicator gestureIndicator(DefinitelyProcessingUserGesture, document());
alterSliderValue(false);
}
Modified: trunk/Source/WebCore/accessibility/AccessibilityObject.cpp (174699 => 174700)
--- trunk/Source/WebCore/accessibility/AccessibilityObject.cpp 2014-10-14 20:36:18 UTC (rev 174699)
+++ trunk/Source/WebCore/accessibility/AccessibilityObject.cpp 2014-10-14 20:59:32 UTC (rev 174700)
@@ -845,7 +845,8 @@
// Hit test at this location to determine if there is a sub-node element that should act
// as the target of the action.
Element* hitTestElement = nullptr;
- if (Document* document = this->document()) {
+ Document* document = this->document();
+ if (document) {
HitTestRequest request(HitTestRequest::ReadOnly | HitTestRequest::Active | HitTestRequest::AccessibilityHitTest);
HitTestResult hitTestResult(clickPoint());
document->renderView()->hitTest(request, hitTestResult);
@@ -866,7 +867,7 @@
if (hitTestElement && hitTestElement->isDescendantOf(pressElement))
pressElement = hitTestElement;
- UserGestureIndicator gestureIndicator(DefinitelyProcessingUserGesture);
+ UserGestureIndicator gestureIndicator(DefinitelyProcessingUserGesture, document);
pressElement->accessKeyAction(true);
return true;
}
Modified: trunk/Source/WebCore/dom/Document.cpp (174699 => 174700)
--- trunk/Source/WebCore/dom/Document.cpp 2014-10-14 20:36:18 UTC (rev 174699)
+++ trunk/Source/WebCore/dom/Document.cpp 2014-10-14 20:59:32 UTC (rev 174700)
@@ -5911,8 +5911,7 @@
}
}
#endif
-
-void Document::resetLastHandledUserGestureTimestamp()
+void Document::updateLastHandledUserGestureTimestamp()
{
m_lastHandledUserGestureTimestamp = monotonicallyIncreasingTime();
}
Modified: trunk/Source/WebCore/dom/Document.h (174699 => 174700)
--- trunk/Source/WebCore/dom/Document.h 2014-10-14 20:36:18 UTC (rev 174699)
+++ trunk/Source/WebCore/dom/Document.h 2014-10-14 20:59:32 UTC (rev 174700)
@@ -1202,7 +1202,7 @@
WEBCORE_EXPORT void didRemoveWheelEventHandler();
double lastHandledUserGestureTimestamp() const { return m_lastHandledUserGestureTimestamp; }
- void resetLastHandledUserGestureTimestamp();
+ void updateLastHandledUserGestureTimestamp();
#if ENABLE(TOUCH_EVENTS)
bool hasTouchEventHandlers() const { return (m_touchEventTargets.get()) ? m_touchEventTargets->size() : false; }
Modified: trunk/Source/WebCore/dom/EventTarget.cpp (174699 => 174700)
--- trunk/Source/WebCore/dom/EventTarget.cpp 2014-10-14 20:36:18 UTC (rev 174699)
+++ trunk/Source/WebCore/dom/EventTarget.cpp 2014-10-14 20:59:32 UTC (rev 174700)
@@ -213,7 +213,6 @@
// Also, don't fire event listeners added during event dispatch. Conveniently, all new event listeners will be added
// after or at index |size|, so iterating up to (but not including) |size| naturally excludes new event listeners.
- bool userEventWasHandled = false;
size_t i = 0;
size_t size = entry.size();
if (!d->firingEventIterators)
@@ -244,13 +243,9 @@
// To match Mozilla, the AT_TARGET phase fires both capturing and bubbling
// event listeners, even though that violates some versions of the DOM spec.
registeredListener.listener->handleEvent(context, event);
- if (!userEventWasHandled && ScriptController::processingUserGesture())
- userEventWasHandled = true;
InspectorInstrumentation::didHandleEvent(cookie);
}
d->firingEventIterators->removeLast();
- if (userEventWasHandled && document)
- document->resetLastHandledUserGestureTimestamp();
if (document)
InspectorInstrumentation::didDispatchEvent(willDispatchEventCookie);
Modified: trunk/Source/WebCore/dom/UserGestureIndicator.cpp (174699 => 174700)
--- trunk/Source/WebCore/dom/UserGestureIndicator.cpp 2014-10-14 20:36:18 UTC (rev 174699)
+++ trunk/Source/WebCore/dom/UserGestureIndicator.cpp 2014-10-14 20:59:32 UTC (rev 174700)
@@ -26,6 +26,7 @@
#include "config.h"
#include "UserGestureIndicator.h"
+#include "Document.h"
#include <wtf/MainThread.h>
namespace WebCore {
@@ -37,7 +38,7 @@
ProcessingUserGestureState UserGestureIndicator::s_state = DefinitelyNotProcessingUserGesture;
-UserGestureIndicator::UserGestureIndicator(ProcessingUserGestureState state)
+UserGestureIndicator::UserGestureIndicator(ProcessingUserGestureState state, Document* document)
: m_previousState(s_state)
{
// Silently ignore UserGestureIndicators on non main threads.
@@ -47,6 +48,9 @@
if (isDefinite(state))
s_state = state;
ASSERT(isDefinite(s_state));
+
+ if (document && s_state == DefinitelyProcessingUserGesture)
+ document->topDocument().updateLastHandledUserGestureTimestamp();
}
UserGestureIndicator::~UserGestureIndicator()
Modified: trunk/Source/WebCore/dom/UserGestureIndicator.h (174699 => 174700)
--- trunk/Source/WebCore/dom/UserGestureIndicator.h 2014-10-14 20:36:18 UTC (rev 174699)
+++ trunk/Source/WebCore/dom/UserGestureIndicator.h 2014-10-14 20:59:32 UTC (rev 174700)
@@ -30,6 +30,8 @@
namespace WebCore {
+class Document;
+
enum ProcessingUserGestureState {
DefinitelyProcessingUserGesture,
PossiblyProcessingUserGesture,
@@ -41,10 +43,10 @@
public:
static bool processingUserGesture();
- WEBCORE_EXPORT explicit UserGestureIndicator(ProcessingUserGestureState);
+ // If a document is provided, its last known user gesture timestamp is updated.
+ WEBCORE_EXPORT explicit UserGestureIndicator(ProcessingUserGestureState, Document* = nullptr);
WEBCORE_EXPORT ~UserGestureIndicator();
-
private:
WEBCORE_EXPORT static ProcessingUserGestureState s_state;
ProcessingUserGestureState m_previousState;
Modified: trunk/Source/WebCore/page/EventHandler.cpp (174699 => 174700)
--- trunk/Source/WebCore/page/EventHandler.cpp 2014-10-14 20:36:18 UTC (rev 174699)
+++ trunk/Source/WebCore/page/EventHandler.cpp 2014-10-14 20:59:32 UTC (rev 174700)
@@ -1653,7 +1653,7 @@
return true;
#endif
- UserGestureIndicator gestureIndicator(DefinitelyProcessingUserGesture);
+ UserGestureIndicator gestureIndicator(DefinitelyProcessingUserGesture, m_frame.document());
// FIXME (bug 68185): this call should be made at another abstraction layer
m_frame.loader().resetMultipleFormSubmissionProtection();
@@ -1790,7 +1790,7 @@
m_frame.selection().setCaretBlinkingSuspended(false);
- UserGestureIndicator gestureIndicator(DefinitelyProcessingUserGesture);
+ UserGestureIndicator gestureIndicator(DefinitelyProcessingUserGesture, m_frame.document());
// We get this instead of a second mouse-up
m_mousePressed = false;
@@ -2035,7 +2035,7 @@
return true;
#endif
- UserGestureIndicator gestureIndicator(DefinitelyProcessingUserGesture);
+ UserGestureIndicator gestureIndicator(DefinitelyProcessingUserGesture, m_frame.document());
#if ENABLE(PAN_SCROLLING)
m_autoscrollController->handleMouseReleaseEvent(platformMouseEvent);
@@ -3071,7 +3071,7 @@
if (!element)
return false;
- UserGestureIndicator gestureIndicator(DefinitelyProcessingUserGesture);
+ UserGestureIndicator gestureIndicator(DefinitelyProcessingUserGesture, m_frame.document());
UserTypingGestureIndicator typingGestureIndicator(m_frame);
if (FrameView* view = m_frame.view())
@@ -3799,7 +3799,7 @@
const Vector<PlatformTouchPoint>& points = event.touchPoints();
- UserGestureIndicator gestureIndicator(DefinitelyProcessingUserGesture);
+ UserGestureIndicator gestureIndicator(DefinitelyProcessingUserGesture, m_frame.document());
unsigned i;
bool freshTouchEvents = true;
Modified: trunk/Source/WebCore/rendering/HitTestResult.cpp (174699 => 174700)
--- trunk/Source/WebCore/rendering/HitTestResult.cpp 2014-10-14 20:36:18 UTC (rev 174699)
+++ trunk/Source/WebCore/rendering/HitTestResult.cpp 2014-10-14 20:59:32 UTC (rev 174700)
@@ -414,7 +414,7 @@
#if ENABLE(VIDEO)
if (HTMLMediaElement* mediaElement = this->mediaElement()) {
if (mediaElement->isVideo() && mediaElement->supportsFullscreen()) {
- UserGestureIndicator indicator(DefinitelyProcessingUserGesture);
+ UserGestureIndicator indicator(DefinitelyProcessingUserGesture, &mediaElement->document());
mediaElement->toggleFullscreenState();
}
}
@@ -428,7 +428,7 @@
if (is<HTMLVideoElement>(mediaElement)) {
HTMLVideoElement& videoElement = downcast<HTMLVideoElement>(*mediaElement);
if (!videoElement.isFullscreen() && mediaElement->supportsFullscreen()) {
- UserGestureIndicator indicator(DefinitelyProcessingUserGesture);
+ UserGestureIndicator indicator(DefinitelyProcessingUserGesture, &mediaElement->document());
videoElement.enterFullscreen();
}
}
