Use element id's for animations

cc/animation/element_id.cc

Index: cc/animation/element_id.cc
diff --git a/cc/animation/element_id.cc b/cc/animation/element_id.cc
new file mode 100644
index 0000000000000000000000000000000000000000..ddc7548cd456d0cafbe7a4d73952b06ff1c975e8
--- /dev/null
+++ b/cc/animation/element_id.cc
@@ -0,0 +1,52 @@
+// Copyright 2016 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 "cc/animation/element_id.h"
+
+#include <limits>
+#include <ostream>
+
+#include "base/trace_event/trace_event_argument.h"
+#include "base/values.h"
+#include "cc/proto/element_id.pb.h"
+
+namespace cc {
+
+bool CC_EXPORT operator==(uint64_t o, const ElementId& e) {

[jbroman, 2016/06/02 20:20:21]

What's going on here? CC_EXPORT is only meaningful if it can actually reach
another translation unit (which it can't -- this isn't a header). Do these have
any call sites, in any event?

[Ian Vollick, 2016/06/03 19:31:21]

This copy and paste error was supposed to have been removed but got missed.
Removed.

+  return o == e.value;
+}
+
+bool CC_EXPORT operator!=(uint64_t o, const ElementId& e) {
+  return o != e.value;
+}
+
+ElementId CC_EXPORT LayerIdToElementIdForTesting(int layer_id) {
+  return ElementId(std::numeric_limits<uint64_t>::max() - layer_id);
+}
+
+void ElementId::AddToTracedValue(base::trace_event::TracedValue* res) const {
+  res->SetInteger("element_id", base::saturated_cast<int>(value));

[jbroman, 2016/06/02 20:20:21]

saturated_cast is wrong here: anything with a non-zero sub-element ID will
become INT_MAX (if, as is usual, int is int32_t).

You seem to be assuming here that int and int32_t are the same type; why not say
int32_t here if you actually want 32-bit parts? Also, a saturated_cast 

Also, does the compositor make any assumptions about the structure of this
integer, aside from this for tracing? Are all cc clients expected to obey this
32/32 split?

[Ian Vollick, 2016/06/03 19:31:21]

Thank you for catching this. Since this approach is prone to bugs like these,
I've switched to storing two ints. I've named them primaryId and secondaryId to
keep them relatively opaque.

+  res->SetInteger("sub_element_id", base::saturated_cast<int>(value >> 32));
+}
+
+std::unique_ptr<base::Value> ElementId::AsValue() const {
+  std::unique_ptr<base::DictionaryValue> res(new base::DictionaryValue());
+  res->SetInteger("element_id", base::saturated_cast<int>(value));
+  res->SetInteger("sub_element_id", base::saturated_cast<int>(value >> 32));
+  return std::move(res);
+}
+
+void ElementId::ToProtobuf(proto::ElementId* proto) const {
+  proto->set_id(value);
+}
+
+void ElementId::FromProtobuf(const proto::ElementId& proto) {
+  value = proto.id();
+}
+
+std::ostream& operator<<(std::ostream& out, const ElementId& id) {
+  return out << id.value;
+}
+
+}  // namespace cc