From 1af9612edfdd9cd7f7d4aecd002f26ee74c0b2aa Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Tue, 23 Jan 2024 09:41:44 -0600 Subject: [PATCH] fix: ElectronBrowserContext::PartitionKey comparisons (#41055) * fix: ElectronBrowserContext::PartitionKey comparisons Use c++20 default comparisons to simplify + fix PartitionKey sorting: - The equality operator is broken. `PartitionKey{"foo", false}` is both equal, to and less than, `PartitionKey{"foo", true}` - For some keys, the same session can be retrieved via both `fromPath()` and `fromPartition()`. This use case was discussed and removed from the original PR after code review said "always returning different sessions feels lower maintenance." The current behavior is a bug that comes from the comparison operators not checking the keys' types. Xref: https://github.com/electron/electron/pull/36728/commits/3f1aea9af91e17b2605eb8a5837bbb81888d76da#r1099745359 Xref: https://chromium.googlesource.com/chromium/src/+/main/styleguide/c++/c++-features.md#Default-comparisons-allowed * fixup! fix: ElectronBrowserContext::PartitionKey comparisons --- shell/browser/electron_browser_context.h | 44 +++++++----------------- 1 file changed, 13 insertions(+), 31 deletions(-) diff --git a/shell/browser/electron_browser_context.h b/shell/browser/electron_browser_context.h index f6c89910c3b..5d27e11025e 100644 --- a/shell/browser/electron_browser_context.h +++ b/shell/browser/electron_browser_context.h @@ -9,6 +9,7 @@ #include #include #include +#include #include #include @@ -80,41 +81,22 @@ class ElectronBrowserContext : public content::BrowserContext { // partition_id => browser_context struct PartitionKey { - enum class KeyType { Partition, FilePath }; - std::string location; - bool in_memory; - KeyType partition_type; + PartitionKey(const std::string_view partition, bool in_memory) + : type_{Type::Partition}, location_{partition}, in_memory_{in_memory} {} - PartitionKey(const std::string& partition, bool in_memory) - : location(partition), - in_memory(in_memory), - partition_type(KeyType::Partition) {} explicit PartitionKey(const base::FilePath& file_path) - : location(file_path.AsUTF8Unsafe()), - in_memory(false), - partition_type(KeyType::FilePath) {} + : type_{Type::Path}, + location_{file_path.AsUTF8Unsafe()}, + in_memory_{false} {} - bool operator<(const PartitionKey& other) const { - if (partition_type == KeyType::Partition) { - if (location == other.location) - return in_memory < other.in_memory; - return location < other.location; - } else { - if (location == other.location) - return false; - return location < other.location; - } - } + friend auto operator<=>(const PartitionKey&, const PartitionKey&) = default; - bool operator==(const PartitionKey& other) const { - if (partition_type == KeyType::Partition) { - return (location == other.location) && (in_memory < other.in_memory); - } else { - if (location == other.location) - return true; - return false; - } - } + private: + enum class Type { Partition, Path }; + + Type type_; + std::string location_; + bool in_memory_; }; using BrowserContextMap =