| Summary: | Web Replay: capture and replay nondeterminism of Date.now() and Math.random() | ||||||
|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | BJ Burg <bburg> | ||||
| Component: | JavaScriptCore | Assignee: | BJ Burg <bburg> | ||||
| Status: | RESOLVED FIXED | ||||||
| Severity: | Normal | CC: | fpizlo, ggaren, joepeck | ||||
| Priority: | P2 | ||||||
| Version: | 528+ (Nightly build) | ||||||
| Hardware: | Unspecified | ||||||
| OS: | Unspecified | ||||||
| Attachments: |
|
||||||
|
Description
BJ Burg
2014-02-11 16:25:44 PST
Created attachment 223920 [details]
patch
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 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 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 Committed r164007: <http://trac.webkit.org/changeset/164007> |