Crash in WebCore::UserGestureIndicator::processingUserGesture with WebWorkers
Created attachment 239753 [details] Patch
Comment on attachment 239753 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=239753&action=review Can a regression test be added for this? I'm concerned that this is not right when there are multiple pages served by one process - interacting with one tab doesn't mean that we interacted with another. > Source/WebCore/loader/FrameLoader.cpp:1862 > + if (m_frame.isMainFrame()) > + UserGestureIndicator::resetLastUserGestureTimestamp(); Did you check that this code runs when doing a cached back/forward load?
<rdar://problem/15735049>
> Can a regression test be added for this? OK, so it looks like the answer is no, and that's because the crash was already fixed, probably in <http://trac.webkit.org/changeset/152238>, which was then polished in <http://trac.webkit.org/changeset/159198>. ScriptController::processingUserGesture() and UserGestureIndicator::processingUserGesture() are now very simple functions that can't crash. I still think that much of the refactoring done in this patch is needed - the code inEventTarget::fireEventListeners() is quite incorrect. Dispatching any arbitrary event shouldn't result in calling resetLastHandledUserGestureTimestamp(), as most events are not user gestures.
(In reply to comment #2) > > Source/WebCore/loader/FrameLoader.cpp:1862 > > + if (m_frame.isMainFrame()) > > + UserGestureIndicator::resetLastUserGestureTimestamp(); > > Did you check that this code runs when doing a cached back/forward load? In the refactored patch (about to be posted) we don't actually care about this any more.
Created attachment 239818 [details] Patch
Comment on attachment 239818 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=239818&action=review Looks good to me. Please wait to EWS to agree :) > Source/WebCore/dom/UserGestureIndicator.h:46 > + WEBCORE_EXPORT explicit UserGestureIndicator(ProcessingUserGestureState, Document* = nullptr); Maybe add a comment here, explaining that latest user event timestamp is updated in the document if provided?
Seems like this might be failing as a result: fast/dom/remove-body-during-body-replacement2.html
Actually, fast/dom/remove-body-during-body-replacement2.html regressed with r174637. And fast/hidpi/image-srcset-relative-svg-canvas.html has been very flaky on EWS recently (but not on regular bots).
Great!
Committed r174700: <http://trac.webkit.org/changeset/174700>