Skip to content

Commit

Permalink
Use FrameSinkId instead of CompositorID
Browse files Browse the repository at this point in the history
For renderer compositors, both are just the process and routing id.
FrameSinkId will be used more widely with webview viz. FrameSinkId
supports equality check, comparison, and hashing, so everything
CompositorID does already. So switch to FrameSinkId and delete
CompositorID.

Bug: 805739
Change-Id: I64c3f2821aaefba0949db111c3e56bfc1a85bcc0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1745807
Reviewed-by: Eric Karl <ericrk@chromium.org>
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Commit-Queue: Bo <boliu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#686276}
  • Loading branch information
Bo Liu authored and Commit Bot committed Aug 13, 2019
1 parent e5614a7 commit 5af9c44
Show file tree
Hide file tree
Showing 29 changed files with 134 additions and 212 deletions.
41 changes: 12 additions & 29 deletions android_webview/browser/aw_contents.cc
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@
#include "components/autofill/core/browser/autofill_manager.h"
#include "components/autofill/core/browser/webdata/autofill_webdata_service.h"
#include "components/navigation_interception/intercept_navigation_delegate.h"
#include "components/viz/common/surfaces/frame_sink_id.h"
#include "content/public/browser/android/child_process_importance.h"
#include "content/public/browser/android/synchronous_compositor.h"
#include "content/public/browser/browser_task_traits.h"
Expand Down Expand Up @@ -234,16 +235,13 @@ AwContents::AwContents(std::unique_ptr<WebContents> web_contents)
std::make_unique<AwContentsUserData>(this));
browser_view_renderer_.RegisterWithWebContents(web_contents_.get());

CompositorID compositor_id;
if (web_contents_->GetRenderViewHost() &&
web_contents_->GetRenderViewHost()->GetProcess()) {
compositor_id.process_id =
web_contents_->GetRenderViewHost()->GetProcess()->GetID();
compositor_id.routing_id =
web_contents_->GetRenderViewHost()->GetWidget()->GetRoutingID();
viz::FrameSinkId frame_sink_id;
if (web_contents_->GetRenderViewHost()) {
frame_sink_id =
web_contents_->GetRenderViewHost()->GetWidget()->GetFrameSinkId();
}

browser_view_renderer_.SetActiveCompositorID(compositor_id);
browser_view_renderer_.SetActiveFrameSinkId(frame_sink_id);
render_view_host_ext_.reset(
new AwRenderViewHostExt(this, web_contents_.get()));

Expand Down Expand Up @@ -1399,15 +1397,12 @@ void AwContents::RenderViewHostChanged(content::RenderViewHost* old_host,
content::RenderViewHost* new_host) {
DCHECK(new_host);

int process_id = new_host->GetProcess()->GetID();
int routing_id = new_host->GetWidget()->GetRoutingID();

// At this point, the current RVH may or may not contain a compositor. So
// compositor_ may be nullptr, in which case
// BrowserViewRenderer::DidInitializeCompositor() callback is time when the
// new compositor is constructed.
browser_view_renderer_.SetActiveCompositorID(
CompositorID(process_id, routing_id));
browser_view_renderer_.SetActiveFrameSinkId(
new_host->GetWidget()->GetFrameSinkId());
}

void AwContents::DidFinishNavigation(
Expand Down Expand Up @@ -1435,28 +1430,16 @@ void AwContents::DidFinishNavigation(
}

void AwContents::DidAttachInterstitialPage() {
CompositorID compositor_id;
RenderFrameHost* rfh = web_contents_->GetInterstitialPage()->GetMainFrame();
compositor_id.process_id = rfh->GetProcess()->GetID();
compositor_id.routing_id =
rfh->GetRenderViewHost()->GetWidget()->GetRoutingID();
browser_view_renderer_.SetActiveCompositorID(compositor_id);
browser_view_renderer_.SetActiveFrameSinkId(
rfh->GetRenderViewHost()->GetWidget()->GetFrameSinkId());
}

void AwContents::DidDetachInterstitialPage() {
CompositorID compositor_id;
if (!web_contents_)
return;
if (web_contents_->GetRenderViewHost() &&
web_contents_->GetRenderViewHost()->GetProcess()) {
compositor_id.process_id =
web_contents_->GetRenderViewHost()->GetProcess()->GetID();
compositor_id.routing_id =
web_contents_->GetRenderViewHost()->GetWidget()->GetRoutingID();
} else {
LOG(WARNING) << "failed setting the compositor on detaching interstitital";
}
browser_view_renderer_.SetActiveCompositorID(compositor_id);
browser_view_renderer_.SetActiveFrameSinkId(
web_contents_->GetRenderViewHost()->GetWidget()->GetFrameSinkId());
}

bool AwContents::CanShowInterstitial() {
Expand Down
2 changes: 0 additions & 2 deletions android_webview/browser/gfx/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,6 @@ source_set("gfx") {
"child_frame.h",
"compositor_frame_consumer.h",
"compositor_frame_producer.h",
"compositor_id.cc",
"compositor_id.h",
"deferred_gpu_command_service.cc",
"deferred_gpu_command_service.h",
"gpu_service_web_view.cc",
Expand Down
40 changes: 18 additions & 22 deletions android_webview/browser/gfx/browser_view_renderer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -212,8 +212,8 @@ gfx::Rect BrowserViewRenderer::ComputeTileRectAndUpdateMemoryPolicy() {
}

content::SynchronousCompositor* BrowserViewRenderer::FindCompositor(
const CompositorID& compositor_id) const {
const auto& compositor_iterator = compositor_map_.find(compositor_id);
const viz::FrameSinkId& frame_sink_id) const {
const auto& compositor_iterator = compositor_map_.find(frame_sink_id);
if (compositor_iterator == compositor_map_.end())
return nullptr;

Expand Down Expand Up @@ -292,7 +292,7 @@ bool BrowserViewRenderer::OnDrawHardware() {
copy_request_ptr->set_result_task_runner(ui_task_runner_);
}
std::unique_ptr<ChildFrame> child_frame = std::make_unique<ChildFrame>(
std::move(future), compositor_id_, viewport_size_for_tile_priority,
std::move(future), frame_sink_id_, viewport_size_for_tile_priority,
external_draw_constraints_.transform, offscreen_pre_raster_,
std::move(requests));

Expand All @@ -315,7 +315,7 @@ void BrowserViewRenderer::OnParentDrawDataUpdated(
bool BrowserViewRenderer::DoUpdateParentDrawData() {
ParentCompositorDrawConstraints new_constraints;
viz::FrameTimingDetailsMap new_timing_details;
CompositorID id;
viz::FrameSinkId id;
uint32_t frame_token = 0u;
current_compositor_frame_consumer_->TakeParentDrawDataOnUI(
&new_constraints, &id, &new_timing_details, &frame_token);
Expand Down Expand Up @@ -359,17 +359,17 @@ void BrowserViewRenderer::ReturnUnusedResource(
viz::TransferableResource::ReturnResources(
child_frame->frame->resource_list);
content::SynchronousCompositor* compositor =
FindCompositor(child_frame->compositor_id);
FindCompositor(child_frame->frame_sink_id);
if (compositor && !resources.empty())
compositor->ReturnResources(child_frame->layer_tree_frame_sink_id,
std::move(resources));
}

void BrowserViewRenderer::ReturnUsedResources(
const std::vector<viz::ReturnedResource>& resources,
const CompositorID& compositor_id,
const viz::FrameSinkId& frame_sink_id,
uint32_t layer_tree_frame_sink_id) {
content::SynchronousCompositor* compositor = FindCompositor(compositor_id);
content::SynchronousCompositor* compositor = FindCompositor(frame_sink_id);
if (compositor && !resources.empty())
compositor->ReturnResources(layer_tree_frame_sink_id, resources);
has_rendered_frame_ = true;
Expand Down Expand Up @@ -541,46 +541,42 @@ gfx::Rect BrowserViewRenderer::GetScreenRect() const {

void BrowserViewRenderer::DidInitializeCompositor(
content::SynchronousCompositor* compositor,
int process_id,
int routing_id) {
const viz::FrameSinkId& frame_sink_id) {
TRACE_EVENT_INSTANT0("android_webview",
"BrowserViewRenderer::DidInitializeCompositor",
TRACE_EVENT_SCOPE_THREAD);
DCHECK(compositor);
CompositorID compositor_id(process_id, routing_id);
// This assumes that a RenderViewHost has at most 1 synchronous compositor
// througout its lifetime.
DCHECK(compositor_map_.count(compositor_id) == 0);
compositor_map_[compositor_id] = compositor;
DCHECK(compositor_map_.count(frame_sink_id) == 0);
compositor_map_[frame_sink_id] = compositor;

// At this point, the RVHChanged event for the new RVH that contains the
// |compositor| might have been fired already, in which case just set the
// current compositor with the new compositor.
if (compositor_id.Equals(compositor_id_))
if (frame_sink_id == frame_sink_id_)
SetActiveCompositor(compositor);
}

void BrowserViewRenderer::DidDestroyCompositor(
content::SynchronousCompositor* compositor,
int process_id,
int routing_id) {
const viz::FrameSinkId& frame_sink_id) {
TRACE_EVENT_INSTANT0("android_webview",
"BrowserViewRenderer::DidDestroyCompositor",
TRACE_EVENT_SCOPE_THREAD);
CompositorID compositor_id(process_id, routing_id);
DCHECK(compositor_map_.count(compositor_id));
DCHECK(compositor_map_.count(frame_sink_id));
if (compositor_ == compositor) {
compositor_ = nullptr;
copy_requests_.clear();
}

compositor_map_.erase(compositor_id);
compositor_map_.erase(frame_sink_id);
}

void BrowserViewRenderer::SetActiveCompositorID(
const CompositorID& compositor_id) {
compositor_id_ = compositor_id;
SetActiveCompositor(FindCompositor(compositor_id));
void BrowserViewRenderer::SetActiveFrameSinkId(
const viz::FrameSinkId& frame_sink_id) {
frame_sink_id_ = frame_sink_id;
SetActiveCompositor(FindCompositor(frame_sink_id));
}

void BrowserViewRenderer::SetActiveCompositor(
Expand Down
24 changes: 9 additions & 15 deletions android_webview/browser/gfx/browser_view_renderer.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,14 @@

#include "android_webview/browser/gfx/child_frame.h"
#include "android_webview/browser/gfx/compositor_frame_producer.h"
#include "android_webview/browser/gfx/compositor_id.h"
#include "android_webview/browser/gfx/parent_compositor_draw_constraints.h"
#include "base/callback.h"
#include "base/cancelable_callback.h"
#include "base/macros.h"
#include "base/memory/ref_counted.h"
#include "base/optional.h"
#include "base/trace_event/trace_event.h"
#include "components/viz/common/surfaces/frame_sink_id.h"
#include "content/public/browser/android/synchronous_compositor.h"
#include "content/public/browser/android/synchronous_compositor_client.h"
#include "third_party/skia/include/core/SkRefCnt.h"
Expand Down Expand Up @@ -119,11 +119,9 @@ class BrowserViewRenderer : public content::SynchronousCompositorClient,

// SynchronousCompositorClient overrides.
void DidInitializeCompositor(content::SynchronousCompositor* compositor,
int process_id,
int routing_id) override;
const viz::FrameSinkId& frame_sink_id) override;
void DidDestroyCompositor(content::SynchronousCompositor* compositor,
int process_id,
int routing_id) override;
const viz::FrameSinkId& frame_sink_id) override;
void PostInvalidate(content::SynchronousCompositor* compositor) override;
void DidUpdateContent(content::SynchronousCompositor* compositor) override;

Expand Down Expand Up @@ -152,14 +150,14 @@ class BrowserViewRenderer : public content::SynchronousCompositorClient,
void RemoveCompositorFrameConsumer(
CompositorFrameConsumer* consumer) override;
void ReturnUsedResources(const std::vector<viz::ReturnedResource>& resources,
const CompositorID& compositor_id,
const viz::FrameSinkId& frame_sink_id,
uint32_t layer_tree_frame_sink_id) override;
void OnParentDrawDataUpdated(
CompositorFrameConsumer* compositor_frame_consumer) override;
void OnViewTreeForceDarkStateChanged(
bool view_tree_force_dark_state) override;

void SetActiveCompositorID(const CompositorID& compositor_id);
void SetActiveFrameSinkId(const viz::FrameSinkId& frame_sink_id);

// Visible for testing.
content::SynchronousCompositor* GetActiveCompositorForTesting() const {
Expand Down Expand Up @@ -190,7 +188,7 @@ class BrowserViewRenderer : public content::SynchronousCompositorClient,
gfx::Rect ComputeTileRectAndUpdateMemoryPolicy();

content::SynchronousCompositor* FindCompositor(
const CompositorID& compositor_id) const;
const viz::FrameSinkId& frame_sink_id) const;
// For debug tracing or logging. Return the string representation of this
// view renderer's state.
std::string ToString() const;
Expand All @@ -201,16 +199,12 @@ class BrowserViewRenderer : public content::SynchronousCompositorClient,

// The current compositor that's owned by the current RVH.
content::SynchronousCompositor* compositor_;
// The process id and routing id of the most recent RVH according to
// RVHChanged.
CompositorID compositor_id_;
// The id of the most recent RVH according to RVHChanged.
viz::FrameSinkId frame_sink_id_;
// A map from compositor's per-WebView unique ID to the compositor's raw
// pointer. A raw pointer here is fine because the entry will be erased when
// a compositor is destroyed.
std::map<CompositorID,
content::SynchronousCompositor*,
CompositorIDComparator>
compositor_map_;
std::map<viz::FrameSinkId, content::SynchronousCompositor*> compositor_map_;

bool is_paused_;
bool view_visible_;
Expand Down
15 changes: 9 additions & 6 deletions android_webview/browser/gfx/browser_view_renderer_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#include "base/stl_util.h"
#include "base/threading/thread_task_runner_handle.h"
#include "components/viz/common/quads/compositor_frame.h"
#include "components/viz/common/surfaces/frame_sink_id.h"
#include "content/public/common/use_zoom_for_dsf_policy.h"
#include "content/public/test/test_synchronous_compositor_android.h"

Expand Down Expand Up @@ -50,14 +51,15 @@ class ActiveCompositorSwitchBeforeConstructionTest : public RenderingTest {
EXPECT_TRUE(success);
// Change compositor here. And do another ondraw.
// The previous active compositor id is 0, 0, now change it to 0, 1.
browser_view_renderer_->SetActiveCompositorID(CompositorID(0, 1));
browser_view_renderer_->SetActiveFrameSinkId(viz::FrameSinkId(0, 1));
browser_view_renderer_->PostInvalidate(ActiveCompositor());
break;
case 2:
// The 2nd ondraw is skipped because there is no active compositor at
// the moment.
EXPECT_FALSE(success);
new_compositor_.reset(new content::TestSynchronousCompositor(0, 1));
new_compositor_.reset(
new content::TestSynchronousCompositor(viz::FrameSinkId(0, 1)));
new_compositor_->SetClient(browser_view_renderer_.get());
EXPECT_EQ(ActiveCompositor(), new_compositor_.get());
browser_view_renderer_->PostInvalidate(ActiveCompositor());
Expand Down Expand Up @@ -99,9 +101,10 @@ class ActiveCompositorSwitchAfterConstructionTest : public RenderingTest {
EXPECT_TRUE(success);
// Create a new compositor here. And switch it to be active. And then
// do another ondraw.
new_compositor_.reset(new content::TestSynchronousCompositor(0, 1));
new_compositor_.reset(
new content::TestSynchronousCompositor(viz::FrameSinkId(0, 1)));
new_compositor_->SetClient(browser_view_renderer_.get());
browser_view_renderer_->SetActiveCompositorID(CompositorID(0, 1));
browser_view_renderer_->SetActiveFrameSinkId(viz::FrameSinkId(0, 1));

EXPECT_EQ(ActiveCompositor(), new_compositor_.get());
browser_view_renderer_->PostInvalidate(ActiveCompositor());
Expand Down Expand Up @@ -218,11 +221,11 @@ class TestAnimateInAndOutOfScreen : public RenderingTest {

void OnParentDrawDataUpdated() override {
ParentCompositorDrawConstraints constraints;
CompositorID id;
viz::FrameSinkId frame_sink_id;
viz::FrameTimingDetailsMap timing_details;
uint32_t frame_token = 0u;
GetCompositorFrameConsumer()->TakeParentDrawDataOnUI(
&constraints, &id, &timing_details, &frame_token);
&constraints, &frame_sink_id, &timing_details, &frame_token);
switch (on_draw_count_) {
case 0u:
// This OnParentDrawDataUpdated is generated by
Expand Down
4 changes: 2 additions & 2 deletions android_webview/browser/gfx/child_frame.cc
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,13 @@ namespace android_webview {

ChildFrame::ChildFrame(
scoped_refptr<content::SynchronousCompositor::FrameFuture> frame_future,
const CompositorID& compositor_id,
const viz::FrameSinkId& frame_sink_id,
const gfx::Size& viewport_size_for_tile_priority,
const gfx::Transform& transform_for_tile_priority,
bool offscreen_pre_raster,
CopyOutputRequestQueue copy_requests)
: frame_future(std::move(frame_future)),
compositor_id(compositor_id),
frame_sink_id(frame_sink_id),
viewport_size_for_tile_priority(viewport_size_for_tile_priority),
transform_for_tile_priority(transform_for_tile_priority),
offscreen_pre_raster(offscreen_pre_raster),
Expand Down
6 changes: 3 additions & 3 deletions android_webview/browser/gfx/child_frame.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@
#include <memory>
#include <vector>

#include "android_webview/browser/gfx/compositor_id.h"
#include "base/containers/circular_deque.h"
#include "base/macros.h"
#include "components/viz/common/surfaces/frame_sink_id.h"
#include "content/public/browser/android/synchronous_compositor.h"
#include "ui/gfx/geometry/size.h"
#include "ui/gfx/transform.h"
Expand All @@ -29,7 +29,7 @@ class ChildFrame {
public:
ChildFrame(
scoped_refptr<content::SynchronousCompositor::FrameFuture> frame_future,
const CompositorID& compositor_id,
const viz::FrameSinkId& frame_sink_id,
const gfx::Size& viewport_size_for_tile_priority,
const gfx::Transform& transform_for_tile_priority,
bool offscreen_pre_raster,
Expand All @@ -45,7 +45,7 @@ class ChildFrame {
uint32_t layer_tree_frame_sink_id = 0u;
std::unique_ptr<viz::CompositorFrame> frame;
// The id of the compositor this |frame| comes from.
const CompositorID compositor_id;
const viz::FrameSinkId frame_sink_id;
const gfx::Size viewport_size_for_tile_priority;
const gfx::Transform transform_for_tile_priority;
const bool offscreen_pre_raster;
Expand Down
7 changes: 5 additions & 2 deletions android_webview/browser/gfx/compositor_frame_consumer.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,14 @@
#define ANDROID_WEBVIEW_BROWSER_GFX_COMPOSITOR_FRAME_CONSUMER_H_

#include "android_webview/browser/gfx/child_frame.h"
#include "android_webview/browser/gfx/compositor_id.h"
#include "android_webview/browser/gfx/parent_compositor_draw_constraints.h"
#include "components/viz/common/frame_timing_details_map.h"
#include "ui/gfx/geometry/vector2d.h"

namespace viz {
class FrameSinkId;
}

namespace android_webview {

class ChildFrame;
Expand All @@ -33,7 +36,7 @@ class CompositorFrameConsumer {
std::unique_ptr<ChildFrame> frame) = 0;
virtual void TakeParentDrawDataOnUI(
ParentCompositorDrawConstraints* constraints,
CompositorID* compositor_id,
viz::FrameSinkId* frame_sink_id,
viz::FrameTimingDetailsMap* timing_details,
uint32_t* frame_token) = 0;
virtual ChildFrameQueue PassUncommittedFrameOnUI() = 0;
Expand Down
Loading

0 comments on commit 5af9c44

Please sign in to comment.