[webkit-reviews] review requested: [Bug 108864] [wk2] REGRESSION (r138858): GIFs don't start playing when they come out of background tabs : [Attachment 197649] patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Thu Apr 11 12:40:08 PDT 2013
Antti Koivisto <koivisto at iki.fi> has asked for review:
Bug 108864: [wk2] REGRESSION (r138858): GIFs don't start playing when they come
out of background tabs
https://bugs.webkit.org/show_bug.cgi?id=108864
Attachment 197649: patch
https://bugs.webkit.org/attachment.cgi?id=197649&action=review
------- Additional Comments from Antti Koivisto <koivisto at iki.fi>
View in context: https://bugs.webkit.org/attachment.cgi?id=197649&action=review
> Source/WebCore/dom/Document.cpp:6167
> +void Document::advanceAllAnimatedImages()
> +{
Something like resumeAnimatingImages() would be more clear about the purpose of
this function.
Perhaps this could go to AnimationController? It seems we already use
resumeAnimations() for other animation types. I feel this doesn't belong to the
Document as it generally does not deal with rendering. The amount of includes
you need to add points to this as well.
> Source/WebCore/dom/Document.cpp:6188
> + CachedResourceHandle<CachedResource> resource = it->value;
> +
> + if (!resource || !resource->isImage())
> + continue;
> +
> + CachedImage* cachedImage =
static_cast<CachedImage*>(resource.get());
> + if (!cachedImage->hasImage())
> + continue;
> +
> + Image* image = cachedImage->image();
> + if (!image->isBitmapImage())
> + continue;
> +
> + BitmapImage* bitmapImage = static_cast<BitmapImage*>(image);
> + if (!bitmapImage->canAnimate())
> + continue;
> +
> + cachedImage->animationAdvanced(bitmapImage);
I would remove the empty lines.
> Source/WebKit2/WebProcess/WebPage/WebPage.cpp:2010
> m_page->didMoveOnscreen();
>
> - if (!pageWasInWindow)
> + if (!pageWasInWindow) {
> WebProcess::shared().pageDidEnterWindow(this);
> +
> + for (Frame* frame = m_page->mainFrame(); frame; frame =
frame->tree()->traverseNext()) {
> + if (Document* document = frame->document())
> + document->advanceAllAnimatedImages();
> + }
> + }
This code should probably done in WebCore side under Page::didMoveOnscreen().
Or maybe it should use the MediaCanStartListener mechanism. I don't think it
belongs to WebKit2 in any case.
More information about the webkit-reviews
mailing list