Bug 129445

Summary: Web Replay: add infrastructure for auto-memoizing nondeterministic DOM APIs
Product: WebKit Reporter: BJ Burg <bburg>
Component: DOMAssignee: BJ Burg <bburg>
Status: RESOLVED FIXED    
Severity: Normal CC: cdumez, cgarcia, commit-queue, ddkilzer, joepeck, kling, timothy
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
WIP patch
none
the complete patch
timothy: review+
bad attachment
none
IDL changes
none
sample generated code none

Description BJ Burg 2014-02-27 13:49:54 PST
This intercession mechanism allows a developer to put [Nondeterministic] on an attribute or method in WebIDL. The code generator will automatically capture or replay the value using AutoMemoized<T>, a subclass of NondeterministicInput for T.

(Maybe it should be called MemoizedValue<T>?)
Comment 1 BJ Burg 2014-03-04 19:29:12 PST
Created attachment 225844 [details]
WIP patch
Comment 2 BJ Burg 2014-03-06 20:04:05 PST
Created attachment 226076 [details]
the complete patch
Comment 3 WebKit Commit Bot 2014-03-06 20:06:15 PST
Attachment 226076 [details] did not pass style-queue:


ERROR: Source/WebCore/replay/MemoizedDOMResult.h:117:  This { should be at the end of the previous line  [whitespace/braces] [4]
ERROR: Source/WebCore/replay/ReplayInputTypes.cpp:42:  Wrong number of spaces before statement. (expected: 4)  [whitespace/indent] [4]
Total errors found: 2 in 13 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 4 BJ Burg 2014-03-06 20:06:48 PST
Created attachment 226077 [details]
bad attachment

I also attached the IDL changes I used to test the build and the various cases the code generator has to support for WEB_REPLAY and vanilla builds. The IDL changes won't be included in this patch, since they need separate testing.
Comment 5 BJ Burg 2014-03-06 20:08:50 PST
Created attachment 226078 [details]
IDL changes
Comment 6 BJ Burg 2014-03-06 20:15:07 PST
Created attachment 226079 [details]
sample generated code
Comment 7 Timothy Hatcher 2014-03-12 11:35:29 PDT
Comment on attachment 226076 [details]
the complete patch

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

> Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:2208
> +                push(@implContent, "     }\n");

I'd add another \n here to give breathing room between the blocks.

> Source/WebCore/replay/MemoizedDOMResult.cpp:76
> +    encodedValue.put<String>("attribute", input.attribute());

These strings passed to put and get should use ASCIILiteral().
Comment 8 Joseph Pecoraro 2014-03-12 11:46:16 PDT
Comment on attachment 226076 [details]
the complete patch

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

> Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:3548
> +            push(@implContent, $indent . "if (!cursor.isReplaying())\n");
> +            push(@implContent, $indent . "    $functionString;\n");

Shouldn't this also setDOMException if raises exception inside this if block?
Comment 9 BJ Burg 2014-03-12 17:42:24 PDT
Committed r165521: <http://trac.webkit.org/changeset/165521>