Skip to content

GitLab

T
tangled
Commit 5e03d1d2 authored by xians@chromium.org's avatar xians@chromium.org
Browse files
Options

Revert 251918 "Do not send a frame swap ack from the browser unt..." 

The bots tells this CL broke most of the webrtc browser tests on Mac.
http://chromegw/i/internal.chromium.webrtc/builders/Mac%20Tester


BUG=344912
TEST=webrtc bots.

> Do not send a frame swap ack from the browser until the frame is drawn
> 
> Code to do this was recently removed because it was no longer being
> used in r241225 (https://codereview.chromium.org/116103002). This is
> now necessary again to throttle renderers when using CoreAnimation.
> 
> Frames are acked only when they are drawn. Both the CoreAnimation and
> the non-CoreAnimation paths are changed to behave this way. By virtue
> of the fact that the non-CoreAnimation path draws immediately, this
> should have no functional effect on that path.
> 
> This differs from the mechanism deleted in the aforementioned patch in
> two ways.
> 
> First, it uses a scoped_ptr of a struct instead of a vector of pairs to
> store the information about the swap being returned. This should improve
> readability in that the ack has struct names instead of just first and
> second in the pair, and in that the scoped_ptr does not suggest support
> for multiple pending swaps (which does not exist).
> 
> Second, it does makes RWHVMac ack frames more aggressively when
> inside a draw call, rather than adding a call from RWHImpl to the view,
> to effect the ack (which is what the old way does).
> 
> BUG=340133
> 
> Review URL: https://codereview.chromium.org/165703002

TBR=ccameron@chromium.org

Review URL: https://codereview.chromium.org/172633002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@252031 0039d316-1c4b-4281-b951-d872f2087c98
parent eb6d597c
Hide whitespace changes
Inline Side-by-side
Showing with 49 additions and 115 deletions
content/browser/renderer_host/compositing_iosurface_layer_mac.mm
View file @ 5e03d1d2
......@@ -16,14 +16,6 @@
#include "ui/gfx/size_conversions.h"
#include "ui/gl/gpu_switching_manager.h"
@interface CompositingIOSurfaceLayer()
// Private method to wait for a frame of the right size if we're in an active
// resize. This may potentially dispatch select messages from the run loop.
- (void)waitForResizedFrameInContext:(CGLContextObj)glContext;
@end
@implementation CompositingIOSurfaceLayer
@synthesize context = context_;
......@@ -74,41 +66,6 @@
[self setNeedsDisplay];
}
- (void)waitForResizedFrameInContext:(CGLContextObj)glContext {
// Cache a copy of renderWidgetHostView_ because it may be reset if
// a software frame is received in GetBackingStore.
content::RenderWidgetHostViewMac* cached_view = renderWidgetHostView_;
if (!cached_view->render_widget_host_ ||
cached_view->render_widget_host_->is_hidden()) {
return;
}
// Note that GetBackingStore can potentially spawn a nested run loop, which
// may change the current GL context, or, because the GL contexts are
// shared, may change the currently-bound FBO. Ensure that, when the run
// loop returns, the original GL context remain current, and the original
// FBO remain bound.
// TODO(ccameron): This is far too fragile a mechanism to rely on. Find
// a way to avoid doing this.
GLuint previous_framebuffer = 0;
glGetIntegerv(GL_FRAMEBUFFER_BINDING,
reinterpret_cast<GLint*>(&previous_framebuffer));
{
// If a resize is in progress then GetBackingStore request a frame of the
// current window size and block until a frame of the right size comes in.
// This makes the window content not lag behind the resize (at the cost of
// blocking on the browser's main thread).
gfx::ScopedCGLSetCurrentContext scoped_set_current_context(NULL);
cached_view->about_to_validate_and_paint_ = true;
(void)cached_view->render_widget_host_->GetBackingStore(true);
cached_view->about_to_validate_and_paint_ = false;
}
CHECK_EQ(CGLGetCurrentContext(), glContext)
<< "original GL context failed to re-bind after nested run loop, "
<< "browser crash is imminent.";
glBindFramebuffer(GL_FRAMEBUFFER, previous_framebuffer);
}
// The remaining methods implement the CAOpenGLLayer interface.
- (CGLPixelFormatObj)copyCGLPixelFormatForDisplayMask:(uint32_t)mask {
......@@ -150,12 +107,36 @@
return;
}
// Acknowledge the frame before we potentially wait for a frame of the right
// size.
renderWidgetHostView_->SendPendingSwapAck();
// Cache a copy of renderWidgetHostView_ because it may be reset if
// a software frame is received in GetBackingStore.
content::RenderWidgetHostViewMac* cached_view = renderWidgetHostView_;
// Wait for a frame of the right size to come in, if needed.
[self waitForResizedFrameInContext:glContext];
// If a resize is in progress then GetBackingStore request a frame of the
// current window size and block until a frame of the right size comes in.
// This makes the window content not lag behind the resize (at the cost of
// blocking on the browser's main thread).
if (cached_view->render_widget_host_) {
// Note that GetBackingStore can potentially spawn a nested run loop, which
// may change the current GL context, or, because the GL contexts are
// shared, may change the currently-bound FBO. Ensure that, when the run
// loop returns, the original GL context remain current, and the original
// FBO remain bound.
// TODO(ccameron): This is far too fragile a mechanism to rely on. Find
// a way to avoid doing this.
GLuint previous_framebuffer = 0;
glGetIntegerv(GL_FRAMEBUFFER_BINDING,
reinterpret_cast<GLint*>(&previous_framebuffer));
{
gfx::ScopedCGLSetCurrentContext scoped_set_current_context(NULL);
cached_view->about_to_validate_and_paint_ = true;
(void)cached_view->render_widget_host_->GetBackingStore(true);
cached_view->about_to_validate_and_paint_ = false;
}
CHECK_EQ(CGLGetCurrentContext(), glContext)
<< "original GL context failed to re-bind after nested run loop, "
<< "browser crash is imminent.";
glBindFramebuffer(GL_FRAMEBUFFER, previous_framebuffer);
}
// If a transition to software mode has occurred, this layer should be
// removed from the heirarchy now, so don't draw anything.
......
content/browser/renderer_host/render_widget_host_view_mac.h
View file @ 5e03d1d2
......@@ -398,11 +398,8 @@ class RenderWidgetHostViewMac : public RenderWidgetHostViewBase,
// someone (other than superview) has retained |cocoa_view_|.
RenderWidgetHostImpl* render_widget_host_;
// This is true when we are currently painting. In the legacy renderer, this
// means we should handle extra paint requests by expanding the invalid rect
// rather than actually painting. In hardware compositing it means that we
// are inside a draw callback and should not wait for frames to draw before
// acknowledging them.
// This is true when we are currently painting and thus should handle extra
// paint requests by expanding the invalid rect rather than actually painting.
bool about_to_validate_and_paint_;
// This is true when we have already scheduled a call to
......@@ -494,24 +491,10 @@ class RenderWidgetHostViewMac : public RenderWidgetHostViewBase,
void SendPendingLatencyInfoToHost();
void TickPendingLatencyInfoDelay();
void SendPendingSwapAck();
private:
friend class RenderWidgetHostView;
friend class RenderWidgetHostViewMacTest;
struct PendingSwapAck {
PendingSwapAck(int32 route_id, int gpu_host_id, int32 renderer_id)
: route_id(route_id),
gpu_host_id(gpu_host_id),
renderer_id(renderer_id) {}
int32 route_id;
int gpu_host_id;
int32 renderer_id;
};
scoped_ptr<PendingSwapAck> pending_swap_ack_;
void AddPendingSwapAck(int32 route_id, int gpu_host_id, int32 renderer_id);
// The view will associate itself with the given widget. The native view must
// be hooked up immediately to the view hierarchy, or else when it is
// deleted it will delete this out from under the caller.
......
content/browser/renderer_host/render_widget_host_view_mac.mm
View file @ 5e03d1d2
......@@ -437,10 +437,6 @@ RenderWidgetHostViewMac::RenderWidgetHostViewMac(RenderWidgetHost* widget)
}
RenderWidgetHostViewMac::~RenderWidgetHostViewMac() {
// If a caller has set this, then when the caller tries to re-set it sometime
// in the future, we will crash.
DCHECK(!about_to_validate_and_paint_);
// This is being called from |cocoa_view_|'s destructor, so invalidate the
// pointer.
cocoa_view_ = nil;
......@@ -568,9 +564,6 @@ bool RenderWidgetHostViewMac::CreateCompositedIOSurfaceLayer() {
void RenderWidgetHostViewMac::DestroyCompositedIOSurfaceAndLayer(
DestroyContextBehavior destroy_context_behavior) {
// Any pending frames will not be displayed, so ack them now.
SendPendingSwapAck();
ScopedCAActionDisabler disabler;
compositing_iosurface_.reset();
......@@ -777,10 +770,6 @@ void RenderWidgetHostViewMac::WasHidden() {
if (render_widget_host_->is_hidden())
return;
// Any pending frames will not be displayed until this is shown again. Ack
// them now.
SendPendingSwapAck();
// If we have a renderer, then inform it that we are being hidden so it can
// reduce its resource utilization.
render_widget_host_->WasHidden();
......@@ -1354,7 +1343,6 @@ void RenderWidgetHostViewMac::CompositorSwapBuffers(
compositing_iosurface_->CopyToVideoFrame(
gfx::Rect(size), frame,
base::Bind(callback, present_time));
SendPendingSwapAck();
return;
}
}
......@@ -1474,7 +1462,6 @@ bool RenderWidgetHostViewMac::DrawIOSurfaceWithoutCoreAnimation() {
underlay_view_->compositing_iosurface_ &&
underlay_view_has_drawn_) {
[underlay_view_->cocoa_view() setNeedsDisplay:YES];
SendPendingSwapAck();
return true;
}
......@@ -1510,7 +1497,6 @@ bool RenderWidgetHostViewMac::DrawIOSurfaceWithoutCoreAnimation() {
}
SendPendingLatencyInfoToHost();
SendPendingSwapAck();
return true;
}
......@@ -1688,14 +1674,19 @@ void RenderWidgetHostViewMac::AcceleratedSurfaceBuffersSwapped(
"RenderWidgetHostViewMac::AcceleratedSurfaceBuffersSwapped");
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
AddPendingSwapAck(params.route_id,
gpu_host_id,
compositing_iosurface_ ?
compositing_iosurface_->GetRendererID() : 0);
CompositorSwapBuffers(params.surface_handle,
params.size,
params.scale_factor,
params.latency_info);
AcceleratedSurfaceMsg_BufferPresented_Params ack_params;
ack_params.sync_point = 0;
ack_params.renderer_id = compositing_iosurface_ ?
compositing_iosurface_->GetRendererID() : 0;
RenderWidgetHostImpl::AcknowledgeBufferPresent(params.route_id,
gpu_host_id,
ack_params);
render_widget_host_->AcknowledgeSwapBuffersToRenderer();
}
void RenderWidgetHostViewMac::AcceleratedSurfacePostSubBuffer(
......@@ -1705,14 +1696,19 @@ void RenderWidgetHostViewMac::AcceleratedSurfacePostSubBuffer(
"RenderWidgetHostViewMac::AcceleratedSurfacePostSubBuffer");
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
AddPendingSwapAck(params.route_id,
gpu_host_id,
compositing_iosurface_ ?
compositing_iosurface_->GetRendererID() : 0);
CompositorSwapBuffers(params.surface_handle,
params.surface_size,
params.surface_scale_factor,
params.latency_info);
AcceleratedSurfaceMsg_BufferPresented_Params ack_params;
ack_params.sync_point = 0;
ack_params.renderer_id = compositing_iosurface_ ?
compositing_iosurface_->GetRendererID() : 0;
RenderWidgetHostImpl::AcknowledgeBufferPresent(params.route_id,
gpu_host_id,
ack_params);
render_widget_host_->AcknowledgeSwapBuffersToRenderer();
}
void RenderWidgetHostViewMac::AcceleratedSurfaceSuspend() {
......@@ -2082,32 +2078,6 @@ void RenderWidgetHostViewMac::TickPendingLatencyInfoDelay() {
[compositing_iosurface_layer_ setNeedsDisplay];
}
void RenderWidgetHostViewMac::AddPendingSwapAck(
int32 route_id, int gpu_host_id, int32 renderer_id) {
DCHECK(!pending_swap_ack_);
pending_swap_ack_.reset(new PendingSwapAck(
route_id, gpu_host_id, renderer_id));
// If we're about to paint, ack this immediatley.
if (about_to_validate_and_paint_)
SendPendingSwapAck();
}
void RenderWidgetHostViewMac::SendPendingSwapAck() {
if (!pending_swap_ack_)
return;
AcceleratedSurfaceMsg_BufferPresented_Params ack_params;
ack_params.sync_point = 0;
ack_params.renderer_id = pending_swap_ack_->renderer_id;
RenderWidgetHostImpl::AcknowledgeBufferPresent(pending_swap_ack_->route_id,
pending_swap_ack_->gpu_host_id,
ack_params);
if (render_widget_host_)
render_widget_host_->AcknowledgeSwapBuffersToRenderer();
pending_swap_ack_.reset();
}
} // namespace content
// RenderWidgetHostViewCocoa ---------------------------------------------------
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment