[webkit-reviews] review requested: [Bug 83060] [chromium] When setting animation started events, should check the root layer : [Attachment 135634] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Apr 4 11:10:43 PDT 2012


vollick at chromium.org has asked	for review:
Bug 83060: [chromium] When setting animation started events, should check the
root layer
https://bugs.webkit.org/show_bug.cgi?id=83060

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

------- Additional Comments from vollick at chromium.org
(In reply to comment #2)
> I feel like we're starting to have if (rootLayer) checks all over the
LayerTreeHosts. Is there one central place in the proxy or somewhere that we
can do this check?

There aren't _that_ many checks ;) With this change, I think the count would be
up to four (two of which are in setRootLayer). I've moved the two extra checks
into the recursive functions, which is perhaps a better place for them.

I really think that the layer tree hosts are the right folks to be doing these
nullity checks, though. I think that bumping the checks up to the proxies is
the wrong move. As I understand things, the proxies operate on layer tree hosts
and the layer tree hosts operate on layers. IMO, if the proxy has logic to
detect when the root layer is null and avoid calling certain functions in that
case, then the separation of concerns of the proxies and hosts has been
muddied. I think a proxy should be able to happily tell the hosts to do things
and it should be up to the hosts to determine whether or not it can actually do
them (possibly returning error codes, if appropriate).


More information about the webkit-reviews mailing list