Bug 55530 - V8 code generator doesn't properly support a single SerializedScriptValue attribute
Summary: V8 code generator doesn't properly support a single SerializedScriptValue att...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other OS X 10.5
: P2 Normal
Assignee: Jeremy Orlow
URL:
Keywords:
Depends on:
Blocks: 55443
  Show dependency treegraph
 
Reported: 2011-03-01 16:54 PST by Jeremy Orlow
Modified: 2011-03-01 17:24 PST (History)
4 users (show)

See Also:


Attachments
Patch (86.06 KB, patch)
2011-03-01 16:55 PST, Jeremy Orlow
jamesr: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jeremy Orlow 2011-03-01 16:54:39 PST
V8 code generator doesn't properly support a single SerializedScriptValue attribute
Comment 1 Jeremy Orlow 2011-03-01 16:55:06 PST
Created attachment 84326 [details]
Patch
Comment 2 WebKit Review Bot 2011-03-01 16:59:03 PST
Attachment 84326 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1

Last 3072 characters of output:
y sorted.  [build/include_order] [4]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestSerializedScriptValueInterface.cpp:26:  Found other header before a header this file implements. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestSerializedScriptValueInterface.cpp:28:  Alphabetical sorting problem.  [build/include_order] [4]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestSerializedScriptValueInterface.cpp:31:  Found header this file implements after other header. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestSerializedScriptValueInterface.cpp:37:  Found header this file implements after other header. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestSerializedScriptValueInterface.cpp:63:  Local variables should never be PassRefPtr (see http://webkit.org/coding/RefPtr.html).  [readability/pass_ptr] [5]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestSerializedScriptValueInterface.cpp:124:  Non-label code inside switch statements should be indented.  [whitespace/indent] [4]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestSerializedScriptValueInterface.cpp:145:  Declaration has space between type name and * in GObjectClass *gobjectClass  [whitespace/declaration] [3]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestSerializedScriptValueInterface.cpp:177:  Extra space between return and WEBKIT_DOM_TEST_SERIALIZED_SCRIPT_VALUE_INTERFACE  [whitespace/declaration] [3]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestSerializedScriptValueInterfacePrivate.h:21:  #ifndef header guard has wrong style, please use: WTF_WebKitDOMTestSerializedScriptValueInterfacePrivate_h  [build/header_guard] [5]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestSerializedScriptValueInterfacePrivate.h:26:  Alphabetical sorting problem.  [build/include_order] [4]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestSerializedScriptValueInterfacePrivate.h:28:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Source/WebCore/bindings/scripts/test/ObjC/DOMTestSerializedScriptValueInterfaceInternal.h:32:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Source/WebCore/bindings/scripts/test/GObject/WebKitDOMTestSerializedScriptValueInterface.h:27:  Alphabetical sorting problem.  [build/include_order] [4]
Source/WebCore/bindings/scripts/test/V8/V8TestSerializedScriptValueInterface.h:29:  wtf includes should be <wtf/file.h> instead of "wtf/file.h".  [build/include] [4]
Source/WebCore/bindings/scripts/test/V8/V8TestSerializedScriptValueInterface.h:38:  The parameter name "value" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 17 in 19 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 James Robinson 2011-03-01 17:09:48 PST
Comment on attachment 84326 [details]
Patch

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

look great.  thanks for adding the test.

i didn't look carefully at the non-v8 changes, assuming that most of those are just churn from out-of-date golden files

> Source/WebCore/bindings/scripts/test/TestSerializedScriptValueInterface.idl:15
> + * EXPRESS OR IMPLIED WARRANTIEstrArg, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED

A few extra characters snuck in here in the middle of "WARRANTIES"

>> Source/WebCore/bindings/scripts/test/V8/V8TestSerializedScriptValueInterface.h:29
>> +#include "wtf/text/StringHash.h"
> 
> wtf includes should be <wtf/file.h> instead of "wtf/file.h".  [build/include] [4]

If you are feeling super ambitious you could probably fix this one easily as well :)
Comment 4 Jeremy Orlow 2011-03-01 17:22:46 PST
(In reply to comment #3)
> (From update of attachment 84326 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=84326&action=review
> 
> look great.  thanks for adding the test.
> 
> i didn't look carefully at the non-v8 changes, assuming that most of those are just churn from out-of-date golden files
> 
> > Source/WebCore/bindings/scripts/test/TestSerializedScriptValueInterface.idl:15
> > + * EXPRESS OR IMPLIED WARRANTIEstrArg, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
> 
> A few extra characters snuck in here in the middle of "WARRANTIES"
> 
> >> Source/WebCore/bindings/scripts/test/V8/V8TestSerializedScriptValueInterface.h:29
> >> +#include "wtf/text/StringHash.h"
> > 
> > wtf includes should be <wtf/file.h> instead of "wtf/file.h".  [build/include] [4]
> 
> If you are feeling super ambitious you could probably fix this one easily as well :)

Fixed.  It used a different code path than the rest of the wtf includes.
Comment 5 Jeremy Orlow 2011-03-01 17:24:06 PST
Committed r80072: <http://trac.webkit.org/changeset/80072>