ElementIdentifier: restore most constexpr behavior
When DECLARE_EXPORTED_ELEMENT_IDENTIFIER_VALUE was created to support Ash user education, the macros were changed making most ElementIdentifiers no longer constexpr. Now with the exception of explicitly exported identifiers, they are once again [inline] constexpr, and all methods on the object are also constexpr except operator <. This does not solve the specific use case listed in the bug, but will address many others and potentially improve performance. We will continue looking for a good way to solve the bug below without introducing gotchas and errors in component builds. Bug: 333028921 Change-Id: I4752dbcdbc8fc6562a29db63584165fc20ecf3af Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5445346 Commit-Queue: Dana Fried <dfried@chromium.org> Reviewed-by: Jan Keitel <jkeitel@google.com> Cr-Commit-Position: refs/heads/main@{#1286050}
This commit is contained in:
Dana Fried
committed by
Chromium LUCI CQ
Chromium LUCI CQ
parent
d9ac8dd553
commit
7cc931f8c6
ash
components/user_education/common
ui/base/interaction
@ -9,32 +9,35 @@
|
||||
namespace ash {
|
||||
|
||||
// Please keep this list alphabetized.
|
||||
DEFINE_ELEMENT_IDENTIFIER_VALUE(kAppListBubbleViewElementId);
|
||||
DEFINE_ELEMENT_IDENTIFIER_VALUE(kAssistantDialogPlateElementId);
|
||||
DEFINE_ELEMENT_IDENTIFIER_VALUE(kBluetoothFeatureTileToggleElementId);
|
||||
DEFINE_ELEMENT_IDENTIFIER_VALUE(kCalendarViewElementId);
|
||||
DEFINE_ELEMENT_IDENTIFIER_VALUE(kEnterpriseManagedView);
|
||||
DEFINE_ELEMENT_IDENTIFIER_VALUE(kExploreAppElementId);
|
||||
DEFINE_ELEMENT_IDENTIFIER_VALUE(kHoldingSpaceTrayElementId);
|
||||
DEFINE_ELEMENT_IDENTIFIER_VALUE(kHomeButtonElementId);
|
||||
DEFINE_ELEMENT_IDENTIFIER_VALUE(kLoginUserViewElementId);
|
||||
DEFINE_ELEMENT_IDENTIFIER_VALUE(kOverviewDeskBarElementId);
|
||||
DEFINE_ELEMENT_IDENTIFIER_VALUE(kOverviewDeskBarNewDeskButtonElementId);
|
||||
DEFINE_ELEMENT_IDENTIFIER_VALUE(kPickerElementId);
|
||||
DEFINE_ELEMENT_IDENTIFIER_VALUE(kPickerSearchFieldTextfieldElementId);
|
||||
DEFINE_ELEMENT_IDENTIFIER_VALUE(kPickerSearchResultsPageElementId);
|
||||
DEFINE_ELEMENT_IDENTIFIER_VALUE(kPickerSearchResultsEmojiItemElementId);
|
||||
DEFINE_ELEMENT_IDENTIFIER_VALUE(kPickerSearchResultsListItemElementId);
|
||||
DEFINE_ELEMENT_IDENTIFIER_VALUE(
|
||||
DEFINE_EXPORTED_ELEMENT_IDENTIFIER_VALUE(kAppListBubbleViewElementId);
|
||||
DEFINE_EXPORTED_ELEMENT_IDENTIFIER_VALUE(kAssistantDialogPlateElementId);
|
||||
DEFINE_EXPORTED_ELEMENT_IDENTIFIER_VALUE(kBluetoothFeatureTileToggleElementId);
|
||||
DEFINE_EXPORTED_ELEMENT_IDENTIFIER_VALUE(kCalendarViewElementId);
|
||||
DEFINE_EXPORTED_ELEMENT_IDENTIFIER_VALUE(kEnterpriseManagedView);
|
||||
DEFINE_EXPORTED_ELEMENT_IDENTIFIER_VALUE(kExploreAppElementId);
|
||||
DEFINE_EXPORTED_ELEMENT_IDENTIFIER_VALUE(kHoldingSpaceTrayElementId);
|
||||
DEFINE_EXPORTED_ELEMENT_IDENTIFIER_VALUE(kHomeButtonElementId);
|
||||
DEFINE_EXPORTED_ELEMENT_IDENTIFIER_VALUE(kLoginUserViewElementId);
|
||||
DEFINE_EXPORTED_ELEMENT_IDENTIFIER_VALUE(kOverviewDeskBarElementId);
|
||||
DEFINE_EXPORTED_ELEMENT_IDENTIFIER_VALUE(
|
||||
kOverviewDeskBarNewDeskButtonElementId);
|
||||
DEFINE_EXPORTED_ELEMENT_IDENTIFIER_VALUE(kPickerElementId);
|
||||
DEFINE_EXPORTED_ELEMENT_IDENTIFIER_VALUE(kPickerSearchFieldTextfieldElementId);
|
||||
DEFINE_EXPORTED_ELEMENT_IDENTIFIER_VALUE(kPickerSearchResultsPageElementId);
|
||||
DEFINE_EXPORTED_ELEMENT_IDENTIFIER_VALUE(
|
||||
kPickerSearchResultsEmojiItemElementId);
|
||||
DEFINE_EXPORTED_ELEMENT_IDENTIFIER_VALUE(kPickerSearchResultsListItemElementId);
|
||||
DEFINE_EXPORTED_ELEMENT_IDENTIFIER_VALUE(
|
||||
kQuickSettingsAudioDetailedViewAudioSettingsButtonElementId);
|
||||
DEFINE_ELEMENT_IDENTIFIER_VALUE(kQuickSettingsAudioDetailedViewButtonElementId);
|
||||
DEFINE_ELEMENT_IDENTIFIER_VALUE(kQuickSettingsSettingsButtonElementId);
|
||||
DEFINE_ELEMENT_IDENTIFIER_VALUE(kQuickSettingsViewElementId);
|
||||
DEFINE_ELEMENT_IDENTIFIER_VALUE(kSearchBoxViewElementId);
|
||||
DEFINE_ELEMENT_IDENTIFIER_VALUE(kSettingsAppElementId);
|
||||
DEFINE_ELEMENT_IDENTIFIER_VALUE(kShelfViewElementId);
|
||||
DEFINE_ELEMENT_IDENTIFIER_VALUE(kUnifiedSystemTrayElementId);
|
||||
DEFINE_ELEMENT_IDENTIFIER_VALUE(kWelcomeTourDialogElementId);
|
||||
DEFINE_EXPORTED_ELEMENT_IDENTIFIER_VALUE(
|
||||
kQuickSettingsAudioDetailedViewButtonElementId);
|
||||
DEFINE_EXPORTED_ELEMENT_IDENTIFIER_VALUE(kQuickSettingsSettingsButtonElementId);
|
||||
DEFINE_EXPORTED_ELEMENT_IDENTIFIER_VALUE(kQuickSettingsViewElementId);
|
||||
DEFINE_EXPORTED_ELEMENT_IDENTIFIER_VALUE(kSearchBoxViewElementId);
|
||||
DEFINE_EXPORTED_ELEMENT_IDENTIFIER_VALUE(kSettingsAppElementId);
|
||||
DEFINE_EXPORTED_ELEMENT_IDENTIFIER_VALUE(kShelfViewElementId);
|
||||
DEFINE_EXPORTED_ELEMENT_IDENTIFIER_VALUE(kUnifiedSystemTrayElementId);
|
||||
DEFINE_EXPORTED_ELEMENT_IDENTIFIER_VALUE(kWelcomeTourDialogElementId);
|
||||
// Please keep this list alphabetized.
|
||||
|
||||
} // namespace ash
|
||||
|
||||
@ -6,7 +6,7 @@
|
||||
|
||||
namespace user_education {
|
||||
|
||||
DEFINE_CUSTOM_ELEMENT_EVENT_TYPE(kHelpBubbleAnchorBoundsChangedEvent);
|
||||
DEFINE_CUSTOM_ELEMENT_EVENT_TYPE(kHelpBubbleNextButtonClickedEvent);
|
||||
DEFINE_EXPORTED_CUSTOM_ELEMENT_EVENT_TYPE(kHelpBubbleAnchorBoundsChangedEvent);
|
||||
DEFINE_EXPORTED_CUSTOM_ELEMENT_EVENT_TYPE(kHelpBubbleNextButtonClickedEvent);
|
||||
|
||||
} // namespace user_education
|
||||
|
||||
@ -106,7 +106,7 @@ struct ElementIdentifierImpl {
|
||||
|
||||
class ElementTracker;
|
||||
|
||||
// Holds a globally-unqiue, value-typed identifier from a set of identifiers
|
||||
// Holds a globally-unique, value-typed identifier from a set of identifiers
|
||||
// which can be declared in any static scope.
|
||||
//
|
||||
// This type is comparable and supports operator bool and negation, where
|
||||
@ -128,22 +128,19 @@ class COMPONENT_EXPORT(UI_BASE) ElementIdentifier final {
|
||||
|
||||
constexpr bool operator!() const { return !handle_; }
|
||||
|
||||
constexpr bool operator==(const ElementIdentifier& other) const {
|
||||
return handle_ == other.handle_;
|
||||
}
|
||||
constexpr bool operator==(const ElementIdentifier& other) const = default;
|
||||
|
||||
constexpr bool operator!=(const ElementIdentifier& other) const {
|
||||
return handle_ != other.handle_;
|
||||
}
|
||||
|
||||
constexpr bool operator<(const ElementIdentifier& other) const {
|
||||
// TODO(crbug.com/333028921): Operator < cannot be constexpr because memory
|
||||
// order of Impl objects is not strictly known at compile time. Fix this...
|
||||
// somehow? Possibilities include compile-time hashing of identifier string.
|
||||
bool operator<(const ElementIdentifier& other) const {
|
||||
return handle_ < other.handle_;
|
||||
}
|
||||
|
||||
// Retrieves the element name, or the empty string if none.
|
||||
std::string GetName() const;
|
||||
|
||||
// Retrieve a known ElementIdentifer by name. An ElementIdentifier is *known*
|
||||
// Retrieve a known ElementIdentifier 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);
|
||||
@ -261,10 +258,26 @@ class ClassPropertyCaster<ui::ElementIdentifier> {
|
||||
} // namespace ui
|
||||
|
||||
// Declaring identifiers outside a scope:
|
||||
//
|
||||
// Note: if you need to use the identifier outside the current component, use
|
||||
// DECLARE/DEFINE_EXPORTED_... below.
|
||||
|
||||
// Use this code in the .h file to declare a new identifier.
|
||||
#define DECLARE_ELEMENT_IDENTIFIER_VALUE(IdentifierName) \
|
||||
DECLARE_EXPORTED_ELEMENT_IDENTIFIER_VALUE(, IdentifierName)
|
||||
#define DECLARE_ELEMENT_IDENTIFIER_VALUE(IdentifierName) \
|
||||
extern const ui::internal::ElementIdentifierImpl IdentifierName##Provider; \
|
||||
inline constexpr ui::ElementIdentifier IdentifierName( \
|
||||
&IdentifierName##Provider)
|
||||
|
||||
// Use this code in the .cc file to define a new identifier.
|
||||
#define DEFINE_ELEMENT_IDENTIFIER_VALUE(IdentifierName) \
|
||||
const ui::internal::ElementIdentifierImpl IdentifierName##Provider { \
|
||||
#IdentifierName \
|
||||
}
|
||||
|
||||
// Declaring identifiers that can be used in other components:
|
||||
//
|
||||
// Note: unlike other declarations, this identifier will not be constexpr in
|
||||
// most cases.
|
||||
|
||||
// Use this code in the .h file to declare a new exported identifier.
|
||||
#define DECLARE_EXPORTED_ELEMENT_IDENTIFIER_VALUE(ExportName, IdentifierName) \
|
||||
@ -272,11 +285,11 @@ class ClassPropertyCaster<ui::ElementIdentifier> {
|
||||
IdentifierName##Provider; \
|
||||
ExportName extern const ui::ElementIdentifier IdentifierName
|
||||
|
||||
// Use this code in the .cc file to define a new identifier.
|
||||
#define DEFINE_ELEMENT_IDENTIFIER_VALUE(IdentifierName) \
|
||||
constexpr ui::internal::ElementIdentifierImpl IdentifierName##Provider{ \
|
||||
#IdentifierName}; \
|
||||
constexpr ui::ElementIdentifier IdentifierName(&IdentifierName##Provider)
|
||||
// Use this code in the .cc file to define a new exported identifier.
|
||||
#define DEFINE_EXPORTED_ELEMENT_IDENTIFIER_VALUE(IdentifierName) \
|
||||
const ui::internal::ElementIdentifierImpl IdentifierName##Provider{ \
|
||||
#IdentifierName}; \
|
||||
const ui::ElementIdentifier IdentifierName(&IdentifierName##Provider)
|
||||
|
||||
// Declaring identifiers in a class:
|
||||
|
||||
@ -284,7 +297,7 @@ class ClassPropertyCaster<ui::ElementIdentifier> {
|
||||
// identifier that is scoped to your class.
|
||||
#define DECLARE_CLASS_ELEMENT_IDENTIFIER_VALUE(IdentifierName) \
|
||||
static const ui::internal::ElementIdentifierImpl IdentifierName##Provider; \
|
||||
static constexpr ui::ElementIdentifier IdentifierName { \
|
||||
static constexpr ui::ElementIdentifier IdentifierName { \
|
||||
&IdentifierName##Provider \
|
||||
}
|
||||
|
||||
|
||||
@ -4,6 +4,8 @@
|
||||
|
||||
#include "ui/base/interaction/element_identifier.h"
|
||||
|
||||
#include <utility>
|
||||
|
||||
#include "testing/gtest/include/gtest/gtest.h"
|
||||
|
||||
namespace ui {
|
||||
@ -16,16 +18,35 @@ DECLARE_ELEMENT_IDENTIFIER_VALUE(kTestElementIdentifier);
|
||||
DEFINE_ELEMENT_IDENTIFIER_VALUE(kTestElementIdentifier);
|
||||
const char* const kTestElementIdentifierName = "kTestElementIdentifier";
|
||||
|
||||
DEFINE_LOCAL_ELEMENT_IDENTIFIER_VALUE(kTestLocalElementIdentifier);
|
||||
|
||||
consteval bool IdEqual(ElementIdentifier e1, ElementIdentifier e2) {
|
||||
return e1 == e2;
|
||||
}
|
||||
|
||||
} // namespace
|
||||
|
||||
class ElementIdentifierTest : public testing::Test {
|
||||
public:
|
||||
void SetUp() override { ElementIdentifier::GetKnownIdentifiers().clear(); }
|
||||
|
||||
DECLARE_CLASS_ELEMENT_IDENTIFIER_VALUE(kTestClassElementIdentifier);
|
||||
|
||||
protected:
|
||||
static intptr_t GetRawValue(ElementIdentifier id) { return id.GetRawValue(); }
|
||||
};
|
||||
|
||||
DEFINE_CLASS_ELEMENT_IDENTIFIER_VALUE(ElementIdentifierTest,
|
||||
kTestClassElementIdentifier);
|
||||
|
||||
TEST_F(ElementIdentifierTest, Constexpr) {
|
||||
EXPECT_TRUE(IdEqual(kTestElementIdentifier, kTestElementIdentifier));
|
||||
EXPECT_FALSE(
|
||||
IdEqual(kTestClassElementIdentifier, kTestLocalElementIdentifier));
|
||||
// TODO(crbug.com/333028921): Put in compile-time checks for `operator <` once
|
||||
// it is constexpr, perhaps using `base::MakeFlatSet<>()`.
|
||||
}
|
||||
|
||||
TEST_F(ElementIdentifierTest, FromName) {
|
||||
EXPECT_FALSE(ElementIdentifier::FromName(kTestElementIdentifierName));
|
||||
EXPECT_EQ(kTestElementIdentifierName, kTestElementIdentifier.GetName());
|
||||
|
||||
@ -305,13 +305,22 @@ class COMPONENT_EXPORT(UI_BASE) SafeElementReference {
|
||||
// your public header file and the DEFINE in corresponding .cc file. For local
|
||||
// values to be used in tests, use DEFINE_LOCAL_CUSTOM_ELEMENT_EVENT_TYPE()
|
||||
// defined below instead.
|
||||
//
|
||||
// Note: if you need to use the identifier outside the current component, use
|
||||
// DECLARE/DEFINE_EXPORTED_... below.
|
||||
#define DECLARE_CUSTOM_ELEMENT_EVENT_TYPE(EventName) \
|
||||
DECLARE_ELEMENT_IDENTIFIER_VALUE(EventName)
|
||||
#define DECLARE_EXPORTED_CUSTOM_ELEMENT_EVENT_TYPE(ExportName, EventName) \
|
||||
DECLARE_EXPORTED_ELEMENT_IDENTIFIER_VALUE(ExportName, EventName)
|
||||
#define DEFINE_CUSTOM_ELEMENT_EVENT_TYPE(EventName) \
|
||||
DEFINE_ELEMENT_IDENTIFIER_VALUE(EventName)
|
||||
|
||||
// Macros for declaring custom element event types that can be accessed in other
|
||||
// components. Put the DECLARE call in your public header file and the DEFINE
|
||||
// call in the corresponding .cc file.
|
||||
#define DECLARE_EXPORTED_CUSTOM_ELEMENT_EVENT_TYPE(ExportName, EventName) \
|
||||
DECLARE_EXPORTED_ELEMENT_IDENTIFIER_VALUE(ExportName, EventName)
|
||||
#define DEFINE_EXPORTED_CUSTOM_ELEMENT_EVENT_TYPE(EventName) \
|
||||
DEFINE_EXPORTED_ELEMENT_IDENTIFIER_VALUE(EventName)
|
||||
|
||||
// Macros for declaring custom class element event type. Put the DECLARE call in
|
||||
// your .h file in your class declaration, and the DEFINE in the corresponding
|
||||
// .cc file.
|
||||
|
||||
Reference in New Issue
Block a user