Bug 76676

Summary: Web Inspector: CodeGeneratorInspector.py: add optional runtime validator
Product: WebKit Reporter: Peter Rybin <prybin>
Component: Web Inspector (Deprecated)Assignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: apavlov, bweinstein, caseq, joepeck, keishi, loislo, pfeldman, pmuellr, prybin, rik, timothy, yurys
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
For buildbots
none
InspectorFrontend.h base version
none
InspectorFrontend.cpp base version
none
sample of new InspectorFrontend.h
none
sample of new InspectorFrontend.cpp
none
Diff
none
Patch
none
Sample of new InspectorFrontend.h
none
Sample of new InspectorFrontend.cpp
none
Patch
none
Patch
none
Patch
none
Sample of new InspectorFrontend.h none

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.