Skip to content

Use embeddedOrigin instead of topLevelOrigin in setPermission#469

Merged
miketaylr merged 1 commit intow3c:mainfrom
jalonthomas:use_embedded_origin
Oct 3, 2025
Merged

Use embeddedOrigin instead of topLevelOrigin in setPermission#469
miketaylr merged 1 commit intow3c:mainfrom
jalonthomas:use_embedded_origin

Conversation

@jalonthomas
Copy link
Copy Markdown
Contributor

@jalonthomas jalonthomas commented Oct 1, 2025

We previously added topLevelOrigin to setPermission to make it possible to set storage-access and top-level-storage-access permissions (#468). However, for all other permissions, 'origin' is interpreted as the top-level origin. To align with that, 'origin' in the setPermission command should correspond to the top-level (embedding) origin. This change enforces that fact and makes the second origin parameter the embedded (requesting) origin instead. We also need to change the key generation algorithm to reflect this swap of parameters. This way 'origin' has the same meaning regardless of the permission, and we can still use the method to properly set storage-access and top-level-storage-access permissions.


Preview | Diff

@jalonthomas jalonthomas marked this pull request as ready for review October 1, 2025 15:42
Copy link
Copy Markdown
Member

@miketaylr miketaylr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks

@miketaylr miketaylr merged commit 054e127 into w3c:main Oct 3, 2025
2 checks passed
github-actions bot added a commit that referenced this pull request Oct 3, 2025
SHA: 054e127
Reason: push, by miketaylr

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
github-actions bot added a commit to miketaylr/permissions that referenced this pull request Oct 6, 2025
SHA: 054e127
Reason: push, by miketaylr

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
annevk pushed a commit to privacycg/storage-access that referenced this pull request Oct 18, 2025
philwo pushed a commit to philwo/chromium that referenced this pull request Oct 30, 2025
The specification says to use the current settings object's origin and
top-level origin, as of w3c/permissions#468 (the
parameter was renamed in w3c/permissions#469).

Note that this change should be a no-op for all permission types
*except* for `storage-access`, since `storage-access` is the only
permission type whose "permission key" is not the context's top-level
origin. This is why only the storage-access-api WPTs are affected.

(This CL fixes a bug where PermissionOverrides used the wrong origin for
non-storage-access permissions. This bug was hidden by
https://crsrc.org/c/content/browser/devtools/protocol/browser_handler.cc;drc=3e7f5adf9807d7eeb6fc72fa58e9fc4c3451f407;l=437
until now, since chromedriver did not supply the embeddedOrigin until
this CL.)

Skipping flake verification since this looks like an existing bug (maybe
https://crbug.com/40096995).

Bug: 384959114
Validate-Test-Flakiness: skip
Change-Id: I07b59ca281bc4a4a56a37fc313b4d606233c8a60
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7093216
Reviewed-by: Alex N. Jose <alexnj@chromium.org>
Auto-Submit: Chris Fredrickson <cfredric@chromium.org>
Commit-Queue: Chris Fredrickson <cfredric@chromium.org>
Reviewed-by: Elias Klim <elklm@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1538255}
T3-M4 pushed a commit to bayandin/chromedriver that referenced this pull request Oct 31, 2025
The specification says to use the current settings object's origin and
top-level origin, as of w3c/permissions#468 (the
parameter was renamed in w3c/permissions#469).

Note that this change should be a no-op for all permission types
*except* for `storage-access`, since `storage-access` is the only
permission type whose "permission key" is not the context's top-level
origin. This is why only the storage-access-api WPTs are affected.

(This CL fixes a bug where PermissionOverrides used the wrong origin for
non-storage-access permissions. This bug was hidden by
https://crsrc.org/c/content/browser/devtools/protocol/browser_handler.cc;drc=3e7f5adf9807d7eeb6fc72fa58e9fc4c3451f407;l=437
until now, since chromedriver did not supply the embeddedOrigin until
this CL.)

Skipping flake verification since this looks like an existing bug (maybe
https://crbug.com/40096995).

Bug: 384959114
Validate-Test-Flakiness: skip
Change-Id: I07b59ca281bc4a4a56a37fc313b4d606233c8a60
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7093216
Reviewed-by: Alex N. Jose <alexnj@chromium.org>
Auto-Submit: Chris Fredrickson <cfredric@chromium.org>
Commit-Queue: Chris Fredrickson <cfredric@chromium.org>
Reviewed-by: Elias Klim <elklm@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1538255}
philwo pushed a commit to philwo/chromium that referenced this pull request Oct 31, 2025
…ame's URL"

This reverts commit 694c3f3.

Reason for revert: https://ci.chromium.org/ui/p/chromium/builders/ci/linux-blink-msan-rel/2563/overview starts to fail on linux-blink-msan-rel (https://ci.chromium.org/ui/b/8699543305082222881)

Original change's description:
> Update chromedriver's SetPermission command to use current frame's URL
>
> The specification says to use the current settings object's origin and
> top-level origin, as of w3c/permissions#468 (the
> parameter was renamed in w3c/permissions#469).
>
> Note that this change should be a no-op for all permission types
> *except* for `storage-access`, since `storage-access` is the only
> permission type whose "permission key" is not the context's top-level
> origin. This is why only the storage-access-api WPTs are affected.
>
> (This CL fixes a bug where PermissionOverrides used the wrong origin for
> non-storage-access permissions. This bug was hidden by
> https://crsrc.org/c/content/browser/devtools/protocol/browser_handler.cc;drc=3e7f5adf9807d7eeb6fc72fa58e9fc4c3451f407;l=437
> until now, since chromedriver did not supply the embeddedOrigin until
> this CL.)
>
> Skipping flake verification since this looks like an existing bug (maybe
> https://crbug.com/40096995).
>
> Bug: 384959114
> Validate-Test-Flakiness: skip
> Change-Id: I07b59ca281bc4a4a56a37fc313b4d606233c8a60
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7093216
> Reviewed-by: Alex N. Jose <alexnj@chromium.org>
> Auto-Submit: Chris Fredrickson <cfredric@chromium.org>
> Commit-Queue: Chris Fredrickson <cfredric@chromium.org>
> Reviewed-by: Elias Klim <elklm@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#1538255}

Bug: 384959114, 414530752
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 384959114
Change-Id: I0d28ccd5c49aec302d3b84db6027cf0a7d1dcb68
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7104484
Commit-Queue: Taiyo Mizuhashi <taiyo@chromium.org>
Owners-Override: Taiyo Mizuhashi <taiyo@chromium.org>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/heads/main@{#1538397}
T3-M4 pushed a commit to bayandin/chromedriver that referenced this pull request Oct 31, 2025
…ame's URL"

This reverts commit 6d49173.

Reason for revert: https://ci.chromium.org/ui/p/chromium/builders/ci/linux-blink-msan-rel/2563/overview starts to fail on linux-blink-msan-rel (https://ci.chromium.org/ui/b/8699543305082222881)

Original change's description:
> Update chromedriver's SetPermission command to use current frame's URL
>
> The specification says to use the current settings object's origin and
> top-level origin, as of w3c/permissions#468 (the
> parameter was renamed in w3c/permissions#469).
>
> Note that this change should be a no-op for all permission types
> *except* for `storage-access`, since `storage-access` is the only
> permission type whose "permission key" is not the context's top-level
> origin. This is why only the storage-access-api WPTs are affected.
>
> (This CL fixes a bug where PermissionOverrides used the wrong origin for
> non-storage-access permissions. This bug was hidden by
> https://crsrc.org/c/content/browser/devtools/protocol/browser_handler.cc;drc=3e7f5adf9807d7eeb6fc72fa58e9fc4c3451f407;l=437
> until now, since chromedriver did not supply the embeddedOrigin until
> this CL.)
>
> Skipping flake verification since this looks like an existing bug (maybe
> https://crbug.com/40096995).
>
> Bug: 384959114
> Validate-Test-Flakiness: skip
> Change-Id: I07b59ca281bc4a4a56a37fc313b4d606233c8a60
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7093216
> Reviewed-by: Alex N. Jose <alexnj@chromium.org>
> Auto-Submit: Chris Fredrickson <cfredric@chromium.org>
> Commit-Queue: Chris Fredrickson <cfredric@chromium.org>
> Reviewed-by: Elias Klim <elklm@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#1538255}

Bug: 384959114, 414530752
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 384959114
Change-Id: I0d28ccd5c49aec302d3b84db6027cf0a7d1dcb68
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7104484
Commit-Queue: Taiyo Mizuhashi <taiyo@chromium.org>
Owners-Override: Taiyo Mizuhashi <taiyo@chromium.org>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/heads/main@{#1538397}
aarongable pushed a commit to chromium/chromium that referenced this pull request Nov 3, 2025
…ame's URL"

This is a reland of commit 694c3f3

This version is a bit more careful with TestExpectations, and doesn't
remove entries that might still timeout on slower bots (e.g.
msan/asan/etc).

Original change's description:
> Update chromedriver's SetPermission command to use current frame's URL
>
> The specification says to use the current settings object's origin and
> top-level origin, as of w3c/permissions#468 (the
> parameter was renamed in w3c/permissions#469).
>
> Note that this change should be a no-op for all permission types
> *except* for `storage-access`, since `storage-access` is the only
> permission type whose "permission key" is not the context's top-level
> origin. This is why only the storage-access-api WPTs are affected.
>
> (This CL fixes a bug where PermissionOverrides used the wrong origin for
> non-storage-access permissions. This bug was hidden by
> https://crsrc.org/c/content/browser/devtools/protocol/browser_handler.cc;drc=3e7f5adf9807d7eeb6fc72fa58e9fc4c3451f407;l=437
> until now, since chromedriver did not supply the embeddedOrigin until
> this CL.)
>
> Skipping flake verification since this looks like an existing bug (maybe
> https://crbug.com/40096995).
>
> Bug: 384959114
> Validate-Test-Flakiness: skip
> Change-Id: I07b59ca281bc4a4a56a37fc313b4d606233c8a60
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7093216
> Reviewed-by: Alex N. Jose <alexnj@chromium.org>
> Auto-Submit: Chris Fredrickson <cfredric@chromium.org>
> Commit-Queue: Chris Fredrickson <cfredric@chromium.org>
> Reviewed-by: Elias Klim <elklm@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#1538255}

Bug: 384959114
Change-Id: Ibe4d28ad5c6359ea0ae43fb00831a4b7824dd89f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7106801
Reviewed-by: Alex N. Jose <alexnj@chromium.org>
Commit-Queue: Chris Fredrickson <cfredric@chromium.org>
Auto-Submit: Chris Fredrickson <cfredric@chromium.org>
Reviewed-by: Elias Klim <elklm@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1539450}
T3-M4 pushed a commit to bayandin/chromedriver that referenced this pull request Nov 4, 2025
…ame's URL"

This is a reland of commit 6d49173

This version is a bit more careful with TestExpectations, and doesn't
remove entries that might still timeout on slower bots (e.g.
msan/asan/etc).

Original change's description:
> Update chromedriver's SetPermission command to use current frame's URL
>
> The specification says to use the current settings object's origin and
> top-level origin, as of w3c/permissions#468 (the
> parameter was renamed in w3c/permissions#469).
>
> Note that this change should be a no-op for all permission types
> *except* for `storage-access`, since `storage-access` is the only
> permission type whose "permission key" is not the context's top-level
> origin. This is why only the storage-access-api WPTs are affected.
>
> (This CL fixes a bug where PermissionOverrides used the wrong origin for
> non-storage-access permissions. This bug was hidden by
> https://crsrc.org/c/content/browser/devtools/protocol/browser_handler.cc;drc=3e7f5adf9807d7eeb6fc72fa58e9fc4c3451f407;l=437
> until now, since chromedriver did not supply the embeddedOrigin until
> this CL.)
>
> Skipping flake verification since this looks like an existing bug (maybe
> https://crbug.com/40096995).
>
> Bug: 384959114
> Validate-Test-Flakiness: skip
> Change-Id: I07b59ca281bc4a4a56a37fc313b4d606233c8a60
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7093216
> Reviewed-by: Alex N. Jose <alexnj@chromium.org>
> Auto-Submit: Chris Fredrickson <cfredric@chromium.org>
> Commit-Queue: Chris Fredrickson <cfredric@chromium.org>
> Reviewed-by: Elias Klim <elklm@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#1538255}

Bug: 384959114
Change-Id: Ibe4d28ad5c6359ea0ae43fb00831a4b7824dd89f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7106801
Reviewed-by: Alex N. Jose <alexnj@chromium.org>
Commit-Queue: Chris Fredrickson <cfredric@chromium.org>
Auto-Submit: Chris Fredrickson <cfredric@chromium.org>
Reviewed-by: Elias Klim <elklm@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1539450}
jmajnert pushed a commit to jmajnert/chromium that referenced this pull request Nov 6, 2025
The specification says to use the current settings object's origin and
top-level origin, as of w3c/permissions#468 (the
parameter was renamed in w3c/permissions#469).

Note that this change should be a no-op for all permission types
*except* for `storage-access`, since `storage-access` is the only
permission type whose "permission key" is not the context's top-level
origin. This is why only the storage-access-api WPTs are affected.

(This CL fixes a bug where PermissionOverrides used the wrong origin for
non-storage-access permissions. This bug was hidden by
https://crsrc.org/c/content/browser/devtools/protocol/browser_handler.cc;drc=3e7f5adf9807d7eeb6fc72fa58e9fc4c3451f407;l=437
until now, since chromedriver did not supply the embeddedOrigin until
this CL.)

Skipping flake verification since this looks like an existing bug (maybe
https://crbug.com/40096995).

Bug: 384959114
Validate-Test-Flakiness: skip
Change-Id: I07b59ca281bc4a4a56a37fc313b4d606233c8a60
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7093216
Reviewed-by: Alex N. Jose <alexnj@chromium.org>
Auto-Submit: Chris Fredrickson <cfredric@chromium.org>
Commit-Queue: Chris Fredrickson <cfredric@chromium.org>
Reviewed-by: Elias Klim <elklm@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1538255}
jmajnert pushed a commit to jmajnert/chromium that referenced this pull request Nov 6, 2025
…ame's URL"

This reverts commit 694c3f3.

Reason for revert: https://ci.chromium.org/ui/p/chromium/builders/ci/linux-blink-msan-rel/2563/overview starts to fail on linux-blink-msan-rel (https://ci.chromium.org/ui/b/8699543305082222881)

Original change's description:
> Update chromedriver's SetPermission command to use current frame's URL
>
> The specification says to use the current settings object's origin and
> top-level origin, as of w3c/permissions#468 (the
> parameter was renamed in w3c/permissions#469).
>
> Note that this change should be a no-op for all permission types
> *except* for `storage-access`, since `storage-access` is the only
> permission type whose "permission key" is not the context's top-level
> origin. This is why only the storage-access-api WPTs are affected.
>
> (This CL fixes a bug where PermissionOverrides used the wrong origin for
> non-storage-access permissions. This bug was hidden by
> https://crsrc.org/c/content/browser/devtools/protocol/browser_handler.cc;drc=3e7f5adf9807d7eeb6fc72fa58e9fc4c3451f407;l=437
> until now, since chromedriver did not supply the embeddedOrigin until
> this CL.)
>
> Skipping flake verification since this looks like an existing bug (maybe
> https://crbug.com/40096995).
>
> Bug: 384959114
> Validate-Test-Flakiness: skip
> Change-Id: I07b59ca281bc4a4a56a37fc313b4d606233c8a60
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7093216
> Reviewed-by: Alex N. Jose <alexnj@chromium.org>
> Auto-Submit: Chris Fredrickson <cfredric@chromium.org>
> Commit-Queue: Chris Fredrickson <cfredric@chromium.org>
> Reviewed-by: Elias Klim <elklm@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#1538255}

Bug: 384959114, 414530752
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 384959114
Change-Id: I0d28ccd5c49aec302d3b84db6027cf0a7d1dcb68
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7104484
Commit-Queue: Taiyo Mizuhashi <taiyo@chromium.org>
Owners-Override: Taiyo Mizuhashi <taiyo@chromium.org>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/heads/main@{#1538397}
jmajnert pushed a commit to jmajnert/chromium that referenced this pull request Nov 6, 2025
…ame's URL"

This is a reland of commit 694c3f3

This version is a bit more careful with TestExpectations, and doesn't
remove entries that might still timeout on slower bots (e.g.
msan/asan/etc).

Original change's description:
> Update chromedriver's SetPermission command to use current frame's URL
>
> The specification says to use the current settings object's origin and
> top-level origin, as of w3c/permissions#468 (the
> parameter was renamed in w3c/permissions#469).
>
> Note that this change should be a no-op for all permission types
> *except* for `storage-access`, since `storage-access` is the only
> permission type whose "permission key" is not the context's top-level
> origin. This is why only the storage-access-api WPTs are affected.
>
> (This CL fixes a bug where PermissionOverrides used the wrong origin for
> non-storage-access permissions. This bug was hidden by
> https://crsrc.org/c/content/browser/devtools/protocol/browser_handler.cc;drc=3e7f5adf9807d7eeb6fc72fa58e9fc4c3451f407;l=437
> until now, since chromedriver did not supply the embeddedOrigin until
> this CL.)
>
> Skipping flake verification since this looks like an existing bug (maybe
> https://crbug.com/40096995).
>
> Bug: 384959114
> Validate-Test-Flakiness: skip
> Change-Id: I07b59ca281bc4a4a56a37fc313b4d606233c8a60
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7093216
> Reviewed-by: Alex N. Jose <alexnj@chromium.org>
> Auto-Submit: Chris Fredrickson <cfredric@chromium.org>
> Commit-Queue: Chris Fredrickson <cfredric@chromium.org>
> Reviewed-by: Elias Klim <elklm@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#1538255}

Bug: 384959114
Change-Id: Ibe4d28ad5c6359ea0ae43fb00831a4b7824dd89f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7106801
Reviewed-by: Alex N. Jose <alexnj@chromium.org>
Commit-Queue: Chris Fredrickson <cfredric@chromium.org>
Auto-Submit: Chris Fredrickson <cfredric@chromium.org>
Reviewed-by: Elias Klim <elklm@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1539450}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants