Bug 76676 - Web Inspector: CodeGeneratorInspector.py: add optional runtime validator
Summary: Web Inspector: CodeGeneratorInspector.py: add optional runtime validator
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-01-19 17:22 PST by Peter Rybin
Modified: 2012-01-25 03:55 PST (History)
12 users (show)

See Also:

Attachments
For buildbots (55.63 KB, patch)
2012-01-19 17:44 PST, Peter Rybin 		no flags Details | Formatted Diff | Diff
InspectorFrontend.h base version (159.69 KB, text/plain)
2012-01-20 08:27 PST, Peter Rybin 		no flags Details
InspectorFrontend.cpp base version (40.81 KB, text/plain)
2012-01-20 08:29 PST, Peter Rybin 		no flags Details
sample of new InspectorFrontend.h (158.06 KB, text/plain)
2012-01-20 08:31 PST, Peter Rybin 		no flags Details
sample of new InspectorFrontend.cpp (46.74 KB, text/plain)
2012-01-20 08:32 PST, Peter Rybin 		no flags Details
Diff (26.77 KB, application/octet-stream)
2012-01-20 08:34 PST, Peter Rybin 		no flags Details
Patch (55.29 KB, patch)
2012-01-23 12:55 PST, Peter Rybin 		no flags Details | Formatted Diff | Diff
Sample of new InspectorFrontend.h (157.92 KB, text/plain)
2012-01-23 12:58 PST, Peter Rybin 		no flags Details
Sample of new InspectorFrontend.cpp (46.27 KB, text/plain)
2012-01-23 12:59 PST, Peter Rybin 		no flags Details
Patch (55.35 KB, patch)
2012-01-23 13:30 PST, Peter Rybin 		no flags Details | Formatted Diff | Diff
Patch (55.48 KB, patch)
2012-01-23 13:39 PST, Peter Rybin 		no flags Details | Formatted Diff | Diff
Patch (55.47 KB, patch)
2012-01-24 10:36 PST, Peter Rybin 		no flags Details | Formatted Diff | Diff
Sample of new InspectorFrontend.h (157.94 KB, text/plain)
2012-01-24 10:43 PST, Peter Rybin 		no flags Details
Show Obsolete (8) 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 Peter Rybin 2012-01-19 17:22:47 PST 
In some cases JSON values cannot be constructed by C++ typesafe builder. Runtime validator should be generated for such values.
Comment 1 Peter Rybin 2012-01-19 17:44:25 PST 
Created attachment 123227 [details]
For buildbots
Comment 2 Peter Rybin 2012-01-20 08:27:36 PST 
Created attachment 123320 [details]
InspectorFrontend.h base version
Comment 3 Peter Rybin 2012-01-20 08:29:59 PST 
Created attachment 123321 [details]
InspectorFrontend.cpp base version
Comment 4 Peter Rybin 2012-01-20 08:31:33 PST 
Created attachment 123323 [details]
sample of new InspectorFrontend.h
Comment 5 Peter Rybin 2012-01-20 08:32:46 PST 
Created attachment 123324 [details]
sample of new InspectorFrontend.cpp 

New InspectorFrontend.cpp
Comment 6 Peter Rybin 2012-01-20 08:34:12 PST 
Created attachment 123325 [details]
Diff
Comment 7 Yury Semikhatsky 2012-01-23 07:14:31 PST 
Comment on attachment 123227 [details]
For buildbots

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

Wrong alignment in generated code:
    if (it != object->end()) {
    Subtype::assertCorrectValue(RefPtr<InspectorValue>(it->second));
        ++count;
    }

> Source/WebCore/inspector/CodeGeneratorInspector.py:180
> +VALIDATOR_IFDEF_NAME = "(!ASSERT_DISABLED)"

should be #if !ASSERT_DISABLED (no parenthesis)

> Source/WebCore/inspector/CodeGeneratorInspector.py:319
> +            return "assertIsString"

assertIsString -> assertString

> Source/WebCore/inspector/CodeGeneratorInspector.py:347
> +            return "assertIsInt"

assertInt

> Source/WebCore/inspector/CodeGeneratorInspector.py:746
> +                                    validator_writer.newline("void %s%s::assertCorrectValue(PassRefPtr<InspectorValue> value)\n" % (helper.full_name_prefix_for_impl, enum_name))

Parameter type should be raw pointer, not PassRefPtr here as you don't pass ownership here.
Comment 8 Peter Rybin 2012-01-23 12:53:30 PST 
> Wrong alignment in generated code:
>     if (it != object->end()) {
>     Subtype::assertCorrectValue(RefPtr<InspectorValue>(it->second));
>         ++count;
>     }
Done

> > Source/WebCore/inspector/CodeGeneratorInspector.py:180
> > +VALIDATOR_IFDEF_NAME = "(!ASSERT_DISABLED)"
> should be #if !ASSERT_DISABLED (no parenthesis)
Done

> > Source/WebCore/inspector/CodeGeneratorInspector.py:319
> > +            return "assertIsString"
> assertIsString -> assertString
> > Source/WebCore/inspector/CodeGeneratorInspector.py:347
> > +            return "assertIsInt"
> assertInt
Done

> > Source/WebCore/inspector/CodeGeneratorInspector.py:746
> > +                                    validator_writer.newline("void %s%s::assertCorrectValue(PassRefPtr<InspectorValue> value)\n" % (helper.full_name_prefix_for_impl, enum_name))
> Parameter type should be raw pointer, not PassRefPtr here as you don't pass ownership here.
Done
Comment 9 Peter Rybin 2012-01-23 12:55:56 PST 
Created attachment 123601 [details]
Patch
Comment 10 Peter Rybin 2012-01-23 12:58:42 PST 
Created attachment 123602 [details]
Sample of new InspectorFrontend.h
Comment 11 Peter Rybin 2012-01-23 12:59:39 PST 
Created attachment 123603 [details]
Sample of new InspectorFrontend.cpp
Comment 12 Peter Rybin 2012-01-23 13:30:37 PST 
Created attachment 123610 [details]
Patch
Comment 13 Peter Rybin 2012-01-23 13:39:51 PST 
Created attachment 123612 [details]
Patch
Comment 14 Yury Semikhatsky 2012-01-24 08:55:08 PST 
Comment on attachment 123612 [details]
Patch

Please use static_cast on raw pointers instead of reinterpret cast on RefPtr<>* in cases like this:

*(reinterpret_cast<RefPtr<CallFrame>*>(&object))
Comment 15 Peter Rybin 2012-01-24 10:36:26 PST 
Created attachment 123764 [details]
Patch
Comment 16 Peter Rybin 2012-01-24 10:43:11 PST 
Created attachment 123766 [details]
Sample of new InspectorFrontend.h
Comment 17 Yury Semikhatsky 2012-01-25 03:55:09 PST 
Comment on attachment 123764 [details]
Patch

Clearing flags on attachment: 123764

Committed r105863: <http://trac.webkit.org/changeset/105863>
Comment 18 Yury Semikhatsky 2012-01-25 03:55:24 PST 
All reviewed patches have been landed.  Closing bug.