Skip to content

Commit

Permalink
Revert of Enable BeginFrame scheduling on aura (patchset chromium#15
Browse files Browse the repository at this point in the history
…id:270001 of https://codereview.chromium.org/1016033006/)

Reason for revert:
Making Autofill interactive tests incredibly flaky again:
http://build.chromium.org/p/chromium.chromiumos/builders/Linux%20ChromiumOS%20Tests%20(1)
http://build.chromium.org/p/chromium.chromiumos/builders/Linux%20ChromiumOS%20Ozone%20Tests%20%281%29/

Original issue's description:
> Enable BeginFrame scheduling on aura
>
> This cl is last partial cl from https://codereview.chromium.org/775143003/
> for easy review.
> Until now, BeginFrame is initiated by each renderer process.
> With this cl, BeginFrame is scheduled from parent(browser) cc scheduler
> to child(renderer) cc scheduler.
> For more detailed information, please see this (http://goo.gl/D1Qxrr)
>
> Also, --enable-begin-frame-scheduling is no longer needed because all platform except Mac
> uses it as a default.
>
> R=brianderson@chromium.org, danakj@chromium.org, mithro@mithis.com
> BUG=372086
>
> Committed: https://crrev.com/17dd2f109f3155ebf183627b63df25f97f34b67f
> Cr-Commit-Position: refs/heads/master@{#322622}
>
> Committed: https://crrev.com/62a759e582743bca13f27d1ce5db3dc2fe66fbb6
> Cr-Commit-Position: refs/heads/master@{#328170}

TBR=brianderson@chromium.org,danakj@chromium.org,mithro@mithis.com,sievers@chromium.org,oshima@chromium.org,boliu@chromium.org,simonhong@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=372086

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

Cr-Commit-Position: refs/heads/master@{#328267}
  • Loading branch information
danbeam authored and Commit bot committed May 5, 2015
1 parent 6f3e50b commit e627c52
Show file tree
Hide file tree
Showing 35 changed files with 269 additions and 83 deletions.
1 change: 1 addition & 0 deletions android_webview/lib/main/aw_main_delegate.cc
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ bool AwMainDelegate::BasicStartupComplete(int* exit_code) {
BrowserViewRenderer::CalculateTileMemoryPolicy();

base::CommandLine* cl = base::CommandLine::ForCurrentProcess();
cl->AppendSwitch(switches::kEnableBeginFrameScheduling);

// WebView uses the Android system's scrollbars and overscroll glow.
cl->AppendSwitch(switches::kDisableOverscrollEdgeEffect);
Expand Down
7 changes: 4 additions & 3 deletions ash/display/display_controller.cc
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
#include "ui/aura/window_tracker.h"
#include "ui/aura/window_tree_host.h"
#include "ui/compositor/compositor.h"
#include "ui/compositor/compositor_vsync_manager.h"
#include "ui/gfx/display.h"
#include "ui/gfx/screen.h"
#include "ui/wm/core/coordinate_conversion.h"
Expand Down Expand Up @@ -150,9 +151,9 @@ void SetDisplayPropertiesOnHost(AshWindowTreeHost* ash_host,
DisplayMode mode =
GetDisplayManager()->GetActiveModeForDisplayId(display.id());
if (mode.refresh_rate > 0.0f) {
host->compositor()->SetAuthoritativeVSyncInterval(
base::TimeDelta::FromMicroseconds(base::Time::kMicrosecondsPerSecond /
mode.refresh_rate));
host->compositor()->vsync_manager()->SetAuthoritativeVSyncInterval(
base::TimeDelta::FromMicroseconds(
base::Time::kMicrosecondsPerSecond / mode.refresh_rate));
}

// Just movnig the display requires the full redraw.
Expand Down
1 change: 1 addition & 0 deletions chrome/browser/chromeos/login/chrome_restart_request.cc
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ std::string DeriveCommandLine(const GURL& start_url,
::switches::kDisableThreadedScrolling,
::switches::kDisableTouchDragDrop,
::switches::kDisableTouchEditing,
::switches::kEnableBeginFrameScheduling,
::switches::kEnableBlinkFeatures,
::switches::kEnableCompositorAnimationTimelines,
::switches::kEnableDelegatedRenderer,
Expand Down
2 changes: 2 additions & 0 deletions content/browser/android/content_startup_flags.cc
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,8 @@ void SetContentCommandLineFlags(bool single_process,
parsed_command_line->AppendSwitch(switches::kSingleProcess);
}

parsed_command_line->AppendSwitch(switches::kEnableBeginFrameScheduling);

parsed_command_line->AppendSwitch(switches::kEnablePinch);
parsed_command_line->AppendSwitch(switches::kEnableOverlayFullscreenVideo);
parsed_command_line->AppendSwitch(switches::kEnableOverlayScrollbar);
Expand Down
27 changes: 25 additions & 2 deletions content/browser/compositor/browser_compositor_output_surface.cc
Original file line number Diff line number Diff line change
Expand Up @@ -15,17 +15,21 @@ namespace content {

BrowserCompositorOutputSurface::BrowserCompositorOutputSurface(
const scoped_refptr<cc::ContextProvider>& context_provider,
const scoped_refptr<ui::CompositorVSyncManager>& vsync_manager,
scoped_ptr<BrowserCompositorOverlayCandidateValidator>
overlay_candidate_validator)
: OutputSurface(context_provider),
vsync_manager_(vsync_manager),
reflector_(nullptr) {
overlay_candidate_validator_ = overlay_candidate_validator.Pass();
Initialize();
}

BrowserCompositorOutputSurface::BrowserCompositorOutputSurface(
scoped_ptr<cc::SoftwareOutputDevice> software_device)
scoped_ptr<cc::SoftwareOutputDevice> software_device,
const scoped_refptr<ui::CompositorVSyncManager>& vsync_manager)
: OutputSurface(software_device.Pass()),
vsync_manager_(vsync_manager),
reflector_(nullptr) {
Initialize();
}
Expand All @@ -34,20 +38,39 @@ BrowserCompositorOutputSurface::~BrowserCompositorOutputSurface() {
if (reflector_)
reflector_->DetachFromOutputSurface();
DCHECK(!reflector_);
if (!HasClient())
return;
vsync_manager_->RemoveObserver(this);
}

void BrowserCompositorOutputSurface::Initialize() {
capabilities_.max_frames_pending = 1;
capabilities_.adjust_deadline_for_parent = false;
}

void BrowserCompositorOutputSurface::OnUpdateVSyncParametersFromGpu(
bool BrowserCompositorOutputSurface::BindToClient(
cc::OutputSurfaceClient* client) {
if (!OutputSurface::BindToClient(client))
return false;
// Don't want vsync notifications until there is a client.
vsync_manager_->AddObserver(this);
return true;
}

void BrowserCompositorOutputSurface::OnUpdateVSyncParameters(
base::TimeTicks timebase,
base::TimeDelta interval) {
DCHECK(HasClient());
CommitVSyncParameters(timebase, interval);
}

void BrowserCompositorOutputSurface::OnUpdateVSyncParametersFromGpu(
base::TimeTicks timebase,
base::TimeDelta interval) {
DCHECK(HasClient());
vsync_manager_->UpdateVSyncParameters(timebase, interval);
}

void BrowserCompositorOutputSurface::SetReflector(ReflectorImpl* reflector) {
// Software mirroring is done by doing a GL copy out of the framebuffer - if
// we have overlays then that data will be missing.
Expand Down
17 changes: 14 additions & 3 deletions content/browser/compositor/browser_compositor_output_surface.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#include "base/threading/non_thread_safe.h"
#include "cc/output/output_surface.h"
#include "content/common/content_export.h"
#include "ui/compositor/compositor_vsync_manager.h"

namespace cc {
class SoftwareOutputDevice;
Expand All @@ -20,12 +21,19 @@ class ReflectorImpl;
class WebGraphicsContext3DCommandBufferImpl;

class CONTENT_EXPORT BrowserCompositorOutputSurface
: public cc::OutputSurface {
: public cc::OutputSurface,
public ui::CompositorVSyncManager::Observer {
public:
~BrowserCompositorOutputSurface() override;

// cc::OutputSurface implementation.
bool BindToClient(cc::OutputSurfaceClient* client) override;
cc::OverlayCandidateValidator* GetOverlayCandidateValidator() const override;

// ui::CompositorOutputSurface::Observer implementation.
void OnUpdateVSyncParameters(base::TimeTicks timebase,
base::TimeDelta interval) override;

void OnUpdateVSyncParametersFromGpu(base::TimeTicks tiembase,
base::TimeDelta interval);

Expand All @@ -41,13 +49,16 @@ class CONTENT_EXPORT BrowserCompositorOutputSurface
// Constructor used by the accelerated implementation.
BrowserCompositorOutputSurface(
const scoped_refptr<cc::ContextProvider>& context,
const scoped_refptr<ui::CompositorVSyncManager>& vsync_manager,
scoped_ptr<BrowserCompositorOverlayCandidateValidator>
overlay_candidate_validator);

// Constructor used by the software implementation.
explicit BrowserCompositorOutputSurface(
scoped_ptr<cc::SoftwareOutputDevice> software_device);
BrowserCompositorOutputSurface(
scoped_ptr<cc::SoftwareOutputDevice> software_device,
const scoped_refptr<ui::CompositorVSyncManager>& vsync_manager);

scoped_refptr<ui::CompositorVSyncManager> vsync_manager_;
ReflectorImpl* reflector_;

private:
Expand Down
16 changes: 14 additions & 2 deletions content/browser/compositor/delegated_frame_host.cc
Original file line number Diff line number Diff line change
Expand Up @@ -888,10 +888,13 @@ void DelegatedFrameHost::OnCompositingShuttingDown(ui::Compositor* compositor) {
DCHECK(!compositor_);
}

void DelegatedFrameHost::SetVSyncParameters(base::TimeTicks timebase,
base::TimeDelta interval) {
void DelegatedFrameHost::OnUpdateVSyncParameters(
base::TimeTicks timebase,
base::TimeDelta interval) {
vsync_timebase_ = timebase;
vsync_interval_ = interval;
if (client_->DelegatedFrameHostIsVisible())
client_->DelegatedFrameHostUpdateVSyncParameters(timebase, interval);
}

////////////////////////////////////////////////////////////////////////////////
Expand All @@ -917,6 +920,8 @@ DelegatedFrameHost::~DelegatedFrameHost() {
surface_factory_->Destroy(surface_id_);
if (resource_collection_.get())
resource_collection_->SetClient(NULL);

DCHECK(!vsync_manager_.get());
}

void DelegatedFrameHost::RunOnCommitCallbacks() {
Expand All @@ -943,6 +948,9 @@ void DelegatedFrameHost::SetCompositor(ui::Compositor* compositor) {
return;
compositor_ = compositor;
compositor_->AddObserver(this);
DCHECK(!vsync_manager_.get());
vsync_manager_ = compositor_->vsync_manager();
vsync_manager_->AddObserver(this);
}

void DelegatedFrameHost::ResetCompositor() {
Expand All @@ -955,6 +963,10 @@ void DelegatedFrameHost::ResetCompositor() {
}
if (compositor_->HasObserver(this))
compositor_->RemoveObserver(this);
if (vsync_manager_.get()) {
vsync_manager_->RemoveObserver(this);
vsync_manager_ = NULL;
}
compositor_ = nullptr;
}

Expand Down
16 changes: 14 additions & 2 deletions content/browser/compositor/delegated_frame_host.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#include "content/public/browser/render_process_host.h"
#include "ui/compositor/compositor.h"
#include "ui/compositor/compositor_observer.h"
#include "ui/compositor/compositor_vsync_manager.h"
#include "ui/compositor/layer.h"
#include "ui/compositor/layer_owner_delegate.h"
#include "ui/gfx/geometry/rect_conversions.h"
Expand Down Expand Up @@ -60,6 +61,10 @@ class CONTENT_EXPORT DelegatedFrameHostClient {
int output_surface_id,
const cc::CompositorFrameAck& ack) = 0;
virtual void DelegatedFrameHostOnLostCompositorResources() = 0;

virtual void DelegatedFrameHostUpdateVSyncParameters(
const base::TimeTicks& timebase,
const base::TimeDelta& interval) = 0;
};

// The DelegatedFrameHost is used to host all of the RenderWidgetHostView state
Expand All @@ -68,6 +73,7 @@ class CONTENT_EXPORT DelegatedFrameHostClient {
// the ui::Compositor associated with its DelegatedFrameHostClient.
class CONTENT_EXPORT DelegatedFrameHost
: public ui::CompositorObserver,
public ui::CompositorVSyncManager::Observer,
public ui::LayerOwnerDelegate,
public ImageTransportFactoryObserver,
public DelegatedFrameEvictorClient,
Expand All @@ -93,7 +99,6 @@ class CONTENT_EXPORT DelegatedFrameHost
gfx::Size GetRequestedRendererSize() const;
void SetCompositor(ui::Compositor* compositor);
void ResetCompositor();
void SetVSyncParameters(base::TimeTicks timebase, base::TimeDelta interval);
// Note: |src_subset| is specified in DIP dimensions while |output_size|
// expects pixels.
void CopyFromCompositingSurface(const gfx::Rect& src_subrect,
Expand Down Expand Up @@ -153,6 +158,10 @@ class CONTENT_EXPORT DelegatedFrameHost
void OnCompositingLockStateChanged(ui::Compositor* compositor) override;
void OnCompositingShuttingDown(ui::Compositor* compositor) override;

// Overridden from ui::CompositorVSyncManager::Observer:
void OnUpdateVSyncParameters(base::TimeTicks timebase,
base::TimeDelta interval) override;

// Overridden from ui::LayerOwnerObserver:
void OnLayerRecreated(ui::Layer* old_layer, ui::Layer* new_layer) override;

Expand Down Expand Up @@ -234,8 +243,11 @@ class CONTENT_EXPORT DelegatedFrameHost

std::vector<base::Closure> on_compositing_did_commit_callbacks_;

// The vsync manager we are observing for changes, if any.
scoped_refptr<ui::CompositorVSyncManager> vsync_manager_;

// The current VSync timebase and interval. These are zero until the first
// call to UpdateVSyncParameters().
// call to OnUpdateVSyncParameters().
base::TimeTicks vsync_timebase_;
base::TimeDelta vsync_interval_;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,11 @@ namespace content {

GpuBrowserCompositorOutputSurface::GpuBrowserCompositorOutputSurface(
const scoped_refptr<ContextProviderCommandBuffer>& context,
const scoped_refptr<ui::CompositorVSyncManager>& vsync_manager,
scoped_ptr<BrowserCompositorOverlayCandidateValidator>
overlay_candidate_validator)
: BrowserCompositorOutputSurface(context,
vsync_manager,
overlay_candidate_validator.Pass()),
#if defined(OS_MACOSX)
should_show_frames_state_(SHOULD_SHOW_FRAMES),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,10 @@
#include "base/cancelable_callback.h"
#include "content/browser/compositor/browser_compositor_output_surface.h"

namespace ui {
class CompositorVSyncManager;
}

namespace content {
class CommandBufferProxyImpl;
class BrowserCompositorOverlayCandidateValidator;
Expand All @@ -20,6 +24,7 @@ class GpuBrowserCompositorOutputSurface
public:
GpuBrowserCompositorOutputSurface(
const scoped_refptr<ContextProviderCommandBuffer>& context,
const scoped_refptr<ui::CompositorVSyncManager>& vsync_manager,
scoped_ptr<BrowserCompositorOverlayCandidateValidator>
overlay_candidate_validator);

Expand Down
9 changes: 5 additions & 4 deletions content/browser/compositor/gpu_process_transport_factory.cc
Original file line number Diff line number Diff line change
Expand Up @@ -280,27 +280,28 @@ void GpuProcessTransportFactory::EstablishedGpuChannel(
scoped_ptr<BrowserCompositorOutputSurface> surface;
if (!create_gpu_output_surface) {
surface = make_scoped_ptr(new SoftwareBrowserCompositorOutputSurface(
CreateSoftwareOutputDevice(compositor.get())));
CreateSoftwareOutputDevice(compositor.get()),
compositor->vsync_manager()));
} else {
DCHECK(context_provider);
if (!data->surface_id) {
surface = make_scoped_ptr(new OffscreenBrowserCompositorOutputSurface(
context_provider,
context_provider, compositor->vsync_manager(),
scoped_ptr<BrowserCompositorOverlayCandidateValidator>()));
} else
#if defined(USE_OZONE)
if (ui::SurfaceFactoryOzone::GetInstance()
->CanShowPrimaryPlaneAsOverlay()) {
surface =
make_scoped_ptr(new GpuSurfacelessBrowserCompositorOutputSurface(
context_provider, data->surface_id,
context_provider, data->surface_id, compositor->vsync_manager(),
CreateOverlayCandidateValidator(compositor->widget()), GL_RGB,
BrowserGpuMemoryBufferManager::current()));
} else
#endif
{
surface = make_scoped_ptr(new GpuBrowserCompositorOutputSurface(
context_provider,
context_provider, compositor->vsync_manager(),
CreateOverlayCandidateValidator(compositor->widget())));
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,13 @@ GpuSurfacelessBrowserCompositorOutputSurface::
GpuSurfacelessBrowserCompositorOutputSurface(
const scoped_refptr<ContextProviderCommandBuffer>& context,
int surface_id,
const scoped_refptr<ui::CompositorVSyncManager>& vsync_manager,
scoped_ptr<BrowserCompositorOverlayCandidateValidator>
overlay_candidate_validator,
unsigned internalformat,
BrowserGpuMemoryBufferManager* gpu_memory_buffer_manager)
: GpuBrowserCompositorOutputSurface(context,
vsync_manager,
overlay_candidate_validator.Pass()),
internalformat_(internalformat),
gpu_memory_buffer_manager_(gpu_memory_buffer_manager) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ class GpuSurfacelessBrowserCompositorOutputSurface
GpuSurfacelessBrowserCompositorOutputSurface(
const scoped_refptr<ContextProviderCommandBuffer>& context,
int surface_id,
const scoped_refptr<ui::CompositorVSyncManager>& vsync_manager,
scoped_ptr<BrowserCompositorOverlayCandidateValidator>
overlay_candidate_validator,
unsigned internalformat,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,11 @@ namespace content {
OffscreenBrowserCompositorOutputSurface::
OffscreenBrowserCompositorOutputSurface(
const scoped_refptr<ContextProviderCommandBuffer>& context,
const scoped_refptr<ui::CompositorVSyncManager>& vsync_manager,
scoped_ptr<BrowserCompositorOverlayCandidateValidator>
overlay_candidate_validator)
: BrowserCompositorOutputSurface(context,
vsync_manager,
overlay_candidate_validator.Pass()),
fbo_(0),
is_backbuffer_discarded_(false),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,10 @@
#include "base/memory/weak_ptr.h"
#include "content/browser/compositor/browser_compositor_output_surface.h"

namespace ui {
class CompositorVSyncManager;
}

namespace content {
class CommandBufferProxyImpl;

Expand All @@ -17,6 +21,7 @@ class OffscreenBrowserCompositorOutputSurface
public:
OffscreenBrowserCompositorOutputSurface(
const scoped_refptr<ContextProviderCommandBuffer>& context,
const scoped_refptr<ui::CompositorVSyncManager>& vsync_manager,
scoped_ptr<BrowserCompositorOverlayCandidateValidator>
overlay_candidate_validator);

Expand Down
Loading

0 comments on commit e627c52

Please sign in to comment.