Bug 107287

Summary: Web Inspector: modify generate_protocol_externs.py to generate JSON typedef's for @constructors
Product: WebKit Reporter: Andrey Adaikin <aandrey>
Component: Web Inspector (Deprecated)Assignee: Andrey Adaikin <aandrey>
Status: RESOLVED FIXED    
Severity: Normal CC: apavlov, buildbot, dglazkov, keishi, loislo, pfeldman, pmuellr, rniwa, vsevik, web-inspector-bugs, webkit.review.bot, yurys
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
Patch
none
Generated protocol externs diff
none
Patch pfeldman: review+, webkit.review.bot: commit-queue-

Description Andrey Adaikin 2013-01-18 08:43:54 PST
We'll need to modify generate_protocol_externs.py to also generate JSON typedef's for @constructors, like:

/** @constructor */
CanvasAgent.TraceLog = function()
{
/** @type {CanvasAgent.TraceLogId} */ this.id;
/** @type {Array.<CanvasAgent.Call>} */ this.calls;
/** @type {number|undefined} */ this.startOffset;
/** @type {boolean|undefined} */ this.alive;
}

/** @typedef {{id:CanvasAgent.TraceLogId, calls:Array.<CanvasAgent.Call>, ...}} */
CanvasAgent.TraceLogJSON;
Comment 1 Andrey Adaikin 2013-01-18 10:56:57 PST
Created attachment 183506 [details]
Patch
Comment 2 Andrey Adaikin 2013-01-18 11:24:31 PST
Created attachment 183515 [details]
Generated protocol externs diff
Comment 3 Build Bot 2013-01-18 11:30:33 PST
Comment on attachment 183506 [details]
Patch

Attachment 183506 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://queues.webkit.org/results/15948481

New failing tests:
inspector/console/command-line-api.html
Comment 4 WebKit Review Bot 2013-01-18 12:04:44 PST
Comment on attachment 183506 [details]
Patch

Attachment 183506 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/15957237

New failing tests:
inspector/console/command-line-api.html
Comment 5 Build Bot 2013-01-18 13:08:45 PST
Comment on attachment 183506 [details]
Patch

Attachment 183506 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/15949458

New failing tests:
inspector/console/command-line-api.html
Comment 6 Pavel Feldman 2013-01-20 23:20:11 PST
Comment on attachment 183506 [details]
Patch

What is CanvasAgent.TraceLogJSON? CanvasAgent.TraceLog is already generated.
Comment 7 Andrey Adaikin 2013-01-21 01:50:01 PST
(In reply to comment #6)
> (From update of attachment 183506 [details])
> What is CanvasAgent.TraceLogJSON? CanvasAgent.TraceLog is already generated.

hmmm... plz see my first comment :) also plz check out the generated protocol externs diff.

These are two different types. The CanvasAgent.TraceLog defines a type of CanvasAgent.TraceLog class with some methods defined in prototype and etc. The CanvasAgent.TraceLogJSON is of the Object type with a set of properties.
Comment 8 Pavel Feldman 2013-01-21 01:52:13 PST
(In reply to comment #7)
> (In reply to comment #6)
> > (From update of attachment 183506 [details] [details])
> > What is CanvasAgent.TraceLogJSON? CanvasAgent.TraceLog is already generated.
> 
> hmmm... plz see my first comment :) also plz check out the generated protocol externs diff.
> 
> These are two different types. The CanvasAgent.TraceLog defines a type of CanvasAgent.TraceLog class with some methods defined in prototype and etc. The CanvasAgent.TraceLogJSON is of the Object type with a set of properties.

But I think they are equivalent from the compiler standpoint. At least the ones you specify in comment #1 are equivalent.
Comment 9 Andrey Adaikin 2013-01-21 02:03:04 PST
(In reply to comment #8)
> (In reply to comment #7)
> > (In reply to comment #6)
> > > (From update of attachment 183506 [details] [details] [details])
> > > What is CanvasAgent.TraceLogJSON? CanvasAgent.TraceLog is already generated.
> > 
> > hmmm... plz see my first comment :) also plz check out the generated protocol externs diff.
> > 
> > These are two different types. The CanvasAgent.TraceLog defines a type of CanvasAgent.TraceLog class with some methods defined in prototype and etc. The CanvasAgent.TraceLogJSON is of the Object type with a set of properties.
> 
> But I think they are equivalent from the compiler standpoint. At least the ones you specify in comment #1 are equivalent.

Those are different. Example:

/** @type {CanvasAgent.TraceLog} */
var a;
/** @type {CanvasAgent.TraceLogJSON} */
var b;

a = {id:1, calls:[], ...}; // compiler warns it cannot be casted to CanvasAgent.TraceLog
b = {id:2, calls:[], ...}; // compiler will do the right thing

b = {calls:[], ...}; // compiler will warn that the 'id' property is missing, cause it's not optional

Even the following is not permitted by the recent version of compiler:
a = /** @type {CanvasAgent.TraceLog} */ ({id:1, calls:[], ...});

not to mention that we would loose all compiler type checking work.
Comment 10 Pavel Feldman 2013-01-21 02:06:54 PST
My point is that CanvasAgent.TraceLog is a type we define for protocol type TraceLog that is supposed to describe the JSON payload. And with that regard, TraceLog is no different from any other type we use. If you are saying compiler could check more if we switched to another notation, please switch entire protocol there. I.e. start generating @typedefs for all payloads and use existing names for the types.
Comment 11 Andrey Adaikin 2013-01-21 02:11:16 PST
(In reply to comment #10)
> My point is that CanvasAgent.TraceLog is a type we define for protocol type TraceLog that is supposed to describe the JSON payload. And with that regard, TraceLog is no different from any other type we use. If you are saying compiler could check more if we switched to another notation, please switch entire protocol there. I.e. start generating @typedefs for all payloads and use existing names for the types.

Oh, I see. Yes, we should use @typedef instead of @constructor, since those payloads are of type Object.

I'll send a new patch soon.
Comment 12 Andrey Adaikin 2013-01-21 04:50:01 PST
Created attachment 183759 [details]
Patch
Comment 13 WebKit Review Bot 2013-01-21 05:33:04 PST
Comment on attachment 183759 [details]
Patch

Attachment 183759 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/16038016

New failing tests:
inspector/console/command-line-api.html
Comment 14 Build Bot 2013-01-21 06:11:03 PST
Comment on attachment 183759 [details]
Patch

Attachment 183759 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://queues.webkit.org/results/16038034

New failing tests:
inspector/console/command-line-api.html
Comment 15 Build Bot 2013-01-21 06:16:35 PST
Comment on attachment 183759 [details]
Patch

Attachment 183759 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/16043028

New failing tests:
inspector/console/command-line-api.html
Comment 16 Andrey Adaikin 2013-01-21 06:17:45 PST
Committed r140329: <http://trac.webkit.org/changeset/140329>