[webkit-reviews] review granted: [Bug 110505] Expose a list of all reasons that qualify a RenderLayer to be composited : [Attachment 189582] Patch

[bugzilla-daemon at webkit.org]

Simon Fraser (smfr) <simon.fraser at apple.com> has granted Antoine Quint
<graouts at apple.com>'s request for review:
Bug 110505: Expose a list of all reasons that qualify a RenderLayer to be
composited
https://bugs.webkit.org/show_bug.cgi?id=110505

Attachment 189582: Patch
https://bugs.webkit.org/attachment.cgi?id=189582&action=review

------- Additional Comments from Simon Fraser (smfr) <simon.fraser at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=189582&action=review


>>>> Source/WebCore/rendering/RenderLayerCompositor.cpp:-1782
>>>> -
>>> 
>>> So what happens if you have this reason and another one. You'll never
report "root" because you exit early above.
>>> 
>>> This makes me wonder why you're using a bitmask over just a regular enum?
>> 
>> The bit mask will be used for the Web Inspector to be able to several
reasons a RenderLayer was composited. The logReasonsForCompsiting() method
replaces the old reasonsForCompositing() and its behavior which only reports
the first matching reason.
> 
> So I still think you should fix this method to return multiple reasons. If
reason has both CompositingReasonVideo and CompositingReason3DTransform then
the string should have "3d transform + video" or something.

It's fine for this logging to not change.

> Source/WebCore/rendering/RenderLayerCompositor.h:85
> +typedef unsigned CompositingReason;

CompositingReasons (plural), since this is a bitmask.

> Source/WebCore/rendering/RenderLayerCompositor.h:269
> +    CompositingReason reasonForCompositing(const RenderLayer*);

This should be const.