Bug 107287 – Web Inspector: modify generate_protocol_externs.py to generate JSON typedef's for @constructors

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector (Deprecated)

[Andrey Adaikin]

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;

[Andrey Adaikin]

Created attachment 183506 [details]
Patch

[Andrey Adaikin]

Created attachment 183515 [details]
Generated protocol externs diff

[Build Bot]

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

[WebKit Review Bot]

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

[Build Bot]

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

[Pavel Feldman]

Comment on attachment 183506 [details]
Patch

What is CanvasAgent.TraceLogJSON? CanvasAgent.TraceLog is already generated.

[Andrey Adaikin]

(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.

[Pavel Feldman]

(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.

[Andrey Adaikin]

(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.

[Pavel Feldman]

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.

[Andrey Adaikin]

(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.

[Andrey Adaikin]

Created attachment 183759 [details]
Patch

[WebKit Review Bot]

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

[Build Bot]

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

[Build Bot]

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

[Andrey Adaikin]

Committed r140329: <http://trac.webkit.org/changeset/140329>