RESOLVED LATER 136051
EventSender dispatches should be per-Document 
https://bugs.webkit.org/show_bug.cgi?id=136051
Summary EventSender dispatches should be per-Document
Brian Burg
Reported 2014-08-18 16:07:57 PDT
EventSender causes JS code to execute nondeterministically. It uses a Timer to asynchronously dispatch the same event to multiple elements of the same type on a future run loop. It also force-dispatches the event when the document closes. Currently, each (event name, element) pair gets its own static EventSender. This is quite nondeterministic because unrelated pages (say, the Inspector or another tab) share the same static EventSenders and can force-dispatch each others' pending events on implicitClose. I propose making EventSender instances per-Document. Then we can use ReplayableTimer to iron out the scheduling.
Attachments
Proposed Fix (31.36 KB, patch)
2014-08-18 18:56 PDT, Brian Burg 		no flags
DetailsFormatted DiffDiff
WIP (31.42 KB, patch)
2014-09-26 13:42 PDT, Brian Burg 		no flags
DetailsFormatted DiffDiff
Patch (40.54 KB, patch)
2014-09-26 15:04 PDT, Brian Burg 		no flags
DetailsFormatted DiffDiff
Proposed Fix (+win build fix) (40.98 KB, patch)
2014-10-01 16:04 PDT, Brian Burg 		kling: review+
commit-queue: commit-queue-
DetailsFormatted DiffDiff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
Brian Burg
Comment 1 2014-08-18 18:56:18 PDT
Created attachment 236793 [details] Proposed Fix I uploaded a patch to fix this. Looking forward to any feedback. I should probably write a replay test for this bug before landing, but the non-replay portions should be covered.
Brian Burg
Comment 2 2014-09-24 16:56:39 PDT
Splitting replay parts from the refactoring.
Brian Burg
Comment 3 2014-09-26 13:42:12 PDT
Created attachment 238735 [details] WIP
Brian Burg
Comment 4 2014-09-26 15:04:16 PDT
Created attachment 238741 [details] Patch
Brian Burg
Comment 5 2014-09-26 15:09:18 PDT
Comment on attachment 238741 [details] Patch To remove a source of nondeterminism for web replay, this patch makes EventSenders associated with the Document, without trying to overly optimize or re-architect how EventSender works. I am not equipped to make useful (ie., data driven) performance optimizations on this code.
Brian Burg
Comment 6 2014-10-01 16:04:51 PDT
Created attachment 239062 [details] Proposed Fix (+win build fix)
Andreas Kling
Comment 7 2014-10-01 16:09:40 PDT
Comment on attachment 239062 [details] Proposed Fix (+win build fix) r=me, thanks for taking care of this.
WebKit Commit Bot
Comment 8 2014-10-01 16:57:01 PDT
Comment on attachment 239062 [details] Proposed Fix (+win build fix) Rejecting attachment 239062 [details] from commit-queue. Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.appspot.com', '--bot-id=webkit-cq-03', 'validate-changelog', '--check-oops', '--non-interactive', 239062, '--port=mac']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit /Volumes/Data/EWS/WebKit/Source/WebCore/ChangeLog neither lists a valid reviewer nor contains the string "Unreviewed" or "Rubber stamp" (case insensitive). Full output: http://webkit-queues.appspot.com/results/5927535393112064
Chris Dumez
Comment 9 2014-10-01 16:58:25 PDT
Comment on attachment 239062 [details] Proposed Fix (+win build fix) View in context: https://bugs.webkit.org/attachment.cgi?id=239062&action=review > Source/WebCore/ChangeLog:5 > + You dropped the Reviewed By NOBODY line, which is why the commit queue complained about not being able to update that line.
Brian Burg
Comment 10 2014-10-01 17:54:15 PDT
Comment on attachment 239062 [details] Proposed Fix (+win build fix) View in context: https://bugs.webkit.org/attachment.cgi?id=239062&action=review >> Source/WebCore/ChangeLog:5 >> + > > You dropped the Reviewed By NOBODY line, which is why the commit queue complained about not being able to update that line. Weird, didn't think I touched that! Will fix and land by `webkit-patch land` later tonight.
Brian Burg
Comment 11 2014-10-03 12:21:53 PDT
Committed r174270: <http://trac.webkit.org/changeset/174270> But, needs to be rolled out as there are several crashes on bots.
Blaze Burg
Comment 12 2017-07-10 14:06:23 PDT
Closing web replay-related bugs until we resume working on the feature again.
Lucas Forschler
Comment 13 2019-02-06 09:19:06 PST
Mass move bugs into the DOM component.
Note You need to log in before you can comment on or make changes to this bug.