[webkit-reviews] review granted: [Bug 131153] [CSS Blending] Add compositing reason for isolation in RenderLayerCompositor. : [Attachment 228532] adding inspector code & test

[bugzilla-daemon at webkit.org]

Joseph Pecoraro <joepeck at webkit.org> has granted Ion Rosca <rosca at adobe.com>'s
request for review:
Bug 131153: [CSS Blending] Add compositing reason for isolation in
RenderLayerCompositor.
https://bugs.webkit.org/show_bug.cgi?id=131153

Attachment 228532: adding inspector code & test
https://bugs.webkit.org/attachment.cgi?id=228532&action=review

------- Additional Comments from Joseph Pecoraro <joepeck at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=228532&action=review


r=me

> Source/WebInspectorUI/UserInterface/Views/LayerTreeSidebarPanel.js:436
> +	   if (compositingReasons.isolatesCompositedBlendingDescendants)
> +	       addReason(WebInspector.UIString("Element isolates composited
descendants having CSS blending applied"));

Very interesting! The patch looks good, As a user facing feature, I'd want to
get a little more information on what isolation is so that maybe we can improve
the UI String. Right now it sounds fine.

Is "-webkit-isolation" with children having blending the only way to get this
state? Is just -webkit-isolation enough?

Note some of the other reasons include the CSS property, so it might be useful
here to include "-webkit-isolation".

>
LayoutTests/inspector-protocol/layers/layers-blending-compositing-reasons.html:
114
> +	       if (messageObject.hasOwnProperty("error")) {

Nit: This can be shorted to:

    InspectorTest.checkForError(messageObject);

Which will print a fail message, complete the test, and short circuit by
throwing an exception.