WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
83060
[chromium] When setting animation started events, should check the root layer
https://bugs.webkit.org/show_bug.cgi?id=83060
Summary
[chromium] When setting animation started events, should check the root layer
vollick
Reported
2012-04-03 13:11:43 PDT
Should ensure that we have a valid root layer before setting animation events.
Attachments
Patch
(1.51 KB, patch)
2012-04-03 18:12 PDT
,
vollick
no flags
Details
Formatted Diff
Diff
Patch
(2.53 KB, patch)
2012-04-04 11:10 PDT
,
vollick
no flags
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
vollick
Comment 1
2012-04-03 18:12:21 PDT
Created
attachment 135475
[details]
Patch
Dana Jansens
Comment 2
2012-04-03 19:00:13 PDT
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?
vollick
Comment 3
2012-04-04 11:10:43 PDT
Created
attachment 135634
[details]
Patch (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).
Adrienne Walker
Comment 4
2012-04-04 11:24:26 PDT
Comment on
attachment 135634
[details]
Patch Looks good to me.
Dana Jansens
Comment 5
2012-04-04 11:32:30 PDT
(In reply to
comment #3
)
> Created an attachment (id=135634) [details] > Patch > > (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).
Makes sense, yeh :)
WebKit Review Bot
Comment 6
2012-04-04 13:31:40 PDT
Comment on
attachment 135634
[details]
Patch Clearing flags on attachment: 135634 Committed
r113232
: <
http://trac.webkit.org/changeset/113232
>
WebKit Review Bot
Comment 7
2012-04-04 13:31:44 PDT
All reviewed patches have been landed. Closing bug.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug