RESOLVED LATER 129451
Web Replay: make calls into FrameLoader::checkLoadComplete() deterministic 
https://bugs.webkit.org/show_bug.cgi?id=129451
Summary Web Replay: make calls into FrameLoader::checkLoadComplete() deterministic
Blaze Burg
Reported 2014-02-27 14:29:42 PST
I have implemented Timer workalike that: - when capturing, saves when it fires into the replay session - when replaying, doesn't schedule itself, and instead is called by EventLoopInputDispatcher - when neither capturing or replaying, it acts like a regular timer (by forwarding calls to an inner Timer instance). The implementation in my branch supports the basic Timer API, but it should also support SuspendableTimer so that it can be used to make DOMTimer deterministic as well.
Attachments
serialization of wip (21.01 KB, patch)
2014-03-28 14:41 PDT, Blaze Burg 		no flags
DetailsFormatted DiffDiff
the patch (49.63 KB, patch)
2014-04-03 18:19 PDT, Brian Burg 		no flags
DetailsFormatted DiffDiff
the patch (53.00 KB, patch)
2014-04-04 12:09 PDT, Brian Burg 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Blaze Burg
Comment 1 2014-03-28 14:41:07 PDT
Created attachment 228085 [details] serialization of wip
Brian Burg
Comment 2 2014-04-02 17:21:00 PDT
A deterministic timer implementation addresses two major sources of nondeterminism: 1) During replay, timers that could possibly cause DOM events to be fired should fire in the same order to ensure JS is triggered in the right order. 2) During replay, we similarly want DOMTimers to fire in the same order with respect to other timers and other run loops like mouse, keyboard, network. For the first version of ReplayableTimer I want to support single-shot timers. Later patches will teach ReplayableTimer to do intervals, and later work as a drop-in replacement of SuspendableTimer. Handling intervals is sufficient for most WebCore-internal timers affecting JS determinism, such as those in FrameLoader, DocumentEventQueue, EventSender, etc. Handling the SuspendableTimer interface is required for this implementation to be used for capture/replay of DOM timers. This will be a bit more work. An alternative (or stopgap) is to write custom replayable subclasses just for DOMTimer, and then switch off of those once ReplayableTimer learns how to deal with suspend. The "stopgap" approach can be seen here: https://github.com/burg/timelapse/blob/timelapse/Source/WebCore/page/DOMTimer.cpp I'm going to work on implementing repeatInterval and suspendable functionality, and if it proves to be more than a few days' work, then we can think about pushing on the DOMTimer stopgap first. It's really important to get DOMTimer to be deterministic ASAP, as it blocks any sort of error-checking and is used by every webpage.
Brian Burg
Comment 3 2014-04-03 18:19:31 PDT
Created attachment 228571 [details] the patch
WebKit Commit Bot
Comment 4 2014-04-03 18:21:56 PDT
Attachment 228571 [details] did not pass style-queue: ERROR: Source/WebCore/replay/ReplayableTimer.h:89: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebCore/replay/ReplayableTimer.h:120: Extra space before ( in function call [whitespace/parens] [4] Total errors found: 2 in 22 files If any of these errors are false positives, please file a bug against check-webkit-style.
Brian Burg
Comment 5 2014-04-04 12:09:25 PDT
Created attachment 228612 [details] the patch
WebKit Commit Bot
Comment 6 2014-04-04 12:10:32 PDT
Attachment 228612 [details] did not pass style-queue: ERROR: Source/WebCore/replay/ReplayableTimer.h:89: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebCore/replay/ReplayableTimer.h:120: Extra space before ( in function call [whitespace/parens] [4] Total errors found: 2 in 27 files If any of these errors are false positives, please file a bug against check-webkit-style.
Timothy Hatcher
Comment 7 2014-04-04 12:37:34 PDT
The patch looks fine to me. I'd like others to take a look. Why shouldn't / can't all Timers be replayable?
Brian Burg
Comment 8 2014-04-04 15:47:47 PDT
(In reply to comment #7) > The patch looks fine to me. I'd like others to take a look. > > Why shouldn't / can't all Timers be replayable? Most of them are irrelevant from a determinism point of view, so they would fill the recording with unnecessary data. The most important ones are those that could transitively dispatch DOM events or otherwise run JS code. I haven't benchmarked it, but ReplayableTimer is probably a little slower because it has to dig out an input cursor. It does an allocation if the cursor is capturing.
Timothy Hatcher
Comment 9 2014-08-05 11:55:07 PDT
Darin, maybe you can review this for Brian? I don't feel as comfortable reviewing it.
Blaze Burg
Comment 10 2015-08-05 11:29:29 PDT
Comment on attachment 228612 [details] the patch Clearing r? flag as this patch is bit-rotted.
Blaze Burg
Comment 11 2017-07-10 13:59:28 PDT
Closing web replay-related bugs until we resume working on the feature again.
Note You need to log in before you can comment on or make changes to this bug.