[chromium] Pipe compositor begin/end frame up to WebWidgetClient
Created attachment 114541 [details] Patch
This corresponds to a Chromium change here: http://codereview.chromium.org/8528006
Please wait for approval from fishd@chromium.org before submitting because this patch contains changes to the Chromium public API.
There seem to be merge conflicts here
Created attachment 114546 [details] Rebaselined
Comment on attachment 114546 [details] Rebaselined Looks good, need Darin to approve the WebWidgetClient changes
Comment on attachment 114546 [details] Rebaselined View in context: https://bugs.webkit.org/attachment.cgi?id=114546&action=review > Source/WebKit/chromium/public/WebWidgetClient.h:63 > + virtual void didBeginFrame() { } the term "Frame" is a bit overloaded. see WebFrame. WebView extends from WebWidget. WebView has a tree of frames. See how this can get confusing? Maybe we can try to stick to using terms involving variations of the word "compositor"?
Comment on attachment 114546 [details] Rebaselined View in context: https://bugs.webkit.org/attachment.cgi?id=114546&action=review (In reply to comment #7) > the term "Frame" is a bit overloaded. see WebFrame. WebView extends from WebWidget. WebView has a tree of frames. See how this can get confusing? Maybe we can try to stick to using terms involving variations of the word "compositor"? Can we do CompositorFrame? We're gonna kill ourselves if we try to not use the word frame, since that's the word we use in graphics... long term, maybe what we should do is remove all this stuff from the WebWidget API and instead have WebWidget have a WebLayerTreeView* accessor that gives access to WebViewImpl::m_layerTreeHost. That'd avoid this better... > Source/WebCore/platform/graphics/chromium/cc/CCLayerTreeHost.h:61 > + virtual void didFinishFrame() = 0; Struggling with the renaming to didFinishFrame here (sorry to be 'that guy'!). We have a finishAllRendering and this definitely isn't related to that. Also it should be clear what was done --- its not that the GPU has finished the work [e.g. swap-ack driven], just that its been submitted. That's why it was so pedantically named as it was before. > Source/WebCore/platform/graphics/chromium/cc/CCSingleThreadProxy.cpp:307 > + m_layerTreeHost->didFinishFrame(); Ooh, does this mean that we can stop the WebGraphicsContext from calling didSwap/etc on the RenderView? That's causing me some headaches right now.. > Source/WebKit/chromium/src/WebViewImpl.cpp:2783 > + m_client->didFinishFrame(); Do we need a null-check? I forget...
Created attachment 114769 [details] Rename functions, pipe swapbuffers properly
Comment on attachment 114769 [details] Rename functions, pipe swapbuffers properly Awesome. Jamesr or darin for official ceremony.
Comment on attachment 114769 [details] Rename functions, pipe swapbuffers properly R=me on everything but WebWidgetClient.h, please wait for Darin on that.
Comment on attachment 114769 [details] Rename functions, pipe swapbuffers properly Attachment 114769 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/10460592
Created attachment 114840 [details] Now with compiling tests
Comment on attachment 114840 [details] Now with compiling tests View in context: https://bugs.webkit.org/attachment.cgi?id=114840&action=review > Source/WebKit/chromium/public/WebWidgetClient.h:63 > + virtual void didCommitAndDrawCompositorFrame() { } nit: please add some documentation explaining what these events are telling the embedder. > Source/WebKit/chromium/public/WebWidgetClient.h:64 > + virtual void didCompleteSwapbuffers() { } nit: should it be SwapBuffers... with a capital B?
Created attachment 114999 [details] Address nits
Comment on attachment 114999 [details] Address nits View in context: https://bugs.webkit.org/attachment.cgi?id=114999&action=review > Source/WebKit/chromium/public/WebWidgetClient.h:66 > + // Called for compositing mode when swapbuffers has been posted in the GPU nit: insert new line above the start of a comment block.
Comment on attachment 114999 [details] Address nits R=me
Committed r100199: <http://trac.webkit.org/changeset/100199>