Commit 860d7158 authored by Bo Liu's avatar Bo Liu Committed by Commit Bot

aw: Post resource returns

In preparation for new draw functor which does not have synchronous
invoke to free render thread resources, change all resource return to
posting to UI thread instead.

Correctness is relatively simple: if BrowserViewRenderer is destroyed
then there is no need to return any resources. Otherwise resources
are returned eventually. Detach/shutdown has the same logic. RTM just
neends to ensure all resource returns are posted.

There are some potential perf downsides:
There will be more tasks on the UI thread. This is not really avoidable.

Can no longer synchronous return resources before starting a new frame.
This may cause higher resource usage. This is technically avoidable
but is technically complicated, so go with the simple solution first.

Bug: 900965
Change-Id: Id34d05a575799f2cf22166da595ba5e062c0078c
Reviewed-on: https://chromium-review.googlesource.com/c/1385854Reviewed-by: default avatarTobias Sargeant <tobiasjs@chromium.org>
Commit-Queue: Bo <boliu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#619982}
parent 0d41b79c
......@@ -85,6 +85,13 @@ jlong AwGLFunctor::GetAwDrawGLViewContext(
return reinterpret_cast<intptr_t>(&render_thread_manager_);
}
void AwGLFunctor::RemoveFromCompositorFrameProducer(
JNIEnv* env,
const base::android::JavaParamRef<jobject>& obj) {
DCHECK_CURRENTLY_ON(BrowserThread::UI);
render_thread_manager_.RemoveFromCompositorFrameProducerOnUI();
}
jlong AwGLFunctor::GetCompositorFrameConsumer(
JNIEnv* env,
const base::android::JavaParamRef<jobject>& obj) {
......
......@@ -25,6 +25,9 @@ class AwGLFunctor : public RenderThreadManagerClient {
const base::android::JavaParamRef<jobject>& obj);
jlong GetAwDrawGLViewContext(JNIEnv* env,
const base::android::JavaParamRef<jobject>& obj);
void RemoveFromCompositorFrameProducer(
JNIEnv* env,
const base::android::JavaParamRef<jobject>& obj);
jlong GetCompositorFrameConsumer(
JNIEnv* env,
const base::android::JavaParamRef<jobject>& obj);
......
......@@ -107,14 +107,16 @@ BrowserViewRenderer::BrowserViewRenderer(
max_page_scale_factor_(0.f),
on_new_picture_enable_(false),
clear_view_(false),
offscreen_pre_raster_(false) {}
offscreen_pre_raster_(false),
weak_ptr_factory_(this) {}
BrowserViewRenderer::~BrowserViewRenderer() {
DCHECK(compositor_map_.empty());
SetCurrentCompositorFrameConsumer(nullptr);
while (compositor_frame_consumers_.size()) {
RemoveCompositorFrameConsumer(*compositor_frame_consumers_.begin());
}
DCHECK(!current_compositor_frame_consumer_);
}
base::WeakPtr<CompositorFrameProducer> BrowserViewRenderer::GetWeakPtr() {
return weak_ptr_factory_.GetWeakPtr();
}
void BrowserViewRenderer::SetCurrentCompositorFrameConsumer(
......@@ -124,7 +126,6 @@ void BrowserViewRenderer::SetCurrentCompositorFrameConsumer(
}
current_compositor_frame_consumer_ = compositor_frame_consumer;
if (current_compositor_frame_consumer_) {
compositor_frame_consumers_.insert(current_compositor_frame_consumer_);
current_compositor_frame_consumer_->SetCompositorFrameProducer(this);
OnParentDrawConstraintsUpdated(current_compositor_frame_consumer_);
}
......@@ -220,7 +221,6 @@ bool BrowserViewRenderer::OnDrawHardware() {
external_draw_constraints_ =
current_compositor_frame_consumer_->GetParentDrawConstraintsOnUI();
ReturnResourceFromParent(current_compositor_frame_consumer_);
UpdateMemoryPolicy();
gfx::Transform transform_for_tile_priority =
......@@ -255,12 +255,6 @@ gfx::Rect BrowserViewRenderer::ComputeViewportRectForTilePriority() {
return viewport_rect_for_tile_priority;
}
void BrowserViewRenderer::ReturnedResourceAvailable(
CompositorFrameConsumer* compositor_frame_consumer) {
DCHECK(compositor_frame_consumers_.count(compositor_frame_consumer));
ReturnResourceFromParent(compositor_frame_consumer);
}
void BrowserViewRenderer::OnParentDrawConstraintsUpdated(
CompositorFrameConsumer* compositor_frame_consumer) {
DCHECK(compositor_frame_consumer);
......@@ -276,20 +270,10 @@ void BrowserViewRenderer::OnParentDrawConstraintsUpdated(
}
void BrowserViewRenderer::RemoveCompositorFrameConsumer(
CompositorFrameConsumer* compositor_frame_consumer) {
DCHECK(compositor_frame_consumers_.count(compositor_frame_consumer));
compositor_frame_consumers_.erase(compositor_frame_consumer);
if (current_compositor_frame_consumer_ == compositor_frame_consumer) {
CompositorFrameConsumer* consumer) {
ReturnUncommittedFrames(consumer->PassUncommittedFrameOnUI());
if (current_compositor_frame_consumer_ == consumer)
SetCurrentCompositorFrameConsumer(nullptr);
}
// At this point the compositor frame consumer has to hand back all resources
// to the child compositor.
compositor_frame_consumer->DeleteHardwareRendererOnUI();
ReturnUncommittedFrames(
compositor_frame_consumer->PassUncommittedFrameOnUI());
ReturnResourceFromParent(compositor_frame_consumer);
compositor_frame_consumer->SetCompositorFrameProducer(nullptr);
}
void BrowserViewRenderer::ReturnUncommittedFrames(
......@@ -313,23 +297,14 @@ void BrowserViewRenderer::ReturnUnusedResource(
std::move(resources));
}
void BrowserViewRenderer::ReturnResourceFromParent(
CompositorFrameConsumer* compositor_frame_consumer) {
CompositorFrameConsumer::ReturnedResourcesMap returned_resource_map;
compositor_frame_consumer->SwapReturnedResourcesOnUI(&returned_resource_map);
for (auto& pair : returned_resource_map) {
CompositorID compositor_id = pair.first;
content::SynchronousCompositor* compositor = FindCompositor(compositor_id);
std::vector<viz::ReturnedResource> resources;
resources.swap(pair.second.resources);
if (compositor && !resources.empty()) {
compositor->ReturnResources(pair.second.layer_tree_frame_sink_id,
resources);
}
has_rendered_frame_ = true;
}
void BrowserViewRenderer::ReturnUsedResources(
const std::vector<viz::ReturnedResource>& resources,
const CompositorID& compositor_id,
uint32_t layer_tree_frame_sink_id) {
content::SynchronousCompositor* compositor = FindCompositor(compositor_id);
if (compositor && !resources.empty())
compositor->ReturnResources(layer_tree_frame_sink_id, resources);
has_rendered_frame_ = true;
}
bool BrowserViewRenderer::OnDrawSoftware(SkCanvas* canvas) {
......@@ -468,11 +443,9 @@ void BrowserViewRenderer::OnComputeScroll(base::TimeTicks animation_time) {
}
void BrowserViewRenderer::ReleaseHardware() {
for (auto* compositor_frame_consumer : compositor_frame_consumers_) {
if (current_compositor_frame_consumer_) {
ReturnUncommittedFrames(
compositor_frame_consumer->PassUncommittedFrameOnUI());
ReturnResourceFromParent(compositor_frame_consumer);
DCHECK(compositor_frame_consumer->ReturnedResourcesEmptyOnUI());
current_compositor_frame_consumer_->PassUncommittedFrameOnUI());
}
hardware_enabled_ = false;
has_rendered_frame_ = false;
......
......@@ -141,11 +141,13 @@ class BrowserViewRenderer : public content::SynchronousCompositorClient,
ui::TouchHandleDrawable* CreateDrawable() override;
// CompositorFrameProducer overrides
void ReturnedResourceAvailable(
CompositorFrameConsumer* compositor_frame_consumer) override;
void OnParentDrawConstraintsUpdated(
CompositorFrameConsumer* compositor_frame_consumer) override;
base::WeakPtr<CompositorFrameProducer> GetWeakPtr() override;
void RemoveCompositorFrameConsumer(
CompositorFrameConsumer* consumer) override;
void ReturnUsedResources(const std::vector<viz::ReturnedResource>& resources,
const CompositorID& compositor_id,
uint32_t layer_tree_frame_sink_id) override;
void OnParentDrawConstraintsUpdated(
CompositorFrameConsumer* compositor_frame_consumer) override;
void SetActiveCompositorID(const CompositorID& compositor_id);
......@@ -186,7 +188,6 @@ class BrowserViewRenderer : public content::SynchronousCompositorClient,
BrowserViewRendererClient* const client_;
const scoped_refptr<base::SingleThreadTaskRunner> ui_task_runner_;
CompositorFrameConsumer* current_compositor_frame_consumer_;
std::set<CompositorFrameConsumer*> compositor_frame_consumers_;
// The current compositor that's owned by the current RVH.
content::SynchronousCompositor* compositor_;
......@@ -243,6 +244,8 @@ class BrowserViewRenderer : public content::SynchronousCompositorClient,
ParentCompositorDrawConstraints external_draw_constraints_;
base::WeakPtrFactory<CompositorFrameProducer> weak_ptr_factory_;
DISALLOW_COPY_AND_ASSIGN(BrowserViewRenderer);
};
......
......@@ -500,10 +500,16 @@ class SwitchLayerTreeFrameSinkIdTest : public ResourceRenderingTest {
}
void CheckResults() override {
GetCompositorFrameConsumer()->DeleteHardwareRendererOnUI();
window_->Detach();
functor_->OnWindowDetached();
window_.reset();
ui_task_runner_->PostTask(
FROM_HERE,
base::BindOnce(&SwitchLayerTreeFrameSinkIdTest::PostedCheckResults,
base::Unretained(this)));
}
void PostedCheckResults() {
// Make sure resources for the last output surface are returned.
EXPECT_EQ(expected_return_count_,
GetReturnedResourceCounts()[last_layer_tree_frame_sink_id_]);
......@@ -539,6 +545,13 @@ class RenderThreadManagerDeletionTest : public ResourceRenderingTest {
void CheckResults() override {
LayerTreeFrameSinkResourceCountMap resource_counts;
functor_.reset();
ui_task_runner_->PostTask(
FROM_HERE,
base::BindOnce(&RenderThreadManagerDeletionTest::PostedCheckResults,
base::Unretained(this)));
}
void PostedCheckResults() {
// Make sure resources for the last frame are returned.
EXPECT_EQ(expected_return_count_, GetReturnedResourceCounts());
EndTest();
......@@ -601,6 +614,13 @@ class RenderThreadManagerSwitchTest : public ResourceRenderingTest {
void CheckResults() override {
LayerTreeFrameSinkResourceCountMap resource_counts;
functor_.reset();
ui_task_runner_->PostTask(
FROM_HERE,
base::BindOnce(&RenderThreadManagerSwitchTest::PostedCheckResults,
base::Unretained(this)));
}
void PostedCheckResults() {
// Make sure resources for all frames are returned.
EXPECT_EQ(expected_return_count_, GetReturnedResourceCounts());
EndTest();
......
......@@ -5,11 +5,8 @@
#ifndef ANDROID_WEBVIEW_BROWSER_COMPOSITOR_FRAME_CONSUMER_H_
#define ANDROID_WEBVIEW_BROWSER_COMPOSITOR_FRAME_CONSUMER_H_
#include <map>
#include "android_webview/browser/child_frame.h"
#include "android_webview/browser/parent_compositor_draw_constraints.h"
#include "components/viz/common/resources/returned_resource.h"
#include "ui/gfx/geometry/vector2d.h"
namespace android_webview {
......@@ -19,16 +16,6 @@ class CompositorFrameProducer;
class CompositorFrameConsumer {
public:
struct ReturnedResources {
ReturnedResources();
~ReturnedResources();
uint32_t layer_tree_frame_sink_id;
std::vector<viz::ReturnedResource> resources;
};
using ReturnedResourcesMap =
std::map<CompositorID, ReturnedResources, CompositorIDComparator>;
// A CompositorFrameConsumer may be registered with at most one
// CompositorFrameProducer.
// The producer is responsible for managing the relationship with its
......@@ -44,11 +31,7 @@ class CompositorFrameConsumer {
std::unique_ptr<ChildFrame> frame) = 0;
virtual ParentCompositorDrawConstraints GetParentDrawConstraintsOnUI()
const = 0;
virtual void SwapReturnedResourcesOnUI(
ReturnedResourcesMap* returned_resource_map) = 0;
virtual bool ReturnedResourcesEmptyOnUI() const = 0;
virtual ChildFrameQueue PassUncommittedFrameOnUI() = 0;
virtual void DeleteHardwareRendererOnUI() = 0;
protected:
virtual ~CompositorFrameConsumer() {}
......
......@@ -5,18 +5,27 @@
#ifndef ANDROID_WEBVIEW_BROWSER_COMPOSITOR_FRAME_PRODUCER_H_
#define ANDROID_WEBVIEW_BROWSER_COMPOSITOR_FRAME_PRODUCER_H_
#include <vector>
#include "android_webview/browser/compositor_id.h"
#include "base/memory/weak_ptr.h"
#include "components/viz/common/resources/returned_resource.h"
namespace android_webview {
class CompositorFrameConsumer;
class CompositorFrameProducer {
public:
virtual void ReturnedResourceAvailable(
CompositorFrameConsumer* compositor_frame_consumer) = 0;
virtual base::WeakPtr<CompositorFrameProducer> GetWeakPtr();
virtual void ReturnUsedResources(
const std::vector<viz::ReturnedResource>& resources,
const CompositorID& compositor_id,
uint32_t layer_tree_frame_sink_id) = 0;
virtual void OnParentDrawConstraintsUpdated(
CompositorFrameConsumer* compositor_frame_consumer) = 0;
virtual void RemoveCompositorFrameConsumer(
CompositorFrameConsumer* compositor_frame_consumer) = 0;
CompositorFrameConsumer* consumer) = 0;
protected:
virtual ~CompositorFrameProducer() {}
......
......@@ -24,17 +24,11 @@
namespace android_webview {
namespace {
constexpr base::TimeDelta kSlightlyMoreThanOneFrame =
base::TimeDelta::FromMilliseconds(17);
}
RenderThreadManager::RenderThreadManager(
RenderThreadManagerClient* client,
const scoped_refptr<base::SingleThreadTaskRunner>& ui_loop)
: ui_loop_(ui_loop),
client_(client),
compositor_frame_producer_(nullptr),
has_received_frame_(false),
inside_hardware_release_(false),
weak_factory_on_ui_thread_(this) {
......@@ -45,16 +39,14 @@ RenderThreadManager::RenderThreadManager(
RenderThreadManager::~RenderThreadManager() {
DCHECK(ui_loop_->BelongsToCurrentThread());
if (compositor_frame_producer_) {
compositor_frame_producer_->RemoveCompositorFrameConsumer(this);
}
DCHECK(!hardware_renderer_.get());
DCHECK(child_frames_.empty());
}
void RenderThreadManager::UpdateParentDrawConstraintsOnUI() {
DCHECK(ui_loop_->BelongsToCurrentThread());
if (compositor_frame_producer_) {
compositor_frame_producer_->OnParentDrawConstraintsUpdated(this);
if (producer_weak_ptr_) {
producer_weak_ptr_->OnParentDrawConstraintsUpdated(this);
}
}
......@@ -135,61 +127,16 @@ bool RenderThreadManager::IsInsideHardwareRelease() const {
return inside_hardware_release_;
}
RenderThreadManager::ReturnedResources::ReturnedResources()
: layer_tree_frame_sink_id(0u) {}
RenderThreadManager::ReturnedResources::~ReturnedResources() {}
void RenderThreadManager::InsertReturnedResourcesOnRT(
const std::vector<viz::ReturnedResource>& resources,
const CompositorID& compositor_id,
uint32_t layer_tree_frame_sink_id) {
if (resources.empty())
return;
base::AutoLock lock(lock_);
ReturnedResources& returned_resources =
returned_resources_map_[compositor_id];
if (returned_resources.layer_tree_frame_sink_id != layer_tree_frame_sink_id) {
returned_resources.resources.clear();
}
returned_resources.resources.insert(returned_resources.resources.end(),
resources.begin(), resources.end());
returned_resources.layer_tree_frame_sink_id = layer_tree_frame_sink_id;
if (!returned_resource_available_pending_) {
returned_resource_available_pending_ = true;
ui_loop_->PostDelayedTask(
FROM_HERE,
base::BindOnce(&RenderThreadManager::ReturnedResourceAvailableOnUI,
ui_thread_weak_ptr_),
kSlightlyMoreThanOneFrame * 2);
}
}
void RenderThreadManager::ReturnedResourceAvailableOnUI() {
bool empty = false;
{
base::AutoLock lock(lock_);
DCHECK(returned_resource_available_pending_);
returned_resource_available_pending_ = false;
empty = returned_resources_map_.empty();
}
if (!empty && compositor_frame_producer_) {
compositor_frame_producer_->ReturnedResourceAvailable(this);
}
}
void RenderThreadManager::SwapReturnedResourcesOnUI(
ReturnedResourcesMap* returned_resource_map) {
DCHECK(returned_resource_map->empty());
base::AutoLock lock(lock_);
returned_resource_map->swap(returned_resources_map_);
}
bool RenderThreadManager::ReturnedResourcesEmptyOnUI() const {
base::AutoLock lock(lock_);
return returned_resources_map_.empty();
ui_loop_->PostTask(
FROM_HERE, base::BindOnce(&CompositorFrameProducer::ReturnUsedResources,
producer_weak_ptr_, resources, compositor_id,
layer_tree_frame_sink_id));
}
void RenderThreadManager::DrawGL(AwDrawGLInfo* draw_info) {
......@@ -232,6 +179,12 @@ void RenderThreadManager::DrawGL(AwDrawGLInfo* draw_info) {
hardware_renderer_->DrawGL(draw_info);
}
void RenderThreadManager::RemoveFromCompositorFrameProducerOnUI() {
DCHECK(ui_loop_->BelongsToCurrentThread());
if (producer_weak_ptr_)
producer_weak_ptr_->RemoveCompositorFrameConsumer(this);
}
void RenderThreadManager::DeleteHardwareRendererOnUI() {
DCHECK(ui_loop_->BelongsToCurrentThread());
......@@ -260,10 +213,8 @@ void RenderThreadManager::DeleteHardwareRendererOnUI() {
void RenderThreadManager::SetCompositorFrameProducer(
CompositorFrameProducer* compositor_frame_producer) {
DCHECK(compositor_frame_producer == compositor_frame_producer_ ||
compositor_frame_producer_ == nullptr ||
compositor_frame_producer == nullptr);
compositor_frame_producer_ = compositor_frame_producer;
DCHECK(ui_loop_->BelongsToCurrentThread());
producer_weak_ptr_ = compositor_frame_producer->GetWeakPtr();
}
bool RenderThreadManager::HasFrameForHardwareRendererOnRT() const {
......
......@@ -40,11 +40,10 @@ class RenderThreadManager : public CompositorFrameConsumer {
std::unique_ptr<ChildFrame> SetFrameOnUI(
std::unique_ptr<ChildFrame> frame) override;
ParentCompositorDrawConstraints GetParentDrawConstraintsOnUI() const override;
void SwapReturnedResourcesOnUI(
ReturnedResourcesMap* returned_resource_map) override;
bool ReturnedResourcesEmptyOnUI() const override;
ChildFrameQueue PassUncommittedFrameOnUI() override;
void DeleteHardwareRendererOnUI() override;
void RemoveFromCompositorFrameProducerOnUI();
void DeleteHardwareRendererOnUI();
// Render thread methods.
gfx::Vector2d GetScrollOffsetOnRT();
......@@ -76,14 +75,13 @@ class RenderThreadManager : public CompositorFrameConsumer {
// UI thread methods.
void UpdateParentDrawConstraintsOnUI();
void ReturnedResourceAvailableOnUI();
bool IsInsideHardwareRelease() const;
void SetInsideHardwareRelease(bool inside);
// Accessed by UI thread.
scoped_refptr<base::SingleThreadTaskRunner> ui_loop_;
RenderThreadManagerClient* const client_;
CompositorFrameProducer* compositor_frame_producer_;
base::WeakPtr<CompositorFrameProducer> producer_weak_ptr_;
base::WeakPtr<RenderThreadManager> ui_thread_weak_ptr_;
// Whether any frame has been received on the UI thread by
// RenderThreadManager.
......@@ -98,8 +96,6 @@ class RenderThreadManager : public CompositorFrameConsumer {
ChildFrameQueue child_frames_;
bool inside_hardware_release_;
ParentCompositorDrawConstraints parent_draw_constraints_;
bool returned_resource_available_pending_ = false;
ReturnedResourcesMap returned_resources_map_;
base::WeakPtrFactory<RenderThreadManager> weak_factory_on_ui_thread_;
......
......@@ -75,6 +75,7 @@ FakeWindow::~FakeWindow() {
void FakeWindow::Detach() {
CheckCurrentlyOnUIThread();
view_->SetCurrentCompositorFrameConsumer(nullptr);
view_->OnDetachedFromWindow();
}
......@@ -213,6 +214,10 @@ void FakeWindow::CheckCurrentlyOnRT() {
FakeFunctor::FakeFunctor() : window_(nullptr) {}
FakeFunctor::~FakeFunctor() {
if (render_thread_manager_) {
render_thread_manager_->RemoveFromCompositorFrameProducerOnUI();
render_thread_manager_->DeleteHardwareRendererOnUI();
}
render_thread_manager_.reset();
}
......@@ -258,6 +263,12 @@ CompositorFrameConsumer* FakeFunctor::GetCompositorFrameConsumer() {
return render_thread_manager_.get();
}
void FakeFunctor::OnWindowDetached() {
if (!render_thread_manager_)
return;
render_thread_manager_->DeleteHardwareRendererOnUI();
}
void FakeFunctor::Invoke(WindowHooks* hooks) {
DCHECK(!callback_.is_null());
AwDrawGLInfo invoke_info;
......
......@@ -112,6 +112,7 @@ class FakeFunctor : public RenderThreadManagerClient {
void Invoke(WindowHooks* hooks);
CompositorFrameConsumer* GetCompositorFrameConsumer();
void OnWindowDetached();
// RenderThreadManagerClient overrides
bool RequestInvokeGL(bool wait_for_completion) override;
......
......@@ -44,6 +44,7 @@ public class AwGLFunctor implements AwFunctor {
@Override
public void destroy() {
assert mRefCount > 0;
nativeRemoveFromCompositorFrameProducer(mNativeAwGLFunctor);
removeReference();
}
......@@ -111,6 +112,7 @@ public class AwGLFunctor implements AwFunctor {
private native void nativeDeleteHardwareRenderer(long nativeAwGLFunctor);
private native long nativeGetAwDrawGLViewContext(long nativeAwGLFunctor);
private native void nativeRemoveFromCompositorFrameProducer(long nativeAwGLFunctor);
private native long nativeGetCompositorFrameConsumer(long nativeAwGLFunctor);
private static native long nativeGetAwDrawGLFunction();
......
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