Crash in WebCore::UserGestureIndicator::processingUserGesture with WebWorkers
Created attachment 239753 [details]
Comment on attachment 239753 [details]
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.
> + if (m_frame.isMainFrame())
> + UserGestureIndicator::resetLastUserGestureTimestamp();
Did you check that this code runs when doing a cached back/forward load?
> 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]
Comment on attachment 239818 [details]
View in context: https://bugs.webkit.org/attachment.cgi?id=239818&action=review
Looks good to me. Please wait to EWS to agree :)
> + 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).
Committed r174700: <http://trac.webkit.org/changeset/174700>