Bug 135295 – Web Replay: dispatch timing information should be stored out-of-line in a replay segment

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Brian Burg]

We need to save a timestamp for each event loop input so that replay can simulate the original user and network delays. Currently that timestamp is stored on each event loop input instance.

I argue that it should be stored in a separate vector attached to the segment rather than as a member variable of some NondeterministicInput subclasses. This will make the event loop input class immutable, and make it easier to modify or add auxiliary debugging data in the future (such as how many DOM events we should expect per user action).

[Brian Burg]

Created attachment 235528 [details]
Patch

[Brian Burg]

Comment on attachment 235528 [details]
Patch

This patch is actually ready for review, don't know why I marked it WIP.

[Timothy Hatcher]

Comment on attachment 235528 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=235528&action=review

> Source/WebCore/replay/CapturingInputCursor.cpp:47
> +    UNUSED_PARAM(page);

I don't think this is needed since it was used when setting m_page.

> Source/WebCore/replay/CapturingInputCursor.cpp:66
> +        // FIXME: rewrite this (and related dispatch code) to use std::chrono.

Are we using std::chrono in other places now?

[Brian Burg]

Comment on attachment 235528 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=235528&action=review

>> Source/WebCore/replay/CapturingInputCursor.cpp:47
>> +    UNUSED_PARAM(page);
> 
> I don't think this is needed since it was used when setting m_page.

I think clang complains about m_page never being used. On my branch, I just removed the field and the various plumbing of it through constructors. (I guess that's why it was WIP? nothing else is different)

>> Source/WebCore/replay/CapturingInputCursor.cpp:66
>> +        // FIXME: rewrite this (and related dispatch code) to use std::chrono.
> 
> Are we using std::chrono in other places now?

It is andersca's New Favorite Thing. I haven't had time to figure it out, but it's supposedly good stuff.

[Brian Burg]

Created attachment 236122 [details]
Remove unused m_page member

[WebKit Commit Bot]

Comment on attachment 236122 [details]
Remove unused m_page member

Clearing flags on attachment: 236122

Committed r172180: <http://trac.webkit.org/changeset/172180>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.