Change cc::ElementId to be a uint64_t

third_party/WebKit/Source/platform/graphics/CompositorElementId.h

 // 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.
 
 #ifndef CompositorElementId_h
 #define CompositorElementId_h
 
 #include "cc/trees/element_id.h"
 #include "platform/PlatformExport.h"
 #include "platform/wtf/HashFunctions.h"
 #include "platform/wtf/HashSet.h"
 #include "platform/wtf/HashTraits.h"
 
 namespace blink {
 
+// Do not add more of these without adjusting the bitfield arithmetic in

[suzyh_UTC10 (ex-contributor), 2017/05/05 04:27:31]

(If we don't remove this enum class) Can we enforce this with a test, assertion
or presubmit check?

+// CreateCompositorElementId.
 enum class CompositorSubElementId {

[pdr., 2017/05/05 03:46:10]

Do you know what this is really needed for? I think we can remove it entirely
and save some complexity.

[chrishtr, 2017/05/05 05:34:52]

Thought about this some. Perhaps the reason for these enum bits is that
it was a way to express different compositor gestures/animations for
the same DOM Element, under the design constraint that all cc::ElementId
objects are unique. The uniqueness is probably required by the way
there is a cc::ElementId->cc::Layer map
(LayerTreeHost::element_layers_map_).

We can refactor this away I think for SPv2.

[ajuma, 2017/05/05 13:46:42]

Yeah, my recollection is that these were added because a single
CompositedLayerMapping (corresponding to a single DOM Element via the owning
PaintLayer) can own multiple GraphicsLayers that each need an element id and
these ids need to be distinct.

-  kPrimary,
-  kScroll,
-  kViewport,
-  kLinkHighlight
+  kPrimary = 0,
+  kScroll = 1,
+  kViewport = 2,
+  kLinkHighlight = 3
 };
 
 using CompositorElementId = cc::ElementId;
+static const int kInvalidElementId = 0;

[suzyh_UTC10 (ex-contributor), 2017/05/05 04:27:31]

Why is this redeclared instead of using the ElementId constant already defined?
Indeed, why is CompositorElementId a separate type from ElementId?

Not being comfortable with this code, my best guess is that CompositorElementId
has some additional constraint that makes it a different conceptual type from
ElementId (something to do with the magic 4s around the place?). If that's the
case, then I'd suggest defining CompositorElementId as a separate typedef of the
relevant integer type (that may or may not be the same size), rather than a
typedef of ElementId directly, since that implies more of an equivalence
relationship than may be present.

[chrishtr, 2017/05/05 05:34:52]

Fixed.
I think it's good that cc::Element is the same as CompositorElementId,
as the latter is just a way of plumbing an external type into Blink.
I don't think the change under my patch is any worse really than the
old code, is it?

Also, see above for a guessed response to pdr's comment.

 
 CompositorElementId PLATFORM_EXPORT
 CreateCompositorElementId(int dom_node_id, CompositorSubElementId);
 
-// Note cc::ElementId has a hash function already implemented via
-// ElementIdHash::operator(). However for consistency's sake we choose to use
-// Blink's hash functions with Blink specific data structures.
-struct CompositorElementIdHash {
-  static unsigned GetHash(const CompositorElementId& p) {
-    return WTF::HashInts(p.primaryId, p.secondaryId);
-  }
-  static bool Equal(const CompositorElementId& a,
-                    const CompositorElementId& b) {
-    return a.primaryId == b.primaryId && a.secondaryId == b.secondaryId;
-  }
-  static const bool safe_to_compare_to_empty_or_deleted = true;
-};
+int PLATFORM_EXPORT DomNodeIdFromCompositorElementId(CompositorElementId);
 
-struct CompositorElementIdHashTraits
-    : WTF::GenericHashTraits<CompositorElementId> {
-  static CompositorElementId EmptyValue() { return CompositorElementId(); }
-  static void ConstructDeletedValue(CompositorElementId& slot, bool) {
-    slot = CompositorElementId(-1, -1);
-  }
-  static bool IsDeletedValue(CompositorElementId value) {
-    return value == CompositorElementId(-1, -1);
-  }
-};
-
-using CompositorElementIdSet = HashSet<CompositorElementId,
-                                       CompositorElementIdHash,
-                                       CompositorElementIdHashTraits>;
+using CompositorElementIdSet = HashSet<CompositorElementId>;
 
 }  // namespace blink
 
 #endif  // CompositorElementId_h