Bug 128633

Summary: Web Replay: capture and replay nondeterminism of Date.now() and Math.random()
Product: WebKit Reporter: BJ Burg <bburg>
Component: JavaScriptCoreAssignee: BJ Burg <bburg>
Status: RESOLVED FIXED    
Severity: Normal CC: fpizlo, ggaren, joepeck
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
patch fpizlo: review+

Description BJ Burg 2014-02-11 16:25:44 PST
These are the only sources of nondeterminism implemented in JSC that are visible to script (excluding VM bugs, etc).

For Math.random(), the seed given to WeakRandom can be memoized. For Date.now() and new Date(), we have to memoize each call.
Comment 1 BJ Burg 2014-02-11 17:25:03 PST
Created attachment 223920 [details]
patch
Comment 2 Filip Pizlo 2014-02-11 17:41:44 PST
Comment on attachment 223920 [details]
patch

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

R=me but please consider encapsulating the #if ENABLE(WEB_REPLAY) inside a clever macro.

> Source/JavaScriptCore/runtime/DateConstructor.cpp:128
> +#if ENABLE(WEB_REPLAY)
> +        value = deterministicCurrentTime(globalObject);
> +#else
>          value = jsCurrentTime();
> +#endif

Can't you have a macro like REPLAY(jsCurrentTime(), deterministicCurrentTime(globalObject))?

The reason why I would like that more is that #if's can often disturb our ability to visualize the nesting of control flow.  The definition of REPLAY would literally be:

#if ENABLE(WEB_REPLAY)
#define REPLAY(a, b) (b)
#else
#define REPLAY(a, b) (a)
#endif

You can call REPLAY() whatever you like...

> Source/JavaScriptCore/runtime/DateConstructor.cpp:214
> +#if ENABLE(WEB_REPLAY)
> +    return JSValue::encode(jsNumber(deterministicCurrentTime(exec->lexicalGlobalObject())));
> +#else
> +    UNUSED_PARAM(exec);
>      return JSValue::encode(jsNumber(jsCurrentTime()));
> +#endif

Another fine example of a place where that REPLAY() macro would simplify things.
Comment 3 BJ Burg 2014-02-12 16:07:45 PST
Comment on attachment 223920 [details]
patch

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

>> Source/JavaScriptCore/runtime/DateConstructor.cpp:128
>> +#endif
> 
> Can't you have a macro like REPLAY(jsCurrentTime(), deterministicCurrentTime(globalObject))?
> 
> The reason why I would like that more is that #if's can often disturb our ability to visualize the nesting of control flow.  The definition of REPLAY would literally be:
> 
> #if ENABLE(WEB_REPLAY)
> #define REPLAY(a, b) (b)
> #else
> #define REPLAY(a, b) (a)
> #endif
> 
> You can call REPLAY() whatever you like...

Sure thing, I can add that. It will only help out in cases where the difference is which function to call, but that's fine. (Most replay-oriented hooks add a branch without interleaving the normal code path.)
Comment 4 Joseph Pecoraro 2014-02-12 16:18:50 PST
Comment on attachment 223920 [details]
patch

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

> Source/JavaScriptCore/runtime/DateConstructor.cpp:88
> +    if (cursor.isCapturing())
> +        cursor.appendInput<GetCurrentTime>(currentTime);
> +
> +    if (cursor.isReplaying()) {

If a cursor cannot be both capturing and replaying then this can be an "else if" instead of another if. Or the states could move to an enum (Default, Capturing, Replaying).

> Source/JavaScriptCore/runtime/JSGlobalObject.cpp:793
> +    if (cursor.isCapturing())
> +        cursor.appendInput<SetRandomSeed>(m_weakRandom.seedUnsafe());
> +
> +    if (cursor.isReplaying()) {

Ditto
Comment 5 BJ Burg 2014-02-12 19:29:26 PST
Committed r164007: <http://trac.webkit.org/changeset/164007>