[webkit-reviews] review denied: [Bug 173087] Web Inspector: Instrument active pixel memory used by canvases : [Attachment 313488] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Sat Jun 24 00:56:15 PDT 2017
Joseph Pecoraro <joepeck at webkit.org> has denied Devin Rousso
<drousso at apple.com>'s request for review:
Bug 173087: Web Inspector: Instrument active pixel memory used by canvases
https://bugs.webkit.org/show_bug.cgi?id=173087
Attachment 313488: Patch
https://bugs.webkit.org/attachment.cgi?id=313488&action=review
--- Comment #15 from Joseph Pecoraro <joepeck at webkit.org> ---
Comment on attachment 313488
--> https://bugs.webkit.org/attachment.cgi?id=313488
Patch
View in context: https://bugs.webkit.org/attachment.cgi?id=313488&action=review
r- to see another patch after dropping the "max" thing. The rest looks good.
> Source/JavaScriptCore/inspector/protocol/Canvas.json:42
> + { "name": "memoryCost", "type": "number", "optional": true,
"description": "Memory usage of the canvas." }
This is great!
> Source/JavaScriptCore/inspector/protocol/Canvas.json:80
> + {
> + "name": "maxPixelMemory",
> + "description": "Gets the maximum pixel memory available for all
canvases.",
> + "returns": [
> + { "name": "maxPixelMemory", "type": "number", "description":
"Maximum pixel memory available for all canvases." }
> + ]
Lets drop this. I don't think this max is useful. If/when the page crosses this
max pixel memory a console message will be logged that lists what the maximum
size is.
> Source/JavaScriptCore/inspector/protocol/Canvas.json:99
> + { "name": "canvasId", "$ref": "CanvasId", "description":
"Identifier of canvas who's memory cost changed." },
Simplify the description.
> Source/WebCore/inspector/InspectorInstrumentation.h:1200
> + if (InstrumentingAgents* instrumentingAgents =
instrumentingAgentsForDocument(&canvasElement.document()))
This should `FAST_RETURN_IF_NO_FRONTENDS(void());`
More information about the webkit-reviews
mailing list