Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(559)

Unified Diff: components/viz/service/hit_test/hit_test_aggregator.cc

Issue 2938953002: Implement HitTestAggregator (Closed)
Patch Set: correct active count logic and add unit test to protect against similar errors Created 3 years, 6 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: components/viz/service/hit_test/hit_test_aggregator.cc
diff --git a/components/viz/service/hit_test/hit_test_aggregator.cc b/components/viz/service/hit_test/hit_test_aggregator.cc
new file mode 100644
index 0000000000000000000000000000000000000000..49bc6ec80a31e6762526143bd46445eb9cf6bd06
--- /dev/null
+++ b/components/viz/service/hit_test/hit_test_aggregator.cc
@@ -0,0 +1,218 @@
+// Copyright 2017 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "components/viz/service/hit_test/hit_test_aggregator.h"
+#include "components/viz/common/display_hit_test_region.h"
+
+namespace viz {
+namespace hit_test {
+
+namespace {
+// TODO(gklassen): Review and select appropriate sizes based on
+// telemetry / UMA.
+constexpr int kInitialSize = 1024;
+constexpr int kIncrementalSize = 1024;
+constexpr int kMaxRegionsPerSurface = 1024;
+constexpr int kMaxSize = 100 * 1024;
+
+bool ValidateHitTestRegion(
+ const hit_test::mojom::HitTestRegionPtr& hit_test_region) {
+ if (hit_test_region->flags == hit_test::mojom::kHitTestChildSurface) {
+ if (!hit_test_region->surface_id.is_valid())
+ return false;
+ }
+
+ return true;
+}
+
+bool ValidateHitTestRegionList(
+ const hit_test::mojom::HitTestRegionListPtr& hit_test_region_list) {
+ if (hit_test_region_list->regions.size() > kMaxRegionsPerSurface)
+ return false;
+ for (auto& region : hit_test_region_list->regions) {
+ if (!ValidateHitTestRegion(region))
+ return false;
+ }
+ return true;
+}
+
+} // namespace
+
+HitTestAggregator::HitTestAggregator()
+ : active_region_count_(0), weak_ptr_factory_(this) {
+ AllocateDisplayHitTestRegionList();
+}
+
+HitTestAggregator::~HitTestAggregator() {}
Fady Samuel 2017/07/12 14:46:38 nit: = default
gklassen 2017/07/12 17:11:53 Done.
+
+void HitTestAggregator::SubmitHitTestRegionList(
+ hit_test::mojom::HitTestRegionListPtr hit_test_region_list) {
+ DCHECK(ValidateHitTestRegionList(hit_test_region_list));
+ // TODO(gklassen): Runtime validation that hit_test_region_list is valid.
+ // TODO(gklassen): Inform FrameSink that the hit_test_region_list is invalid.
+ // TODO(gklassen): FrameSink needs to inform the host of a difficult renderer.
+ pending_[hit_test_region_list->surface_id] = std::move(hit_test_region_list);
+}
+
+bool HitTestAggregator::OnSurfaceDamaged(const cc::SurfaceId& surface_id,
+ const cc::BeginFrameAck& ack) {
+ return false;
+}
+
+void HitTestAggregator::OnSurfaceDiscarded(const cc::SurfaceId& surface_id) {
+ // Update the region count.
+ auto active_search = active_.find(surface_id);
+ if (active_search != active_.end()) {
+ hit_test::mojom::HitTestRegionList* old_hit_test_data =
+ active_search->second.get();
+ active_region_count_ -= old_hit_test_data->regions.size();
+ }
+ DCHECK_GE(active_region_count_, 0);
+
+ pending_.erase(surface_id);
+ active_.erase(surface_id);
+}
+
+void HitTestAggregator::OnSurfaceWillDraw(const cc::SurfaceId& surface_id) {
+ auto pending_search = pending_.find(surface_id);
+ if (pending_search == pending_.end()) {
+ // Have already activated pending hit_test_region_list objects for this
+ // surface.
+ return;
+ }
+ hit_test::mojom::HitTestRegionList* hit_test_region_list =
+ pending_search->second.get();
+
+ // Update the region count.
+ auto active_search = active_.find(surface_id);
+ if (active_search != active_.end()) {
+ hit_test::mojom::HitTestRegionList* old_hit_test_data =
+ active_search->second.get();
+ active_region_count_ -= old_hit_test_data->regions.size();
+ }
+ active_region_count_ += hit_test_region_list->regions.size();
+ DCHECK_GE(active_region_count_, 0);
+
+ active_[surface_id] = std::move(pending_[surface_id]);
+ pending_.erase(surface_id);
+}
+
+void HitTestAggregator::AllocateDisplayHitTestRegionList() {
+ AllocateDisplayHitTestRegionList(kInitialSize);
+ Swap();
+ AllocateDisplayHitTestRegionList(kInitialSize);
+}
+
+void HitTestAggregator::AllocateDisplayHitTestRegionList(int size) {
+ size_t num_bytes = size * sizeof(DisplayHitTestRegion);
+ write_handle_ = mojo::SharedBufferHandle::Create(num_bytes);
+ write_size_ = size;
+ write_buffer_ = write_handle_->Map(num_bytes);
+
+ DisplayHitTestRegion* region = (DisplayHitTestRegion*)write_buffer_.get();
+ region[0].child_count = kEndOfList;
+}
+
+void HitTestAggregator::PostTaskAggregate(cc::SurfaceId display_surface_id) {
+ base::ThreadTaskRunnerHandle::Get()->PostTask(
+ FROM_HERE,
+ base::BindOnce(&HitTestAggregator::Aggregate,
+ weak_ptr_factory_.GetWeakPtr(), display_surface_id));
+}
+
+void HitTestAggregator::Aggregate(cc::SurfaceId display_surface_id) {
+ // Check to ensure that enough memory has been allocated.
+ int size = write_size_;
+ int max_size = active_region_count_ + active_.size() + 1;
+ if (max_size > kMaxSize)
+ max_size = kMaxSize;
+
+ if (max_size > size) {
+ size = (1 + max_size / kIncrementalSize) * kIncrementalSize;
+ AllocateDisplayHitTestRegionList(size);
+ }
+
+ AppendRoot(display_surface_id);
+}
+
+void HitTestAggregator::AppendRoot(cc::SurfaceId surface_id) {
+ auto search = active_.find(surface_id);
+ CHECK(search != active_.end());
+
+ hit_test::mojom::HitTestRegionList* hit_test_region_list =
+ search->second.get();
+
+ DisplayHitTestRegion* regions = (DisplayHitTestRegion*)write_buffer_.get();
+
+ regions[0].frame_sink_id = hit_test_region_list->surface_id.frame_sink_id();
+ regions[0].flags = hit_test_region_list->flags;
+ regions[0].rect = hit_test_region_list->bounds;
+ regions[0].transform = hit_test_region_list->transform;
+
+ int index = 1;
+ for (const auto& region : hit_test_region_list->regions) {
+ if (index >= write_size_ - 1)
+ break;
+ index = AppendRegion(regions, region, index);
+ }
+
+ DCHECK_GE(index, 1);
+ regions[0].child_count = index - 1;
+ regions[index].child_count = kEndOfList;
+}
+
+int HitTestAggregator::AppendRegion(
+ DisplayHitTestRegion* regions,
+ const hit_test::mojom::HitTestRegionPtr& region,
+ int index) {
+ DisplayHitTestRegion* element = &regions[index];
+
+ element->frame_sink_id = region->surface_id.frame_sink_id();
+ element->flags = region->flags;
+ element->rect = region->rect;
+ element->transform = region->transform;
+
+ int parent_index = index++;
+ if (index >= write_size_ - 1) {
+ element->child_count = 0;
+ return index;
+ }
+
+ if (region->flags == hit_test::mojom::kHitTestChildSurface) {
+ auto search = active_.find(region->surface_id);
+ if (search == active_.end()) {
+ // Surface HitTestRegionList not found - it may be late.
+ // Don't include this region so that it doesn't receive events.
+ return parent_index;
+ }
+
+ // Rather than add a node in the tree for this hit_test_region_list element
+ // we can simplify the tree by merging the flags and transform into
+ // the kHitTestChildSurface element.
+ hit_test::mojom::HitTestRegionList* hit_test_region_list =
+ search->second.get();
+ if (!hit_test_region_list->transform.IsIdentity())
+ element->transform.PreconcatTransform(hit_test_region_list->transform);
+
+ element->flags |= hit_test_region_list->flags;
+
+ for (const auto& child_region : hit_test_region_list->regions) {
+ index = AppendRegion(regions, child_region, index);
+ if (index >= write_size_ - 1)
+ break;
+ }
+ }
+ DCHECK_GE(index - parent_index - 1, 0);
+ element->child_count = index - parent_index - 1;
+ return index;
+}
+
+void HitTestAggregator::Swap() {
+ std::swap(read_handle_, write_handle_);
+ std::swap(read_size_, write_size_);
+ std::swap(read_buffer_, write_buffer_);
+}
+
+} // namespace hit_test
+} // namespace viz

Powered by Google App Engine
This is Rietveld 408576698