Bug 129445 - Web Replay: add infrastructure for auto-memoizing nondeterministic DOM APIs
Summary: Web Replay: add infrastructure for auto-memoizing nondeterministic DOM APIs
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: BJ Burg
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-02-27 13:49 PST by BJ Burg
Modified: 2014-03-12 17:42 PDT (History)
7 users (show)

See Also:

Attachments
WIP patch (24.74 KB, patch)
2014-03-04 19:29 PST, BJ Burg 		no flags Details | Formatted Diff | Diff
the complete patch (37.70 KB, patch)
2014-03-06 20:04 PST, BJ Burg 		timothy: review+
Details | Formatted Diff | Diff
bad attachment (41.46 KB, patch)
2014-03-06 20:06 PST, BJ Burg 		no flags Details | Formatted Diff | Diff
IDL changes (2.13 KB, patch)
2014-03-06 20:08 PST, BJ Burg 		no flags Details | Formatted Diff | Diff
sample generated code (6.96 KB, text/plain)
2014-03-06 20:15 PST, BJ Burg 		no flags Details
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
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>