[webkit-reviews] review granted: [Bug 87301] [chromium] WebLayerTreeViewImpl should not hide methods in CCLayerTreeHost with signatures that match the Client interface : [Attachment 143659] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Thu May 24 13:10:35 PDT 2012
James Robinson <jamesr at chromium.org> has granted Dana Jansens
<danakj at chromium.org>'s request for review:
Bug 87301: [chromium] WebLayerTreeViewImpl should not hide methods in
CCLayerTreeHost with signatures that match the Client interface
https://bugs.webkit.org/show_bug.cgi?id=87301
Attachment 143659: Patch
https://bugs.webkit.org/attachment.cgi?id=143659&action=review
------- Additional Comments from James Robinson <jamesr at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=143659&action=review
R=me
We chatted offline about this. The problem today the call sequence looks like:
WebViewImpl -> WebLayerTreeView::updateAnimations() ->
WebLayerTreeViewImpl::updateAnimations() ->
WebLayerTreeViewClient::updateAnimations() -> WebViewImpl::updateAnimations()
essentially it's short-circuiting in WLTVI. This is a problem because
CCLayerTreeHost::updateAnimations isn't called, so CCLTH::m_animating is not
set and CCLTH::animateLayers() is not called. CCLTH::animateLayers() updates
the main-thread animated state of layers, which isn't what is drawn (that's the
impl-thread animated state of layers) but is what determines painting /
uploading.
With this patch the call sequence becomes:
WebViewImpl -> WebLayerTreeView::updateAnimations() ->
WebLayerTreeViewImpl::updateAnimations() -> CCLayerTreeHost::updateAnimations()
-> CCLayerTreeHostClient::updateAnimations() ->
WebLayerTreeViewImpl::ClientConverter::updateAnimations() ->
WebLayerTreeViewClient::updateAnimations() -> WebViewImpl::updateAnimations()
for the win!
>>> Source/WebKit/chromium/src/WebLayerTreeViewImpl.h:46
>>> + class ClientConverter : public WebCore::CCLayerTreeHostClient {
>>
>> does this have to be in the header (could it be in the cpp)?
>
> I think the WebLayerTreeView needs to hold an instance of the class, so that
it can hold the WebLayerTreeViewClient*. So I think it needs to be in the
header?
WebLayerTreeViewImpl could just have an OwnPtr<ClientConverter> to not need to
see the definition, but it's a minor thing.
Our typical naming for this sort of type is "...Adapter"
More information about the webkit-reviews
mailing list