WIP Hittest Component.

components/viz/hit_test/hit_test_aggregator.cc

Index: components/viz/hit_test/hit_test_aggregator.cc
diff --git a/components/viz/hit_test/hit_test_aggregator.cc b/components/viz/hit_test/hit_test_aggregator.cc
new file mode 100644
index 0000000000000000000000000000000000000000..48b5bf3c52b39d13973ac8e3f920ee37644257e8
--- /dev/null
+++ b/components/viz/hit_test/hit_test_aggregator.cc
@@ -0,0 +1,186 @@
+// 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/hit_test/hit_test_aggregator.h"
+#include "base/logging.h"
+#include "display_hit_test_data.h"
+
+namespace viz {
+namespace hit_test {
+
+namespace {
+// TODO(gklassen): Review and select appropriate sizes ( based on
+// telemetry? ).
+constexpr int kInitialSize = 1024;
+constexpr int kIncrementalSize = 1024;

[rjkroege, 2017/06/12 18:13:01]

You cannot tear this up and replace it later.

[gklassen, 2017/06/13 18:54:23]

It copies over the portion in-use before switching buffers, so it should be ok? 
( Although this may evolve as we implement shared memory and if we decide to use
two buffers and add data to the BeginFrame for synchronization instead of using
the read_offset_ to synchronize swap ). 

+}
+
+HitTestAggregator::HitTestAggregator() : weak_ptr_factory_(this) {
+  AllocateDisplayHitTestData();
+}
+HitTestAggregator::~HitTestAggregator() {}
+
+namespace {
+
+bool ValidateHitTestRegion(
+    const hit_test::mojom::HitTestRegionPtr& hit_test_region) {
+  if (hit_test_region->flags_ ==
+      hit_test::mojom::HitTestRegionFlags::HIT_TEST_CHILD_SURFACE) {
+    if (!hit_test_region->surface_id_.is_valid()) {
+      return false;
+    }
+  }
+  return true;
+}
+bool ValidateHitTestData(const hit_test::mojom::HitTestDataPtr& hit_test_data) {
+  for (auto& region : hit_test_data->regions_) {
+    if (!ValidateHitTestRegion(region)) {
+      return false;
+    }
+  }
+  return true;
+}
+
+}  // namespace
+
+void HitTestAggregator::SubmitHitTestData(
+    hit_test::mojom::HitTestDataPtr hit_test_data) {
+  DCHECK(ValidateHitTestData(hit_test_data));

[rjkroege, 2017/06/12 18:13:00]

We will need a comprehensive policy to deal with this that's smarter. You need
to leave a TODO here. In particular, if a renderer is submitting invalid HTD
instances, viz needs to tattle on it to the host and the host has to take
appropriate action. 

[gklassen, 2017/06/13 18:54:23]

Good call and thank you. Done.

+  pending_[hit_test_data->surface_id_] = std::move(hit_test_data);
+}
+
+void HitTestAggregator::OnSurfaceDiscarded(const cc::SurfaceId& surface_id) {
+  pending_.erase(surface_id);
+  active_.erase(surface_id);
+}
+
+void HitTestAggregator::OnSurfaceWillDraw(const cc::SurfaceId& surface_id) {
+  auto search = pending_.find(surface_id);
+  if (search == pending_.end()) {
+    // Have already activated pending hit_test_data objects for this surface.
+    return;
+  }
+  active_[surface_id] = std::move(pending_[surface_id]);
+  pending_.erase(surface_id);
+}
+
+void HitTestAggregator::AllocateDisplayHitTestData() {
+  AllocateDisplayHitTestData(kInitialSize);
+}
+
+void HitTestAggregator::AllocateDisplayHitTestData(int length) {
+  size_t byte_count =
+      sizeof(DisplayHitTestData) + length * sizeof(DisplayHitTestRegion);
+  display_hit_test_data_ = (DisplayHitTestData*)malloc(byte_count);
+
+  display_hit_test_data_->length_ = length;
+  display_hit_test_data_->read_offset_ = 0;
+  display_hit_test_data_->regions_[0].child_count_ = kEndOfList;
+  display_hit_test_data_->regions_[length / 2].child_count_ = kEndOfList;
+}
+
+void HitTestAggregator::ResizeDisplayHitTestData(int length) {
+  DisplayHitTestData* old_display_hit_test_data = display_hit_test_data_;
+
+  AllocateDisplayHitTestData(length);
+
+  // Copy over the current data and then mark the old structure

[rjkroege, 2017/06/12 18:13:01]

Hm. We need to talk about this.

[gklassen, 2017/06/13 18:54:23]

Based on the discussion yesterday is this ok for now - until we can complete the
shared memory implementation and review whether to add values to BeginFrame to
facilitate swap?

+  // as invalid so that clients will re-acquire their reference.
+  int old_length = old_display_hit_test_data->length_;
+  int new_length = display_hit_test_data_->length_;
+  DCHECK(new_length > old_length);
+  memcpy(display_hit_test_data_->regions_, old_display_hit_test_data->regions_,
+         old_length * sizeof(DisplayHitTestRegion));
+
+  old_display_hit_test_data->read_offset_ = kOldPleaseReAcquire;
+}
+
+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));
+}
+
+namespace {

[rjkroege, 2017/06/12 18:13:01]

one anonymous namespace at the top is conventional

[gklassen, 2017/06/13 18:54:23]

Conventionalized & Done.

+
+int CountRegions(const HitTestDataMap& map) {
+  int size = 0;
+  for (auto const& it : map) {
+    hit_test::mojom::HitTestData* hit_test_data = it.second.get();
+    size += hit_test_data->regions_.size();

[rjkroege, 2017/06/12 18:13:01]

the regions are the same size? This is O(n). I am opposed. Track the length in a
member?

[gklassen, 2017/06/13 18:54:23]

Agreed.  I couldn't find a convenient place to track the size - but after review
we can keep track as they are added to the active_ map.  

Done. 

+  }
+  return size;
+}
+
+}  // namespace
+
+void HitTestAggregator::Aggregate(cc::SurfaceId display_surface_id) {
+  // Size check.
+  int number_of_regions = CountRegions(active_);
+  int length = display_hit_test_data_->length_ / 2;
+  if (number_of_regions + 1 > length) {
+    while (length < number_of_regions) {
+      length += kIncrementalSize;
+    }
+    ResizeDisplayHitTestData(length);

[rjkroege, 2017/06/12 18:13:00]

This means that we can have a large variance in the running time of Aggregate?
This is not necessarily a good idea. 

[gklassen, 2017/06/13 18:54:23]

Agreed, but I don't know of a better place to do this work ( yet ).  Any
suggestions?

+  }
+
+  int index = GetBackIndex();
+  int last_index = Append(display_surface_id, index);
+  display_hit_test_data_->regions_[last_index].child_count_ = kEndOfList;
+}
+
+int HitTestAggregator::Append(cc::SurfaceId surface_id, int index) {
+  auto search = active_.find(surface_id);
+  if (search == active_.end()) {
+    // Referenced surface not found ( it may be late ).
+    return index;
+  }
+  hit_test::mojom::HitTestData* hit_test_data = search->second.get();
+
+  for (auto& region : hit_test_data->regions_) {
+    index = Append(region, index);
+  }
+  return index;
+}
+
+int HitTestAggregator::Append(const hit_test::mojom::HitTestRegionPtr& region,
+                              int index) {
+  DisplayHitTestRegion* element = &display_hit_test_data_->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 (region->flags_ ==
+      hit_test::mojom::HitTestRegionFlags::HIT_TEST_CHILD_SURFACE) {
+    index = Append(region->surface_id_, index);
+  }
+
+  DCHECK((index - parent_index - 1) >= 0);
+  element->child_count_ = index - parent_index - 1;
+  return index;
+}
+
+void HitTestAggregator::Swap() {
+  display_hit_test_data_->read_offset_ = GetBackIndex();
+}
+
+int HitTestAggregator::GetBackIndex() {
+  if (display_hit_test_data_->read_offset_ == 0)
+    return display_hit_test_data_->length_ / 2;
+  return 0;
+}
+
+DisplayHitTestRegion* HitTestAggregator::GetCurrentRegions() {
+  return display_hit_test_data_->regions_ +
+         display_hit_test_data_->read_offset_;
+}
+
+}  // namespace hit_test
+}  // namespace viz