Bug 110461 – Web Inspector: [Protocol] Generate JS enum definitions.

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

[Eugene Klyuchnikov]

In protocol enums are defined in 3 ways:

1) As a type, like Page.ResourceType, currently 3 occurrences;
2) As type member, like Runtime.RemoteObject.type, 18 occurrences;
3) Directly in command parameters, like Page.getScriptExecutionStatus.type, 5 occurrences.

Generating corresponding type annotations would help to compiler to point errors like this:

this.responseReceived(requestId, frameId, loaderId, time, "Other" /* actually in should be "other" */, redirectResponse);

[Eugene Klyuchnikov]

Created attachment 189527 [details]
Patch

[Andrey Adaikin]

This is exiting!
Could you plz also attach diffs of the generated files as a patch?

[Eugene Klyuchnikov]

(In reply to comment #2)
> This is exiting!
> Could you plz also attach diffs of the generated files as a patch?

Diff for protocol-externs.js: http://diffchecker.com/1l1aEUqu
Diff for InspectorBackendCommands.js: http://diffchecker.com/67s9Jkv5

[Pavel Feldman]

Comment on attachment 189527 [details]
Patch

You should produce CamelCase enum literals per WebKit code guidelines.

[Eugene Klyuchnikov]

Created attachment 189710 [details]
Patch

[Pavel Feldman]

Could you attach the diff in the generated files as a patch to this bug?

[Eugene Klyuchnikov]

Updated literal names. New diffs:

  protocol-externs.js: http://diffchecker.com/p0799b5q

  InspectorBackendCommands.js: http://www.diffchecker.com/69982Uz9

[Pavel Feldman]

Could you attach the diff in the generated files as a patch to this bug?

[Eugene Klyuchnikov]

Created attachment 189713 [details]
Diff of generated files.

[Timothy Hatcher]

Comment on attachment 189713 [details]
Diff of generated files.

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

> new/InspectorBackendCommands.js:91
> +InspectorBackend.registerEnum("Console.ConsoleMessageSource", {Html: "html", Wml: "wml", Xml: "xml", Javascript: "javascript", Network: "network", ConsoleApi: "console-api", Other: "other"});

You should special case acronyms to make them all CAPS. Html => HTML, ConsoleApi => ConsoleAPI, etc.

[Pavel Feldman]

Comment on attachment 189710 [details]
Patch

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

> Source/WebCore/inspector/front-end/InspectorBackend.js:85
> +        var domainAndMethod = type.split(".");

You need to call this from loadFromJSONIfNeeded as well.

[Andrey Adaikin]

Comment on attachment 189710 [details]
Patch

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

> Source/WebCore/inspector/front-end/InspectorBackend.js:90
> +        this._initialized = true;

Is this correct? seems suspicious to have this line both in registerCommand and registerEnum

[Eugene Klyuchnikov]

Created attachment 190278 [details]
Patch

[Eugene Klyuchnikov]

Comment on attachment 189710 [details]
Patch

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

>> Source/WebCore/inspector/front-end/InspectorBackend.js:85
>> +        var domainAndMethod = type.split(".");
> 
> You need to call this from loadFromJSONIfNeeded as well.

Done.

>> Source/WebCore/inspector/front-end/InspectorBackend.js:90
>> +        this._initialized = true;
> 
> Is this correct? seems suspicious to have this line both in registerCommand and registerEnum

As I see, this marker is used do avoid recursion/repetitive loading. So, all 3 methods should mark instance as initialized.

[Eugene Klyuchnikov]

Comment on attachment 189713 [details]
Diff of generated files.

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

>> new/InspectorBackendCommands.js:91
>> +InspectorBackend.registerEnum("Console.ConsoleMessageSource", {Html: "html", Wml: "wml", Xml: "xml", Javascript: "javascript", Network: "network", ConsoleApi: "console-api", Other: "other"});
> 
> You should special case acronyms to make them all CAPS. Html => HTML, ConsoleApi => ConsoleAPI, etc.

Fixed

[Eugene Klyuchnikov]

Created attachment 190280 [details]
Difference in generated files.

[WebKit Review Bot]

Comment on attachment 190278 [details]
Patch

Clearing flags on attachment: 190278

Committed r144265: <http://trac.webkit.org/changeset/144265>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.