[webkit-reviews] review denied: [Bug 51952] Implement mozilla's animationTime property : [Attachment 78081] webkit2 support
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Wed Jan 5 17:54:44 PST 2011
Simon Fraser (smfr) <simon.fraser at apple.com> has denied James Robinson
<jamesr at chromium.org>'s request for review:
Bug 51952: Implement mozilla's animationTime property
https://bugs.webkit.org/show_bug.cgi?id=51952
Attachment 78081: webkit2 support
https://bugs.webkit.org/attachment.cgi?id=78081&action=review
------- Additional Comments from Simon Fraser (smfr) <simon.fraser at apple.com>
(In reply to comment #6)
> (In reply to comment #5)
> > (From update of attachment 78058 [details] [details])
> > View in context:
https://bugs.webkit.org/attachment.cgi?id=78058&action=review
> >
> > r=me, but think about the WebHTMLView changes.
> >
> > > WebKit/mac/WebView/WebHTMLView.mm:3512
> > > + Frame* frame = [webView _mainCoreFrame];
> > > + if (frame && frame->page())
> > > + frame->page()->animationTime()->clearCurrentAnimationTime();
> > > +
> >
> > Is it correct to call clearCurrentAnimationTime() for each subframe?
>
> I'm not quite sure what you mean. The AnimationTimeController is per-page,
so the intent here is to call clearCurrentAnimationTime() on the page
associated with this WebHTMLView. If there are multiple Frames, is there a
WebHTMLView associated with each FrameView (and thus a drawRect call for each?)
In WebKit1, this is true. This is probably OK since scripts and animations
won't run between painting different frames, but you may get a paint
notification for a subframe when the main frame has nothing to paint.
> > You may also want to think about whether it's appropriate to call this for
printing, and when views are being snapshotted.
> >
> > You should make this work in WebKit2 as well.
>
> Sure thing. I believe it should be sufficient to call
clearCurrentAnimationTime() after the paint in
ChunkedUpdateDrawingArea::paintIntoUpdateChunk() for non-composited pages
I think ChunkedUpdateDrawingArea::display() would be better. Later
optimizations may result in paintIntoUpdateChunk() being called more than once.
> and after syncCompositingLayers() in
LayerBackedDrawingArea::updateLayoutRunLoopObserverFired() for composited
pages.
I think syncCompositingLayers() is a better place.
More information about the webkit-reviews
mailing list