Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: ElectronBrowserContext::PartitionKey comparisons #41084

Conversation

trop[bot]
Copy link
Contributor

@trop trop bot commented Jan 23, 2024

Backport of #41055

See that PR for details.

Notes: Fixed session.fromPartition() key lookup bug.

trop bot and others added 2 commits January 23, 2024 15:42
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: 3f1aea9#r1099745359

Xref: https://chromium.googlesource.com/chromium/src/+/main/styleguide/c++/c++-features.md#Default-comparisons-allowed

Co-authored-by: Charles Kerr <charles@charleskerr.com>
Co-authored-by: Charles Kerr <charles@charleskerr.com>
@trop trop bot requested a review from ckerr January 23, 2024 15:42
@trop trop bot added 29-x-y backport This is a backport PR semver/patch backwards-compatible bug fixes labels Jan 23, 2024
@ckerr ckerr merged commit 95e3df5 into 29-x-y Jan 23, 2024
20 checks passed
@ckerr ckerr deleted the trop/29-x-y-bp-fix-electronbrowsercontext-partitionkey-comparisons-1706024523684 branch January 23, 2024 23:39
Copy link

release-clerk bot commented Jan 23, 2024

Release Notes Persisted

Fixed session.fromPartition() key lookup bug.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
29-x-y backport This is a backport PR semver/patch backwards-compatible bug fixes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant