The first patch of many.. This one includes the base classes for captured nondeterministic inputs, and the machinery to set the input cursor (which mediates storing and loading inputs) on a JSGlobalObject or Document.
Created attachment 223013 [details] patch
Attachment 223013 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/replay/NondeterministicInput.h:69: This { should be at the end of the previous line [whitespace/braces] [4] ERROR: Source/JavaScriptCore/replay/NondeterministicInput.h:74: This { should be at the end of the previous line [whitespace/braces] [4] ERROR: Source/JavaScriptCore/replay/EmptyInputCursor.h:52: This { should be at the end of the previous line [whitespace/braces] [4] ERROR: Source/JavaScriptCore/replay/EmptyInputCursor.h:58: This { should be at the end of the previous line [whitespace/braces] [4] ERROR: Source/JavaScriptCore/replay/EmptyInputCursor.h:64: This { should be at the end of the previous line [whitespace/braces] [4] Total errors found: 5 in 15 files If any of these errors are false positives, please file a bug against check-webkit-style.
Filed bug against style checker: https://bugs.webkit.org/show_bug.cgi?id=128119
Comment on attachment 223013 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=223013&action=review r=me, I only have style comments and nits. I may have more questions once these get used and when ReplayController is added. > Source/JavaScriptCore/replay/EmptyInputCursor.h:32 > +#include <replay/InputCursor.h> Nit: You should be able to just do #include "InputCursor.h" and all ports should build fine. > Source/JavaScriptCore/replay/EmptyInputCursor.h:38 > +class EmptyInputCursor : public InputCursor { Nit: Easier and cleaner to put "final" here in the class definition then on every single method. class EmptyInputCursor final : public InputCursor > Source/JavaScriptCore/replay/EmptyInputCursor.h:45 > + return adoptRef(new EmptyInputCursor()); Style: Parens not needed. "new EmptyInputCursor" > Source/JavaScriptCore/replay/InputCursor.h:48 > + virtual bool isCapturing() const =0; > + virtual bool isReplaying() const =0; Style: "= 0;", here and elsewhere. > Source/JavaScriptCore/replay/InputCursor.h:53 > + return storeInput(std::unique_ptr<NondeterministicInputBase>(WTF::safeCast<InputType*>(new InputType(std::forward<Args>(args)...)))); Maybe break this up into 2 lines so it is easier to read? InputType* inputType = WTF::safeCast<InputType*>(blah); return storeInput(std::unique_ptr<NondeterministicInputBase>(inputType)); > Source/JavaScriptCore/replay/NondeterministicInput.h:67 > +class NondeterministicInput : public NondeterministicInputBase { Nit: final in class > Source/WebCore/WebCore.xcodeproj/project.pbxproj:17859 > + name = replay; Nit: This should be "path = replay". That way, if you want to add files to this group in Xcode, Xcode automatically opens up the Source/WebCore/replay folder instead of the project base Source/WebCore folder. You can change this in Xcode by selecting the Group, showing the right sidebar, and setting the Relative to Group path. You'll need to re-add the files inside the group. Better to do this now while there is 1 file then when there are 10+! > Source/WebCore/replay/EventLoopInput.h:38 > +namespace JSC { > +class EncodedValue; > +} Nit: This forward declaration is unneeded, as it is guaranteed by the base header. > Source/WebCore/replay/EventLoopInput.h:46 > + explicit ReplayPosition() Nit: explicit not needed here. > Source/WebCore/replay/EventLoopInput.h:61 > +class EventLoopInputBase : public NondeterministicInputBase { Nit: final in class. > Source/WebCore/replay/EventLoopInput.h:80 > + virtual const AtomicString& type() const override final { Style: brace on its own line.
Comment on attachment 223013 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=223013&action=review >> Source/JavaScriptCore/replay/EmptyInputCursor.h:32 >> +#include <replay/InputCursor.h> > > Nit: You should be able to just do #include "InputCursor.h" and all ports should build fine. Oops, nice catch. This class grew up in WebCore. >> Source/JavaScriptCore/replay/EmptyInputCursor.h:45 >> + return adoptRef(new EmptyInputCursor()); > > Style: Parens not needed. "new EmptyInputCursor" Filed against check-webkit-style: https://bugs.webkit.org/show_bug.cgi?id=128134 >> Source/JavaScriptCore/replay/InputCursor.h:48 >> + virtual bool isReplaying() const =0; > > Style: "= 0;", here and elsewhere. Filed against check-webkit-style: https://bugs.webkit.org/show_bug.cgi?id=128134 >> Source/JavaScriptCore/replay/InputCursor.h:53 >> + return storeInput(std::unique_ptr<NondeterministicInputBase>(WTF::safeCast<InputType*>(new InputType(std::forward<Args>(args)...)))); > > Maybe break this up into 2 lines so it is easier to read? > > InputType* inputType = WTF::safeCast<InputType*>(blah); > return storeInput(std::unique_ptr<NondeterministicInputBase>(inputType)); OK >> Source/WebCore/WebCore.xcodeproj/project.pbxproj:17859 >> + name = replay; > > Nit: This should be "path = replay". That way, if you want to add files to this group in Xcode, Xcode automatically opens up the Source/WebCore/replay folder instead of the project base Source/WebCore folder. > > You can change this in Xcode by selecting the Group, showing the right sidebar, and setting the Relative to Group path. You'll need to re-add the files inside the group. Better to do this now while there is 1 file then when there are 10+! I could never figure out how to make it do this. Thanks! I wrote up some of the steps details in the wiki, since I seem to forget frequently. https://trac.webkit.org/wiki/AddingFiles >> Source/WebCore/replay/EventLoopInput.h:61 >> +class EventLoopInputBase : public NondeterministicInputBase { > > Nit: final in class. dispatch() is overridden by every subclass of EventLoopInput.
http://trac.webkit.org/changeset/163346