Introducing SavePackageId and SaveItemId as distinct IdType<...>-based types.

content/common/id_type.h

+// Copyright 2015 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 CONTENT_COMMON_ID_TYPE_H_
+#define CONTENT_COMMON_ID_TYPE_H_
+
+#include <stdint.h>
+#include <ostream>
+#include <type_traits>
+
+#include "base/containers/hash_tables.h"
+
+// IdType32<>, IdType64<>, etc. wrap an integer id in a custom, type-safe type.
+//
+// IdType32<Foo> is an alternative to int, for a class Foo with methods like:
+//
+//    int GetId() { return id_; };
+//    static Foo* FromId(int id) { return g_all_foos_by_id[id]; }
+//
+// Such methods are a standard means of safely referring to objects across
+// thread and process boundaries.  But if a nearby class Bar also represents
+// its IDs as a bare int, horrific mixups are possible -- one example, of many,
+// is http://crrev.com/365437.  IdType<> offers compile-time protection against
+// such mishaps, since IdType32<Foo> is incompatible with IdType32<Bar>, even
+// though both just compile down to an int32_t.
+//
+// Templates in this file:
+//   IdType32<T> / IdTypeU32<T>: Signed / unsigned 32-bit IDs
+//   IdType64<T> / IdTypeU64<T>: Signed / unsigned 64-bit IDs
+//   IdType<>: For when you need a different underlying type or
+//             a default/invalid value other than zero.
+//
+// IdType32<Foo> behaves just like an int32_t in the following aspects:
+// - it can be used as a key in STL containers;
+// - it can be used as an argument to DCHECK_EQ or streamed to LOG(ERROR);
+// - it has the same memory footprint and runtime overhead as int32_t;
+// - it can be copied by memcpy.
+//
+// IdType32<Foo> has the following differences from a bare int32_t:
+// - it forces coercions to go through GetUnsafeValue and FromUnsafeValue;
+// - it restricts the set of available operations (i.e. no multiplication);
+// - it ensures initialization to zero and allows checking against
+//   default-initialized values via is_null and/or is_valid methods.
+
+namespace content {
+
+template <typename TypeMarker, typename WrappedType, WrappedType kInvalidValue>
+class IdType {
+ public:
+  IdType() : value_(kInvalidValue) {}
+  bool is_valid() const { return value_ != kInvalidValue; }
+  bool is_null() const { return value_ == kInvalidValue; }

[dcheng, 2016/01/07 22:07:28]

Bikeshedding... seems a bit odd that the first is named "is valid", and the
second is named "is null".

In practice, I think providing is_null() is good enough. !is_null() is the exact
same number of characters as is_valid(), and doesn't leave someone wondering if
null might be valid, or there's a third possible state that's not valid and not
null.

[ncarter (slow), 2016/01/07 22:49:42]

Continuing the bikeshed: If we go to only one getter, I think is_valid() (or
is_invalid()) is the better one to keep, since sometimes "null" actually would
mean "has a value of MSG_ROUTING_NONE / -2" whereas "null" to me suggests a zero
value?

[dcheng, 2016/01/07 22:50:37]

Either way is fine. Just that having two different spellings for the same thing
feels funky.

[Łukasz Anforowicz, 2016/01/07 23:12:43]

Done.  Thanks for chiming in.  I don't actually remember why I have is_valid
above (I am guessing that I started with is_valid and then realized that
SurfaceId has is_null and then ended up with 2 without thinking about it much). 
I agree that having only is_null is better.

[Łukasz Anforowicz, 2016/01/07 23:12:43]

I'd prefer "is_null", because this is the name of an existing SurfaceId::is_null
method.  Having the same name will make it easier to "reimplement" SurfaceId on
top of IdType in the future.

+
+  static IdType FromUnsafeValue(WrappedType value) { return IdType(value); }
+  WrappedType GetUnsafeValue() const { return value_; }
+
+  IdType(const IdType& other) = default;
+  IdType& operator=(const IdType& other) = default;
+
+  bool operator==(const IdType& other) const { return value_ == other.value_; }
+  bool operator!=(const IdType& other) const { return value_ != other.value_; }
+  bool operator<(const IdType& other) const { return value_ < other.value_; }
+
+ protected:
+  explicit IdType(WrappedType val) : value_(val) {}
+
+ private:
+  // In theory WrappedType could be any type that supports ==, <, <<, std::hash,
+  // etc., but to make things simpler (both for users and for maintainers) we
+  // explicitly restrict the design space to integers.  This means the users
+  // can safely assume that IdType is relatively small and cheap to copy
+  // and the maintainers don't have to worry about WrappedType being a complex
+  // type (i.e. std::string or std::pair or a move-only type).
+  using IntegralWrappedType =
+      typename std::enable_if<std::is_integral<WrappedType>::value,
+                              WrappedType>::type;
+  IntegralWrappedType value_;
+};
+
+// Type aliases for convenience:
+template <typename TypeMarker>
+using IdType32 = IdType<TypeMarker, int32_t, 0>;
+template <typename TypeMarker>
+using IdTypeU32 = IdType<TypeMarker, uint32_t, 0>;
+template <typename TypeMarker>
+using IdType64 = IdType<TypeMarker, int64_t, 0>;
+template <typename TypeMarker>
+using IdTypeU64 = IdType<TypeMarker, uint64_t, 0>;
+
+template <typename TypeMarker, typename WrappedType, WrappedType kInvalidValue>
+std::ostream& operator<<(
+    std::ostream& stream,
+    const IdType<TypeMarker, WrappedType, kInvalidValue>& id) {
+  return stream << id.GetUnsafeValue();
+}
+
+}  // namespace content
+
+namespace BASE_HASH_NAMESPACE {
+
+template <typename TypeMarker, typename WrappedType, WrappedType kInvalidValue>
+struct hash<content::IdType<TypeMarker, WrappedType, kInvalidValue>> {
+  using argument_type = content::IdType<TypeMarker, WrappedType, kInvalidValue>;
+  using result_type = std::size_t;
+  result_type operator()(const argument_type& id) const {
+    return BASE_HASH_NAMESPACE::hash<WrappedType>()(id.GetUnsafeValue());
+  }
+};
+
+}  // namespace BASE_HASH_NAMESPACE
+
+// If defined(COMPILER_MSVC) then BASE_HASH_NAMESPACE == std.
+// In this case we need to avoid defininig std::hash<...> second time
+// (it has already been defined above).
+#if !defined(COMPILER_MSVC)
+
+namespace std {
+
+template <typename TypeMarker, typename WrappedType, WrappedType kInvalidValue>
+struct hash<content::IdType<TypeMarker, WrappedType, kInvalidValue>> {
+  using argument_type = content::IdType<TypeMarker, WrappedType, kInvalidValue>;
+  using result_type = std::size_t;
+  result_type operator()(const argument_type& id) const {
+    return std::hash<WrappedType>()(id.GetUnsafeValue());
+  }
+};
+
+}  // namespace std;
+
+#endif  // !defined(COMPILER_MSVC)
+
+#endif  // CONTENT_COMMON_ID_TYPE_H_