Bug 191340 – Web Inspector: Fix "Javascript" => "JavaScript" enum in protocol generated objects

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector

[Joseph Pecoraro]

Fix "Javascript" => "JavaScript" enum in protocol generated objects

[Joseph Pecoraro]

Created attachment 354033 [details]
[PATCH] Proposed Fix

[EWS Watchlist]

This patch modifies the inspector protocol generator. Please ensure that you have rebaselined any generator test results (i.e., by running `Tools/Scripts/run-inspector-generator-tests --reset-results`)

[Matt Baker]

Comment on attachment 354033 [details]
[PATCH] Proposed Fix

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

Looks good, except for the issue mentioned.

> Source/WebInspectorUI/UserInterface/Protocol/Legacy/11.3/InspectorBackendCommands.js:70
> +InspectorBackend.registerEnum("Canvas.ContextType", {Canvas2D: "canvas-2d", BitmapRenderer: "bitmaprenderer", WebGL: "webgl", WebGL2: "webgl2", Webgpu: "webgpu"});

The frontend uses CanvasAgent.ContextType.WebGPU. This change causes calls to InspectorCanvasAgent::requestContent to fail with "unsupported canvas context type".

[Joseph Pecoraro]

Created attachment 354049 [details]
[PATCH] Proposed Fix

[Joseph Pecoraro]

Comment on attachment 354033 [details]
[PATCH] Proposed Fix

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

>> Source/WebInspectorUI/UserInterface/Protocol/Legacy/11.3/InspectorBackendCommands.js:70
>> +InspectorBackend.registerEnum("Canvas.ContextType", {Canvas2D: "canvas-2d", BitmapRenderer: "bitmaprenderer", WebGL: "webgl", WebGL2: "webgl2", Webgpu: "webgpu"});
> 
> The frontend uses CanvasAgent.ContextType.WebGPU. This change causes calls to InspectorCanvasAgent::requestContent to fail with "unsupported canvas context type".

Hmm, a couple things.

  • This seems to be a pre-existing mistake. Nothing in the frontend used `CanvasAgent.ContextType.Webgpu` only `CanvasAgent.ContextType.WebGPU` so this would be fixing iOS 11 support.
  • However, WI.Canvas.fromPayload has a mistake (missing `break`) that needs to be fixed!

Other than that, this is just changing the enum name, not the protocol string, so it's fine to make this change otherwise. I'll update.

[Joseph Pecoraro]

Updated patch.

[Devin Rousso]

Comment on attachment 354049 [details]
[PATCH] Proposed Fix

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

r=me

> Source/JavaScriptCore/inspector/scripts/codegen/generator.py:45
> +_ALWAYS_SPECIALCASED_ENUM_VALUE_SUBSTRINGS = set(['2D', 'API', 'CSS', 'DOM', 'HTML', 'JIT', 'XHR', 'XML', 'IOS', 'MacOS', 'JavaScript'])

While we're at it, shouldn't "IOS" be "iOS" and "MacOS" be "macOS"?

[Joseph Pecoraro]

(In reply to Devin Rousso from comment #7)
> Comment on attachment 354049 [details]
> [PATCH] Proposed Fix
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=354049&action=review
> 
> r=me
> 
> > Source/JavaScriptCore/inspector/scripts/codegen/generator.py:45
> > +_ALWAYS_SPECIALCASED_ENUM_VALUE_SUBSTRINGS = set(['2D', 'API', 'CSS', 'DOM', 'HTML', 'JIT', 'XHR', 'XML', 'IOS', 'MacOS', 'JavaScript'])
> 
> While we're at it, shouldn't "IOS" be "iOS" and "MacOS" be "macOS"?

These would be for enum values. I don't think we'd want those representations since we generally start them with a capital.

[WebKit Commit Bot]

Comment on attachment 354049 [details]
[PATCH] Proposed Fix

Clearing flags on attachment: 354049

Committed r237934: <https://trac.webkit.org/changeset/237934>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/45884080>