Add ability to safely retrieve ElementIdentifiers by name.

This CL contains a bunch of quality-of-life and security improvements:
- Can now DCHECK_EQ/NE on ElementIdentifier and ElementContext
- ElementIdentifier::FromRawValue() is now much safer
- Can get the name for an ElementIdentifier and translate safely back
  to ElementIdentifier (identifier must be actively used first)
- ElementTracker[Views] now allows getting all elements with a given ID
  from any context

Implementation details:

The first time the name or raw value of an identifier is used, or when
an element with an identifier that is not in use starts being tracked,
the identifier is added to a master list of known identifiers.

An ElementIdentifier can only be retrieved for a name or raw value if
it has been registered as a known identifier. That way, it is not
possible to convert an arbitrary value to an ElementIdentifier via a
debug API.

This approach does mean that if an ElementIdentifier exists but its
associated name or raw value has not been accessed and no element has
been created with that value, the identifier cannot be retrieved by
name. In the future, we may create ways to register known identifiers
or identifier names, but for now we don't need it.

The "retrieve all elements" methods on ElementTracker and
ElementTrackerViews are implemented in the same manner as
context-specific retrieval methods.

Tests are added for all new functionality.

Change-Id: I061d6d87c2456ade0bd8d561056f319373e35c50
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3265908
Reviewed-by: Yuheng Huang <yuhengh@chromium.org>
Commit-Queue: Dana Fried <dfried@chromium.org>
Cr-Commit-Position: refs/heads/main@{#940026}

ui/base/BUILD.gn

@@ -865,6 +865,7 @@ test("ui_base_unittests") {
     "class_property_unittest.cc",
     "clipboard/file_info_unittest.cc",
     "ime/utf_offset_unittest.cc",
+    "interaction/element_identifier_unittest.cc",
     "interaction/element_tracker_unittest.cc",
     "interaction/interaction_sequence_unittest.cc",
     "l10n/l10n_util_unittest.cc",

ui/base/interaction/README.md

@@ -86,12 +86,14 @@ class method, there is a single-line declaration available, as shown in these
 examples:
 ``` cpp
 // This declares a module-local identifier. The anonymous namespace is optional
-// (though recommended) as kModuleLocalIdentifier will also be marked 'static'.
+// (though recommended) as kModuleLocalIdentifier will also be marked 'static'
+// and the identifier's name will contain the file and line.
 namespace {
 DEFINE_LOCAL_ELEMENT_IDENTIFIER_VALUE(kModuleLocalIdentifier);
 }
 
-// This declares a method-local identifier.
+// This declares a method-local identifier. Again, the use of the local
+// identifier will cause the generated name to include file and line number.
 /* static */ ui::ElementIdentifier MyClass::GetIdentifier() {
   DEFINE_LOCAL_ELEMENT_IDENTIFIER_VALUE(kMethodLocalIdentifier);
   return kMethodLocalIdentifier;

ui/base/interaction/element_identifier.cc

@@ -4,18 +4,83 @@
 
 #include "ui/base/interaction/element_identifier.h"
 
+#include "base/containers/contains.h"
+#include "base/no_destructor.h"
+
 namespace ui {
 
+std::string ElementIdentifier::GetName() const {
+  if (!handle_)
+    return std::string();
+  RegisterKnownIdentifier(*this);
+  return handle_->name;
+}
+
+intptr_t ElementIdentifier::GetRawValue() const {
+  if (!handle_)
+    return 0;
+  RegisterKnownIdentifier(*this);
+  return reinterpret_cast<intptr_t>(handle_);
+}
+
+// static
+ElementIdentifier ElementIdentifier::FromRawValue(intptr_t value) {
+  if (!value)
+    return ElementIdentifier();
+  const auto* impl =
+      reinterpret_cast<const internal::ElementIdentifierImpl*>(value);
+  CHECK(base::Contains(GetKnownIdentifiers(), impl));
+  return ElementIdentifier(impl);
+}
+
+// static
+ElementIdentifier ElementIdentifier::FromName(const char* name) {
+  for (const auto* impl : GetKnownIdentifiers()) {
+    if (!strcmp(impl->name, name))
+      return ElementIdentifier(impl);
+  }
+  return ElementIdentifier();
+}
+
+// static
+void ElementIdentifier::RegisterKnownIdentifier(
+    ElementIdentifier element_identifier) {
+  CHECK(element_identifier);
+
+#if DCHECK_IS_ON()
+  // Enforce uniqueness in DCHECK builds.
+  const ElementIdentifier existing = FromName(element_identifier.handle_->name);
+  DCHECK(!existing || existing == element_identifier);
+#endif
+
+  GetKnownIdentifiers().insert(element_identifier.handle_);
+}
+
+// static
+ElementIdentifier::KnownIdentifiers& ElementIdentifier::GetKnownIdentifiers() {
+  static base::NoDestructor<KnownIdentifiers> known_identifiers;
+  return *known_identifiers.get();
+}
+
 void PrintTo(ElementIdentifier element_identifier, std::ostream* os) {
-  const internal::ElementIdentifierImpl* impl =
-      reinterpret_cast<const internal::ElementIdentifierImpl*>(
-          element_identifier.raw_value());
-  *os << "ElementIdentifier " << impl << " [" << (impl ? impl->name : "")
-      << "]";
+  *os << "ElementIdentifier " << element_identifier.GetRawValue() << " ["
+      << element_identifier.GetName() << "]";
 }
 
 void PrintTo(ElementContext element_context, std::ostream* os) {
   *os << "ElementContext " << static_cast<const void*>(element_context);
 }
 
+extern std::ostream& operator<<(std::ostream& os,
+                                ElementIdentifier element_identifier) {
+  PrintTo(element_identifier, &os);
+  return os;
+}
+
+extern std::ostream& operator<<(std::ostream& os,
+                                ElementContext element_context) {
+  PrintTo(element_context, &os);
+  return os;
+}
+
 }  // namespace ui

ui/base/interaction/element_identifier.h

@@ -8,6 +8,7 @@
 #include <stdint.h>
 
 #include <ostream>
+#include <set>
 
 #include "base/component_export.h"
 #include "base/containers/contains.h"
@@ -19,15 +20,18 @@
 //
 // Unique identifier constants must be both declared and defined. To create a
 // publicly-visible identifier, declare a new unique value in your .h file,
-// with:
+// with the following; the string name of the identifier will be the same as
+// the identifier's C++ identifier (in this case, kMyIdentifierName):
 //
 //   DECLARE_ELEMENT_IDENTIFIER_VALUE(kMyIdentifierName);
 //
 // In the corresponding .cc file, make sure it is defined:
 //
 //   DEFINE_ELEMENT_IDENTIFIER_VALUE(kMyIdentifierName);
 //
-// If you want to add an identifier as a class member, use the following:
+// If you want to add an identifier as a class member, use the following; the
+// string name of the identifier will be in the form
+// "MyClass::kMyIdentifierName":
 //
 //   class MyClass {
 //     DECLARE_CLASS_ELEMENT_IDENTIFIER_VALUE(MyClass, kMyIdentifierName);
@@ -38,7 +42,9 @@
 //   DEFINE_CLASS_ELEMENT_IDENTIFIER_VALUE(MyClass, kMyIdentifierValue);
 //
 // If you want to create an identifier local to a .cc file or to a method, you
-// can instead use the following all-in-one declaration:
+// can instead use the following all-in-one declaration. Note that this is only
+// really useful in tests and that the resulting identifier name is mangled
+// with the file and line number:
 //
 //   DEFINE_LOCAL_ELEMENT_IDENTIFIER_VALUE(kMyIdentifierName);
 //
@@ -92,6 +98,8 @@ struct ElementIdentifierImpl {
 
 }  // namespace internal
 
+class ElementTracker;
+
 // Holds a globally-unqiue, value-typed identifier from a set of identifiers
 // which can be declared in any static scope using
 // DECLARE_UNIQUE_ELEMENT_VALUE().
@@ -127,24 +135,45 @@ class COMPONENT_EXPORT(UI_BASE) ElementIdentifier final {
     return handle_ < other.handle_;
   }
 
-  // Included for interoperability with PropertyHandler. Only works because this
-  // class contains a single pointer, has a trivial destructor, and has no
-  // virtual methods.
-  intptr_t raw_value() const { return reinterpret_cast<intptr_t>(handle_); }
-
-  // Included for interoperability with PropertyHandler. Can be used when
-  // overriding PropertyHandler::AfterPropertyChange() to recover the old
-  // identifier value.
-  static ElementIdentifier FromRawValue(intptr_t value) {
-    return ElementIdentifier(
-        reinterpret_cast<const internal::ElementIdentifierImpl*>(value));
-  }
+  // Retrieves the element name, or the empty string if none.
+  std::string GetName() const;
+
+  // Retrieve a known ElementIdentifer by name. An ElementIdentifier is *known*
+  // if a TrackedElement has been created with the id, or if the value of the
+  // identifier has been serialized using GetRawValue() or GetName().
+  static ElementIdentifier FromName(const char* name);
+
+  // Included for interoperability with PropertyHandler. Retrieves an element
+  // identifier from the result of calling GetRawValue(). The `value` passed in
+  // MUST either have been generated by calling GetRawValue() or be zero (this
+  // is strictly enforced even in release builds).
+  static ElementIdentifier FromRawValue(intptr_t value);
 
  private:
+  using KnownIdentifiers = std::set<const internal::ElementIdentifierImpl*>;
+
+  friend class ClassPropertyCaster<ElementIdentifier>;
+  friend class ElementTracker;
+  friend class ElementIdentifierTest;
+  friend class ElementTrackerIdentifierTest;
+  friend COMPONENT_EXPORT(UI_BASE) void PrintTo(
+      ElementIdentifier element_identifier,
+      std::ostream* os);
+
+  // Included for interoperability with PropertyHandler.
+  intptr_t GetRawValue() const;
+
+  // Registers a non-null identifier as known. Has no effect if the element is
+  // already registered.
+  static void RegisterKnownIdentifier(ElementIdentifier element_dentifier);
+
+  // Returns the singleton set of known identifiers.
+  static KnownIdentifiers& GetKnownIdentifiers();
+
   // The value of the identifier. Because all non-null values point to static
   // ElementIdentifierImpl objects this can be treated as a value from a set of
   // unique, opaque handles.
-  const void* handle_ = nullptr;
+  const internal::ElementIdentifierImpl* handle_ = nullptr;
 };
 
 // The context of an element is unique to the top-level, primary window that the
@@ -204,11 +233,19 @@ extern void PrintTo(ElementIdentifier element_identifier, std::ostream* os);
 COMPONENT_EXPORT(UI_BASE)
 extern void PrintTo(ElementContext element_context, std::ostream* os);
 
+COMPONENT_EXPORT(UI_BASE)
+extern std::ostream& operator<<(std::ostream& os,
+                                ElementIdentifier element_identifier);
+
+COMPONENT_EXPORT(UI_BASE)
+extern std::ostream& operator<<(std::ostream& os,
+                                ElementContext element_context);
+
 // Required for interoperability with PropertyHandler.
 template <>
 class ClassPropertyCaster<ui::ElementIdentifier> {
  public:
-  static int64_t ToInt64(ui::ElementIdentifier x) { return x.raw_value(); }
+  static int64_t ToInt64(ui::ElementIdentifier x) { return x.GetRawValue(); }
   static ui::ElementIdentifier FromInt64(int64_t x) {
     return ui::ElementIdentifier::FromRawValue(x);
   }
@@ -248,12 +285,18 @@ class ClassPropertyCaster<ui::ElementIdentifier> {
   constexpr ui::ElementIdentifier ClassName::IdentifierName
 
 // Declaring local identifiers in functions, class methods, or local to a .cc
-// file:
+// file (often used in tests). File and line are included to guarantee that the
+// text of the name generated is unique, though that makes the exact text
+// harder to predict.
+
+#define LOCAL_ELEMENT_IDENTIFIER_NAME(File, Line, Name) \
+  File "::" #Line "::" #Name
 
 // Use this code to declare a local identifier in a function body.
-#define DEFINE_LOCAL_ELEMENT_IDENTIFIER_VALUE(IdentifierName) \
-  static constexpr ui::internal::ElementIdentifierImpl        \
-      IdentifierName##Provider{#IdentifierName};              \
+#define DEFINE_LOCAL_ELEMENT_IDENTIFIER_VALUE(IdentifierName)                 \
+  static constexpr ui::internal::ElementIdentifierImpl                        \
+      IdentifierName##Provider{                                               \
+          LOCAL_ELEMENT_IDENTIFIER_NAME(__FILE__, __LINE__, IdentifierName)}; \
   constexpr ui::ElementIdentifier IdentifierName(&IdentifierName##Provider)
 
 #endif  // UI_BASE_INTERACTION_ELEMENT_IDENTIFIER_H_

ui/base/interaction/element_identifier_unittest.cc

@@ -0,0 +1,43 @@
+// Copyright 2021 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 "ui/base/interaction/element_identifier.h"
+
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace ui {
+
+namespace {
+
+// Use DECLARE/DEFINE_ELEMENT instead of DEFINE_LOCAL_ELEMENT so that this
+// ElementIdentifier's name is predictable.
+DECLARE_ELEMENT_IDENTIFIER_VALUE(kTestElementIdentifier);
+DEFINE_ELEMENT_IDENTIFIER_VALUE(kTestElementIdentifier);
+const char* const kTestElementIdentifierName = "kTestElementIdentifier";
+
+}  // namespace
+
+class ElementIdentifierTest : public testing::Test {
+ public:
+  void SetUp() override { ElementIdentifier::GetKnownIdentifiers().clear(); }
+
+ protected:
+  static intptr_t GetRawValue(ElementIdentifier id) { return id.GetRawValue(); }
+};
+
+TEST_F(ElementIdentifierTest, FromName) {
+  EXPECT_FALSE(ElementIdentifier::FromName(kTestElementIdentifierName));
+  EXPECT_EQ(kTestElementIdentifierName, kTestElementIdentifier.GetName());
+  EXPECT_EQ(kTestElementIdentifier,
+            ElementIdentifier::FromName(kTestElementIdentifierName));
+}
+
+TEST_F(ElementIdentifierTest, FromRawValue) {
+  EXPECT_FALSE(ElementIdentifier::FromRawValue(0));
+  const intptr_t raw_value = GetRawValue(kTestElementIdentifier);
+  EXPECT_NE(0, raw_value);
+  EXPECT_EQ(kTestElementIdentifier, ElementIdentifier::FromRawValue(raw_value));
+}
+
+}  // namespace ui

ui/base/interaction/element_tracker.cc

@@ -62,7 +62,7 @@ class ElementTracker::ElementData {
   }
 
   void NotifyElementShown(TrackedElement* element) {
-    DCHECK_EQ(identifier().raw_value(), element->identifier().raw_value());
+    DCHECK_EQ(identifier(), element->identifier());
     DCHECK_EQ(static_cast<intptr_t>(context()),
               static_cast<intptr_t>(element->context()));
     const auto it = elements_.insert(elements_.end(), element);
@@ -203,6 +203,16 @@ ElementTracker::ElementList ElementTracker::GetAllMatchingElements(
   return result;
 }
 
+ElementTracker::ElementList ElementTracker::GetAllMatchingElementsInAnyContext(
+    ElementIdentifier id) {
+  ElementList result;
+  for (const auto& pr : element_to_data_lookup_) {
+    if (pr.first->identifier() == id)
+      result.push_back(pr.first);
+  }
+  return result;
+}
+
 bool ElementTracker::IsElementVisible(ElementIdentifier id,
                                       ElementContext context) {
   const auto it = element_data_.find(LookupKey(id, context));
@@ -276,6 +286,9 @@ ElementTracker::ElementData* ElementTracker::GetOrAddElementData(
   const LookupKey key(id, context);
   auto it = element_data_.find(key);
   if (it == element_data_.end()) {
+    // This might be the first time we've referenced this identifier, so make
+    // sure it's registered.
+    ElementIdentifier::RegisterKnownIdentifier(id);
     const auto result = element_data_.emplace(
         key, std::make_unique<ElementData>(this, id, context));
     DCHECK(result.second);

ui/base/interaction/element_tracker.h

@@ -164,6 +164,10 @@ class COMPONENT_EXPORT(UI_BASE) ElementTracker
   ElementList GetAllMatchingElements(ElementIdentifier id,
                                      ElementContext context);
 
+  // Returns all known elements with the given `id`. The context for each can
+  // be retrieved from the TrackedElement itself. No order is guaranteed.
+  ElementList GetAllMatchingElementsInAnyContext(ElementIdentifier id);
+
   // Returns whether an element with identifier `id` in `context` is visible.
   bool IsElementVisible(ElementIdentifier id, ElementContext context);