Bug 70160 - [chromium] Route requestAnimationFrame through CCProxy in threaded mode
Summary: [chromium] Route requestAnimationFrame through CCProxy in threaded mode
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nat Duca
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-10-14 17:15 PDT by Nat Duca
Modified: 2011-10-19 19:22 PDT (History)
2 users (show)

See Also:


Attachments
Patch (11.58 KB, patch)
2011-10-14 17:17 PDT, Nat Duca
no flags Details | Formatted Diff | Diff
Patch for landing (11.63 KB, patch)
2011-10-19 15:18 PDT, Nat Duca
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Nat Duca 2011-10-14 17:15:19 PDT
[chromium] Route requestAnimationFrame through CCProxy in threaded mode
Comment 1 Nat Duca 2011-10-14 17:17:21 PDT
Created attachment 111112 [details]
Patch
Comment 2 James Robinson 2011-10-17 15:24:47 PDT
Comment on attachment 111112 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=111112&action=review

R=me - have some nits to address while landing.

> Source/WebCore/platform/graphics/chromium/cc/CCSingleThreadProxy.cpp:144
> +    ASSERT(0);

we have ASSERT_NOT_REACHED() for this

> Source/WebKit/chromium/src/ChromeClientImpl.cpp:542
> +#if USE(ACCELERATED_COMPOSITING)
> +    if (!m_webView->isAcceleratedCompositingActive())
> +#endif
> +        m_webView->client()->scheduleAnimation();
> +#if USE(ACCELERATED_COMPOSITING)
> +    else
> +        m_webView->scheduleAnimation();
> +#endif

i think things would be slightly cleaner if ChromeClientImpl::scheduleAnimation() simply delegated to m_webView->scheduleAnimation() and all of the #if USE(ACCELE...) etc crap was inside WebViewImpl.cpp. WebViewImpl::scheduleAnimation() call call out to its client if it needs to

> Source/WebKit/chromium/src/WebViewImpl.h:-384
> -    void doUpdateAndComposite();

guessing this function wasn't actually defined or referenced before?
Comment 3 Nat Duca 2011-10-17 18:10:19 PDT
All great points. Thanks...
(In reply to comment #2)
Comment 4 Nat Duca 2011-10-19 15:18:37 PDT
Created attachment 111681 [details]
Patch for landing
Comment 5 WebKit Review Bot 2011-10-19 19:22:41 PDT
Comment on attachment 111681 [details]
Patch for landing

Clearing flags on attachment: 111681

Committed r97922: <http://trac.webkit.org/changeset/97922>
Comment 6 WebKit Review Bot 2011-10-19 19:22:46 PDT
All reviewed patches have been landed.  Closing bug.