Implement HitTestAggregator

components/viz/common/hit_test/display_hit_test_data.h

Index: components/viz/common/hit_test/display_hit_test_data.h
diff --git a/components/viz/common/hit_test/display_hit_test_data.h b/components/viz/common/hit_test/display_hit_test_data.h
new file mode 100644
index 0000000000000000000000000000000000000000..d1182f96cffcf5b77969a477e2a52f83b7157f5f
--- /dev/null
+++ b/components/viz/common/hit_test/display_hit_test_data.h
@@ -0,0 +1,50 @@
+// 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.
+
+#ifndef COMPONENTS_VIZ_COMMON_HIT_TEST_DISPLAY_HIT_TEST_DATA_H_
+#define COMPONENTS_VIZ_COMMON_HIT_TEST_DISPLAY_HIT_TEST_DATA_H_
+
+#include <stdint.h>
+
+#include "cc/surfaces/surface_id.h"
+#include "cc/surfaces/surface_observer.h"

[varkha, 2017/06/19 21:15:04]

Is this header necessary?

[gklassen, 2017/06/20 16:09:38]

No, you're right, it isn't.  Done.

+#include "services/viz/hit_test/public/interfaces/hit_test_data.mojom.h"

[varkha, 2017/06/19 21:15:04]

Ditto.

[gklassen, 2017/06/20 16:09:37]

This one is necessary.  It pulls in the constants that define valid values for
flags and it also pulls in the appropriate definitions for the types used in the
structure. 

[varkha, 2017/06/20 19:56:58]

Not sure about the style with mojo. Do we usually do that? I am used to bringing
in only the strictly required includes in the header files and leaving the rest
to be included / imported in .cc files.

[gklassen, 2017/06/26 21:55:19]

Done.

+
+namespace viz {
+
+// DiplayHitTestData contains the hit_test data for the Display.

[varkha, 2017/06/19 21:15:03]

nit: introduce hit_test? Or did you mean hit-test in a general sense?

[gklassen, 2017/06/20 16:09:38]

Done.

+//
+// It is designed to be in shared memory so that the viz process can
+// write the hit_test data, and the browser / ws process can read without
+// process hops.
+//
+// One instance of this class contains a double buffer of entries -
+// the viz process will write into one half while client processes read from
+// the other.  read_offset_ is used to swap buffers atomically.

[varkha, 2017/06/19 21:15:03]

nit: s/read_offset_/|read_offset|

[gklassen, 2017/06/20 16:09:37]

Done.

+
+// A DisplayHitTestRegion element with child_count of kEndOfList indicates
+// the last element ( end of the list ).
+constexpr int kEndOfList = -1;
+
+// A read_offset_ set to kOldPleaseReAcquire indicates that the buffer has been

[varkha, 2017/06/19 21:15:04]

nit: s/read_offset_/|read_offset|

[gklassen, 2017/06/20 16:09:37]

Done.

+// resized and clients should re-acquire their reference.
+constexpr int kOldPleaseReAcquire = -1;
+
+struct DisplayHitTestRegion {
+  cc::FrameSinkId frame_sink_id;

[varkha, 2017/06/19 21:15:04]

Needs cc/surfaces/frame_sink_id.h ?

[gklassen, 2017/06/20 16:09:37]

These are pulled in with the mojom reference and definition. 

[gklassen, 2017/06/26 21:55:19]

Done.

+  uint32_t flags;
+  gfx::Rect rect;

[varkha, 2017/06/19 21:15:04]

does this need a header?

[gklassen, 2017/06/26 21:55:18]

Done.

+  gfx::Transform transform;

[varkha, 2017/06/19 21:15:03]

Ditto.

[gklassen, 2017/06/26 21:55:19]

Done.

+  int child_count;
+};
+
+struct DisplayHitTestData {
+  int length;
+  base::subtle::Atomic32 read_offset;

[varkha, 2017/06/19 21:15:03]

Needs base/atomicops.h ?

[gklassen, 2017/06/20 16:09:37]

This will need to change anyways - we will be changing the synchronization
strategy.

[gklassen, 2017/06/26 21:55:19]

Done.

+  DisplayHitTestRegion regions[];
+};
+
+}  // namespace viz
+
+#endif  //  COMPONENTS_VIZ_COMMON_HIT_TEST_DISPLAY_HIT_TEST_DATA_H_

[varkha, 2017/06/19 21:15:04]

nit: remove extra space after //

[gklassen, 2017/06/20 16:09:38]

Done.