[PM] Allow RenderProcessHostId to accept either 0 or -1 as null values

Also augments util::IdType to allow starting generators at different
values.

R=fdoray

Bug: 1136350
Change-Id: I48673d90e754371892f25021607d2986db8f0bce
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2461597
Commit-Queue: Joe Mason <joenotcharles@chromium.org>
Reviewed-by: François Doray <fdoray@chromium.org>
Cr-Commit-Position: refs/heads/master@{#816689}

base/util/type_safety/id_type.h

@@ -47,7 +47,10 @@ namespace util {
 // - it restricts the set of available operations (i.e. no multiplication);
 // - it default-constructs to a null value and allows checking against the null
 //   value via is_null method.
-template <typename TypeMarker, typename WrappedType, WrappedType kInvalidValue>
+template <typename TypeMarker,
+          typename WrappedType,
+          WrappedType kInvalidValue,
+          WrappedType kFirstGeneratedId = kInvalidValue + 1>
 class IdType : public StrongAlias<TypeMarker, WrappedType> {
  public:
   static_assert(
@@ -55,6 +58,15 @@ class IdType : public StrongAlias<TypeMarker, WrappedType> {
       "If signed, the invalid value should be negative or equal to zero to "
       "avoid overflow issues.");
 
+  static_assert(kFirstGeneratedId != kInvalidValue,
+                "The first generated ID cannot be invalid.");
+
+  static_assert(std::is_unsigned<WrappedType>::value ||
+                    kFirstGeneratedId > kInvalidValue,
+                "If signed, the first generated ID must be greater than the "
+                "invalid value so that the monotonically increasing "
+                "GenerateNextId method will never return the invalid value.");
+
   using StrongAlias<TypeMarker, WrappedType>::StrongAlias;
 
   // This class can be used to generate unique IdTypes. It keeps an internal
@@ -71,7 +83,7 @@ class IdType : public StrongAlias<TypeMarker, WrappedType> {
     Generator& operator=(const Generator&) = delete;
 
    private:
-    WrappedType next_id_ = kInvalidValue + 1;
+    WrappedType next_id_ = kFirstGeneratedId;
   };
 
   // Default-construct in the null state.

base/util/type_safety/id_type_unittest.cc

@@ -52,6 +52,14 @@ TEST(IdType, GeneratorWithBigUnsignedInvalidValue) {
   }
 }
 
+TEST(IdType, GeneratorWithDifferentStartingValue) {
+  using TestId = IdType<class TestIdTag, int, -1, 1>;
+
+  TestId::Generator test_id_generator;
+  for (int i = 1; i < 10; i++)
+    EXPECT_EQ(test_id_generator.GenerateNextId(), TestId::FromUnsafeValue(i));
+}
+
 TEST(IdType, EnsureConstexpr) {
   using TestId = IdType32<class TestTag>;
 

components/performance_manager/BUILD.gn

@@ -231,6 +231,7 @@ source_set("unit_tests") {
     "performance_manager_tab_helper_unittest.cc",
     "performance_manager_unittest.cc",
     "registered_objects_unittest.cc",
+    "render_process_host_id_unittest.cc",
     "v8_memory/v8_context_tracker_helpers_unittest.cc",
     "v8_memory/v8_context_tracker_internal_unittest.cc",
     "v8_memory/v8_detailed_memory_unittest.cc",

components/performance_manager/public/render_process_host_id.h

@@ -6,11 +6,33 @@
 #define COMPONENTS_PERFORMANCE_MANAGER_PUBLIC_RENDER_PROCESS_HOST_ID_H_
 
 #include "base/util/type_safety/id_type.h"
+#include "content/public/common/child_process_host.h"
 
 namespace performance_manager {
 
+using RenderProcessHostIdBase =
+    util::IdType<class RenderProcessHostIdTag,
+                 int32_t,
+                 content::ChildProcessHost::kInvalidUniqueID,
+                 1>;
+
 // A strongly typed wrapper for the id returned by RenderProcessHost::GetID().
-using RenderProcessHostId = util::IdType32<class RenderProcessHostIdTag>;
+//
+// This uses ChildProcessHost::kInvalidUniqueId (-1) as the default invalid id,
+// but also recognizes 0 as an invalid id because there is existing code that
+// uses 0 as an invalid value. It starts generating id's at 1.
+class RenderProcessHostId : public RenderProcessHostIdBase {
+ public:
+  using RenderProcessHostIdBase::RenderProcessHostIdBase;
+
+  // 0 is also an invalid value.
+  constexpr bool is_null() const {
+    return RenderProcessHostIdBase::is_null() || this->value() == 0;
+  }
+
+  // Override operator bool() to call the overridden is_null().
+  constexpr explicit operator bool() const { return !is_null(); }
+};
 
 }  // namespace performance_manager
 

components/performance_manager/render_process_host_id_unittest.cc

+// Copyright 2019 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/performance_manager/public/render_process_host_id.h"
+
+#include "content/public/common/child_process_host.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace performance_manager {
+
+TEST(RenderProcessHostIdTest, InvalidValues) {
+  RenderProcessHostId default_id;
+  EXPECT_TRUE(default_id.is_null());
+  EXPECT_FALSE(default_id);
+
+  RenderProcessHostId invalid_id(content::ChildProcessHost::kInvalidUniqueID);
+  EXPECT_TRUE(invalid_id.is_null());
+  EXPECT_FALSE(invalid_id);
+
+  RenderProcessHostId zero_id(0);
+  EXPECT_TRUE(zero_id.is_null());
+  EXPECT_FALSE(zero_id);
+
+  EXPECT_EQ(default_id, invalid_id);
+  EXPECT_NE(default_id, zero_id);
+
+  RenderProcessHostId valid_id(1);
+  EXPECT_FALSE(valid_id.is_null());
+  EXPECT_TRUE(valid_id);
+}
+
+TEST(RenderProcessHostIdTest, Generator) {
+  RenderProcessHostId::Generator generator;
+  EXPECT_EQ(generator.GenerateNextId(), RenderProcessHostId(1));
+  EXPECT_EQ(generator.GenerateNextId(), RenderProcessHostId(2));
+}
+
+}  // namespace performance_manager