[webkit-reviews] review granted: [Bug 225180] Web Inspector: Graphics: add instrumentation for new `CanvasRenderingContext2DSettings` : [Attachment 435499] Patch

[bugzilla-daemon at webkit.org]

BJ Burg <bburg at apple.com> has granted Devin Rousso <drousso at apple.com>'s
request for review:
Bug 225180: Web Inspector: Graphics: add instrumentation for new
`CanvasRenderingContext2DSettings`
https://bugs.webkit.org/show_bug.cgi?id=225180

Attachment 435499: Patch

https://bugs.webkit.org/attachment.cgi?id=435499&action=review




--- Comment #7 from BJ Burg <bburg at apple.com> ---
Comment on attachment 435499
  --> https://bugs.webkit.org/attachment.cgi?id=435499
Patch

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

r=me

The Windows EWS failure looks like a clean build is needed, for some reason the
InspectorProtocolObjects.h file is not being regenerated. Please confirm before
landing.

https://ews-build.webkit.org/#/builders/10/builds/102675

>>> Source/WebCore/inspector/InspectorCanvas.cpp:860
>>> +		 contextAttributesPayload->setPowerPreference("default"_s);
>> 
>> Maybe power preference values should be an enum in the Protocol?
> 
> Hmm, we could?  We don't actually explicitly use these values manually
anywhere in the frontend.  It'd just be a sanity check on the backend really. 
The frontend pipes this data directly into a `canvasElement.createContext(type,
attributes)`.  Frankly we could replace `Canvas.ContextAttributes` with an
anonymous object and it'd work the same.

I'm with Devin here. I don't see much point in making an enum if we aren't
verifying the values that come through.