Sunggook Chue (Gerrit)

unread,

Feb 7, 2024, 8:06:43 PM2/7/24

to Guido Urdaneta, edgecapab...@microsoft.com, android-web...@chromium.org, asvitkine...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, blundell+...@chromium.org, chfreme...@chromium.org, devtools-re...@chromium.org, devtools...@chromium.org, dominickn+wat...@chromium.org, dullweb...@chromium.org, feature-co...@chromium.org, feature-me...@chromium.org, iclella...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org, michaelpg+wa...@chromium.org, msrame...@chromium.org, permissio...@chromium.org

Attention needed from Guido Urdaneta

Sunggook Chue has uploaded the change for review

Sunggook Chue would like Guido Urdaneta to review this change.

Commit message

Add placeholder of speaker-selection permission.

Incorporating the ‘speaker-selection’ permission from
'https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Permissions-Policy/speaker-selection'
this addition aligns with Chromium’s permission enhancements
(https://source.chromium.org/chromium/chromium/src/+/main:components/permissions/add_new_permission.md)

However, it’s important to note that this feature is not yet implemented in
the chrome://settings/content page. Additionally, the need for a permission
prompt remains uncertain (not edited related code), and use 'test' permission
policy to prevent unintended prompts outside of testing scenarios."

Bug: 40075730

Change-Id: Ia4025021424c2dc317e92bc026e2d3c8077950c7

Change diff

Change information

Files:

M android_webview/browser/aw_permission_manager.cc
M chrome/browser/permissions/permission_manager_factory.cc
M chrome/browser/ui/webui/settings/site_settings_helper.cc
M components/content_settings/core/browser/content_settings_registry.cc
M components/content_settings/core/browser/content_settings_uma_util.cc
M components/content_settings/core/common/content_settings_types.mojom
M components/permissions/BUILD.gn
A components/permissions/contexts/speaker_selection_permission_context.cc
A components/permissions/contexts/speaker_selection_permission_context.h
M components/permissions/permission_util.cc
M content/browser/devtools/protocol/browser_handler.cc
M content/browser/permissions/permission_controller_impl.cc
M content/shell/browser/shell_permission_manager.cc
A media/muxers/mp4_muxer_delegate_fragment.cc
A media/muxers/mp4_muxer_delegate_fragment.h
M third_party/blink/common/permissions/permission_utils.cc
M third_party/blink/public/common/permissions/permission_utils.h
M third_party/blink/public/devtools_protocol/browser_protocol.pdl
M third_party/blink/public/mojom/permissions/permission.mojom
M third_party/blink/public/mojom/permissions_policy/permissions_policy_feature.mojom
M third_party/blink/renderer/core/permissions_policy/feature_policy.dict
M third_party/blink/renderer/core/permissions_policy/permissions_policy_features.json5
M third_party/blink/renderer/modules/permissions/permission_descriptor.idl
M third_party/blink/renderer/modules/permissions/permission_utils.cc
M third_party/blink/renderer/platform/runtime_enabled_features.json5
M third_party/blink/web_tests/http/tests/resources/permissions-helper.js
M tools/metrics/histograms/metadata/histogram_suffixes_list.xml

Change size: L

Delta: 27 files changed, 530 insertions(+), 4 deletions(-)

Open in Gerrit

Related details

Attention is currently required from:

Guido Urdaneta

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Sunggook Chue (Gerrit)

unread,

Feb 7, 2024, 8:06:50 PM2/7/24

to edgecapab...@microsoft.com, android-web...@chromium.org, asvitkine...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, blundell+...@chromium.org, chfreme...@chromium.org, devtools-re...@chromium.org, devtools...@chromium.org, dominickn+wat...@chromium.org, dullweb...@chromium.org, feature-co...@chromium.org, feature-me...@chromium.org, iclella...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org, michaelpg+wa...@chromium.org, msrame...@chromium.org, permissio...@chromium.org, Guido Urdaneta, Tricium, Chromium Metrics Reviews, chromium...@chromium.org, Luna Lu, Peter Beverloo, Rijubrata Bhaumik

Attention needed from Guido Urdaneta

Sunggook Chue added 1 comment

Patchset-level comments

File-level comment, Patchset 1 (Latest):

Sunggook Chue . resolved

This is a placeholder, not enabled yet all.
Thanks fore reviewing!

Open in Gerrit

Related details

Attention is currently required from:

Guido Urdaneta

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Sunggook Chue (Gerrit)

unread,

Feb 7, 2024, 8:07:42 PM2/7/24

to edgecapab...@microsoft.com, android-web...@chromium.org, asvitkine...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, blundell+...@chromium.org, chfreme...@chromium.org, devtools-re...@chromium.org, devtools...@chromium.org, dominickn+wat...@chromium.org, dullweb...@chromium.org, feature-co...@chromium.org, feature-me...@chromium.org, iclella...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org, michaelpg+wa...@chromium.org, msrame...@chromium.org, permissio...@chromium.org

Attention needed from Guido Urdaneta

Sunggook Chue uploaded new patchset

Sunggook Chue uploaded patch set #2 to this change.

Open in Gerrit

Related details

Attention is currently required from:

Guido Urdaneta

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Guido Urdaneta (Gerrit)

unread,

Feb 8, 2024, 5:05:33 AM2/8/24

to Sunggook Chue, edgecapab...@microsoft.com, android-web...@chromium.org, asvitkine...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, blundell+...@chromium.org, chfreme...@chromium.org, devtools-re...@chromium.org, devtools...@chromium.org, dominickn+wat...@chromium.org, dullweb...@chromium.org, feature-co...@chromium.org, feature-me...@chromium.org, iclella...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org, michaelpg+wa...@chromium.org, msrame...@chromium.org, permissio...@chromium.org, Chromium LUCI CQ, Tricium, Chromium Metrics Reviews, chromium...@chromium.org, Luna Lu, Peter Beverloo, Rijubrata Bhaumik

Attention needed from Sunggook Chue

Guido Urdaneta added 5 comments

Patchset-level comments

File-level comment, Patchset 2 (Latest):

Guido Urdaneta . resolved

The general structure looks good (modulo removal of unrelated files), but obviously it needs to be reviewed by the proper owners.
Once it's ready, please send me a follow-up CL where you make use of the new permission to authorize/block speaker swtiches (made via the setSinkId() API).

File android_webview/browser/aw_permission_manager.cc

Line 349, Patchset 2 (Latest): case PermissionType::SPEAKER - SELECTION:

Guido Urdaneta . unresolved

Should this be `SPEAKER_SELECTION`?
```suggestion
      case PermissionType::SPEAKER_SELECTION:
```

File components/content_settings/core/browser/content_settings_registry.cc

Line 723, Patchset 2 (Latest):

Guido Urdaneta . unresolved

Delete trailing whitespace

File components/permissions/contexts/speaker_selection_permission_context.cc

Line 24, Patchset 2 (Latest): // Avoid recording the setting; it is not really associated with

Guido Urdaneta . unresolved

Speaker selection is not about capture, but about playout.
Note also that the way Chromium works right now is that if you have the microphone permission, you automatically gain an equivalent of this permission (e.g., to be able to switch output to a headset if you change capture to the headset mic)

File media/muxers/mp4_muxer_delegate_fragment.h

Guido Urdaneta . unresolved

It looks like this file, and the corresponding .cc shouldn't be part of this CL.

Open in Gerrit

Related details

Attention is currently required from:

Sunggook Chue

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Sunggook Chue (Gerrit)

unread,

Feb 8, 2024, 6:32:25 PM2/8/24

to edgecapab...@microsoft.com, android-web...@chromium.org, asvitkine...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, blundell+...@chromium.org, chfreme...@chromium.org, devtools-re...@chromium.org, devtools...@chromium.org, dominickn+wat...@chromium.org, dullweb...@chromium.org, feature-co...@chromium.org, feature-me...@chromium.org, iclella...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org, michaelpg+wa...@chromium.org, msrame...@chromium.org, permissio...@chromium.org

Attention needed from Sunggook Chue

Sunggook Chue uploaded new patchset

Sunggook Chue uploaded patch set #3 to this change.

Open in Gerrit

Related details

Attention is currently required from:

Sunggook Chue

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Sunggook Chue (Gerrit)

unread,

Feb 8, 2024, 6:33:03 PM2/8/24

to edgecapab...@microsoft.com, android-web...@chromium.org, asvitkine...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, blundell+...@chromium.org, chfreme...@chromium.org, devtools-re...@chromium.org, devtools...@chromium.org, dominickn+wat...@chromium.org, dullweb...@chromium.org, feature-co...@chromium.org, feature-me...@chromium.org, iclella...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org, michaelpg+wa...@chromium.org, msrame...@chromium.org, permissio...@chromium.org, Chromium LUCI CQ, Guido Urdaneta, Tricium, Chromium Metrics Reviews, chromium...@chromium.org, Luna Lu, Peter Beverloo, Rijubrata Bhaumik

Attention needed from Guido Urdaneta

Sunggook Chue added 5 comments

Patchset-level comments

File-level comment, Patchset 3 (Latest):

Sunggook Chue . resolved

addressed comments, thanks!

File android_webview/browser/aw_permission_manager.cc

Line 349, Patchset 2: case PermissionType::SPEAKER - SELECTION:

Guido Urdaneta . resolved

Should this be `SPEAKER_SELECTION`?
```suggestion
      case PermissionType::SPEAKER_SELECTION:
```

Sunggook Chue

Done

File components/content_settings/core/browser/content_settings_registry.cc

Line 723, Patchset 2:

Guido Urdaneta . resolved

Delete trailing whitespace

Sunggook Chue

Done

File components/permissions/contexts/speaker_selection_permission_context.cc

Line 24, Patchset 2: // Avoid recording the setting; it is not really associated with

Guido Urdaneta . resolved

Speaker selection is not about capture, but about playout.
Note also that the way Chromium works right now is that if you have the microphone permission, you automatically gain an equivalent of this permission (e.g., to be able to switch output to a headset if you change capture to the headset mic)

Sunggook Chue

I will replace this "
  // TODO: Implement this method.
  // The current speaker selection permission comes from the
  // microphone permission at this time. Initially, we can continue
  // to use the microphone permission for top frame, but follow
  // the spec for iframes."
  
What's is your idea how user can get speaker-selection permission in the top frame (when HTTP response header does not grant its permission);

File media/muxers/mp4_muxer_delegate_fragment.h

Guido Urdaneta . resolved

It looks like this file, and the corresponding .cc shouldn't be part of this CL.

Sunggook Chue

sorry,

Open in Gerrit

Related details

Attention is currently required from:

Guido Urdaneta

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Guido Urdaneta (Gerrit)

unread,

Feb 9, 2024, 4:17:53 AM2/9/24

to Sunggook Chue, edgecapab...@microsoft.com, android-web...@chromium.org, asvitkine...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, blundell+...@chromium.org, chfreme...@chromium.org, devtools-re...@chromium.org, devtools...@chromium.org, dominickn+wat...@chromium.org, dullweb...@chromium.org, feature-co...@chromium.org, feature-me...@chromium.org, iclella...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org, michaelpg+wa...@chromium.org, msrame...@chromium.org, permissio...@chromium.org, Chromium LUCI CQ, Tricium, Chromium Metrics Reviews, chromium...@chromium.org, Luna Lu, Peter Beverloo, Rijubrata Bhaumik

Attention needed from Sunggook Chue

Guido Urdaneta added 1 comment

File components/permissions/contexts/speaker_selection_permission_context.cc

Line 24, Patchset 3 (Latest): // TODO: The current speaker selection permission comes from the

Guido Urdaneta . unresolved

reference a bug inside the TODO.
Also, the wording is a bit confusing.
There is no current speaker selection permission (you are implementing it).
What exists is implicit consent via getUserMedia() for the setSinkId() API, which is part of the spec ("Implementations MUST also support implicit consent via the getUserMedia() permission prompt" [1]).
The specific permission will allow the implementation of other parts of the spec.

[1] https://w3c.github.io/mediacapture-output/#privacy-obtaining-consent)

Open in Gerrit

Related details

Attention is currently required from:

Sunggook Chue

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Guido Urdaneta (Gerrit)

unread,

Feb 9, 2024, 4:19:26 AM2/9/24

to Sunggook Chue, edgecapab...@microsoft.com, android-web...@chromium.org, asvitkine...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, blundell+...@chromium.org, chfreme...@chromium.org, devtools-re...@chromium.org, devtools...@chromium.org, dominickn+wat...@chromium.org, dullweb...@chromium.org, feature-co...@chromium.org, feature-me...@chromium.org, iclella...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org, michaelpg+wa...@chromium.org, msrame...@chromium.org, permissio...@chromium.org, Chromium LUCI CQ, Tricium, Chromium Metrics Reviews, chromium...@chromium.org, Luna Lu, Peter Beverloo, Rijubrata Bhaumik

Attention needed from Sunggook Chue

Guido Urdaneta voted and added 1 comment

Votes added by Guido Urdaneta

Code-Review

+1

1 comment

Patchset-level comments

File-level comment, Patchset 3 (Latest):

Guido Urdaneta . resolved

Other than my comment about the TODO, I think this is ready to be sent to owners for review.

satisfied_requirement

unsatisfied_requirement

open

diffy

Sunggook Chue (Gerrit)

unread,

Feb 9, 2024, 3:52:43 PM2/9/24

to Balazs Engedy, Peter Beverloo, Chromium IPC Reviews, Florian Jacky, edgecapab...@microsoft.com, android-web...@chromium.org, asvitkine...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, blundell+...@chromium.org, chfreme...@chromium.org, devtools-re...@chromium.org, devtools...@chromium.org, dominickn+wat...@chromium.org, dullweb...@chromium.org, feature-co...@chromium.org, feature-me...@chromium.org, iclella...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org, michaelpg+wa...@chromium.org, msrame...@chromium.org, permissio...@chromium.org

Attention needed from Balazs Engedy, Chromium IPC Reviews, Florian Jacky and Peter Beverloo

Sunggook Chue has uploaded the change for review

Sunggook Chue would like Balazs Engedy, Peter Beverloo, Chromium IPC Reviews and Florian Jacky to review this change.

Commit message

Add placeholder of speaker-selection permission.

Incorporating the ‘speaker-selection’ permission from
'https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Permissions-Policy/speaker-selection'
this addition aligns with Chromium’s permission enhancements
(https://source.chromium.org/chromium/chromium/src/+/main:components/permissions/add_new_permission.md)

However, it’s important to note that this feature is not yet implemented in
the chrome://settings/content page. Additionally, the need for a permission
prompt remains uncertain (not edited related code), and use 'test' permission
policy to prevent unintended prompts outside of testing scenarios."

Bug: 41492674

Change-Id: Ia4025021424c2dc317e92bc026e2d3c8077950c7

Change diff

Change information

Files:

M android_webview/browser/aw_permission_manager.cc
M chrome/browser/permissions/permission_manager_factory.cc
M chrome/browser/ui/webui/settings/site_settings_helper.cc
M components/content_settings/core/browser/content_settings_registry.cc
M components/content_settings/core/browser/content_settings_uma_util.cc
M components/content_settings/core/common/content_settings_types.mojom
M components/permissions/BUILD.gn
A components/permissions/contexts/speaker_selection_permission_context.cc
A components/permissions/contexts/speaker_selection_permission_context.h
M components/permissions/permission_util.cc
M content/browser/devtools/protocol/browser_handler.cc
M content/browser/permissions/permission_controller_impl.cc
M content/shell/browser/shell_permission_manager.cc

M third_party/blink/common/permissions/permission_utils.cc
M third_party/blink/public/common/permissions/permission_utils.h
M third_party/blink/public/devtools_protocol/browser_protocol.pdl
M third_party/blink/public/mojom/permissions/permission.mojom
M third_party/blink/public/mojom/permissions_policy/permissions_policy_feature.mojom
M third_party/blink/renderer/core/permissions_policy/feature_policy.dict
M third_party/blink/renderer/core/permissions_policy/permissions_policy_features.json5
M third_party/blink/renderer/modules/permissions/permission_descriptor.idl
M third_party/blink/renderer/modules/permissions/permission_utils.cc
M third_party/blink/renderer/platform/runtime_enabled_features.json5
M third_party/blink/web_tests/http/tests/resources/permissions-helper.js
M tools/metrics/histograms/metadata/histogram_suffixes_list.xml

Change size: M

Delta: 25 files changed, 135 insertions(+), 4 deletions(-)

Open in Gerrit

Related details

Attention is currently required from:

Balazs Engedy
Chromium IPC Reviews
Florian Jacky
Peter Beverloo

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

gwsq (Gerrit)

unread,

Feb 9, 2024, 3:55:05 PM2/9/24

to Tom Sepez, edgecapab...@microsoft.com, android-web...@chromium.org, asvitkine...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, blundell+...@chromium.org, chfreme...@chromium.org, devtools-re...@chromium.org, devtools...@chromium.org, dominickn+wat...@chromium.org, dullweb...@chromium.org, feature-co...@chromium.org, feature-me...@chromium.org, iclella...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org, michaelpg+wa...@chromium.org, msrame...@chromium.org, permissio...@chromium.org

Attention needed from Balazs Engedy, Chromium IPC Reviews, Florian Jacky, Peter Beverloo and Tom Sepez

Sunggook Chue has uploaded the change for review

gwsq would like Tom Sepez to review this change authored by Sunggook Chue.

Related details

Tom Sepez

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

gwsq (Gerrit)

unread,

Feb 9, 2024, 3:55:08 PM2/9/24

to edgecapab...@microsoft.com, android-web...@chromium.org, asvitkine...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, blundell+...@chromium.org, chfreme...@chromium.org, devtools-re...@chromium.org, devtools...@chromium.org, dominickn+wat...@chromium.org, dullweb...@chromium.org, feature-co...@chromium.org, feature-me...@chromium.org, iclella...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org, michaelpg+wa...@chromium.org, msrame...@chromium.org, permissio...@chromium.org, Chromium IPC Reviews

Attention needed from Balazs Engedy, Florian Jacky, Peter Beverloo and Tom Sepez

Sunggook Chue has uploaded the change for review

Related details

Balazs Engedy

Florian Jacky
Peter Beverloo
Tom Sepez

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

gwsq (Gerrit)

unread,

Feb 9, 2024, 3:55:20 PM2/9/24

to Sunggook Chue, edgecapab...@microsoft.com, android-web...@chromium.org, asvitkine...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, blundell+...@chromium.org, chfreme...@chromium.org, devtools-re...@chromium.org, devtools...@chromium.org, dominickn+wat...@chromium.org, dullweb...@chromium.org, feature-co...@chromium.org, feature-me...@chromium.org, iclella...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org, michaelpg+wa...@chromium.org, msrame...@chromium.org, permissio...@chromium.org, Chromium IPC Reviews, Tom Sepez, Balazs Engedy, Florian Jacky, Peter Beverloo, Guido Urdaneta, Chromium LUCI CQ, Tricium, Chromium Metrics Reviews, chromium...@chromium.org, Luna Lu, Rijubrata Bhaumik

Attention needed from Balazs Engedy, Florian Jacky, Peter Beverloo and Tom Sepez

Message from gwsq

From googleclient/chrome/chromium_gwsq/ipc/config.gwsq:
IPC: tse...@chromium.org

📎 It looks like you’re making a possibly security-sensitive change! 📎 IPC security review isn’t a rubberstamp, so your friendly security reviewer will need a fair amount of context to review your CL effectively. Please review your CL description and code comments to make sure they provide context for someone unfamiliar with your project/area. Pay special attention to where data comes from and which processes it flows between (and their privilege levels). Feel free to point your security reviewer at design docs, bugs, or other links if you can’t reasonably make a self-contained CL description. (Also see https://cbea.ms/git-commit/).

IPC reviewer(s): tse...@chromium.org

Reviewer source(s):
tse...@chromium.org is from context(googleclient/chrome/chromium_gwsq/ipc/config.gwsq)

Open in Gerrit

Related details

Attention is currently required from:

Balazs Engedy
Florian Jacky
Peter Beverloo
Tom Sepez

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Sunggook Chue (Gerrit)

unread,

Feb 9, 2024, 8:11:21 PM2/9/24

to edgecapab...@microsoft.com, android-web...@chromium.org, asvitkine...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, blundell+...@chromium.org, chfreme...@chromium.org, devtools-re...@chromium.org, devtools...@chromium.org, dominickn+wat...@chromium.org, dullweb...@chromium.org, feature-co...@chromium.org, feature-me...@chromium.org, iclella...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org, michaelpg+wa...@chromium.org, msrame...@chromium.org, permissio...@chromium.org

Attention needed from Balazs Engedy, Florian Jacky, Peter Beverloo and Tom Sepez

Sunggook Chue uploaded new patchset

Sunggook Chue uploaded patch set #4 to this change.

Open in Gerrit

Related details

Attention is currently required from:

Balazs Engedy
Florian Jacky
Peter Beverloo
Tom Sepez

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Sunggook Chue (Gerrit)

unread,

Feb 9, 2024, 8:11:49 PM2/9/24

to edgecapab...@microsoft.com, android-web...@chromium.org, asvitkine...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, blundell+...@chromium.org, chfreme...@chromium.org, devtools-re...@chromium.org, devtools...@chromium.org, dominickn+wat...@chromium.org, dullweb...@chromium.org, feature-co...@chromium.org, feature-me...@chromium.org, iclella...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org, michaelpg+wa...@chromium.org, msrame...@chromium.org, permissio...@chromium.org, Chromium IPC Reviews, Tom Sepez, Balazs Engedy, Florian Jacky, Peter Beverloo, Guido Urdaneta, Chromium LUCI CQ, Tricium, Chromium Metrics Reviews, chromium...@chromium.org, Luna Lu, Rijubrata Bhaumik

Attention needed from Balazs Engedy, Florian Jacky, Peter Beverloo and Tom Sepez

Sunggook Chue added 2 comments

Patchset-level comments

File-level comment, Patchset 4 (Latest):

Sunggook Chue . resolved

updates comment, Thanks.

File components/permissions/contexts/speaker_selection_permission_context.cc

Line 24, Patchset 3: // TODO: The current speaker selection permission comes from the

Guido Urdaneta . resolved

reference a bug inside the TODO.
Also, the wording is a bit confusing.
There is no current speaker selection permission (you are implementing it).
What exists is implicit consent via getUserMedia() for the setSinkId() API, which is part of the spec ("Implementations MUST also support implicit consent via the getUserMedia() permission prompt" [1]).
The specific permission will allow the implementation of other parts of the spec.

[1] https://w3c.github.io/mediacapture-output/#privacy-obtaining-consent)

Sunggook Chue

Thanks for clarification, I've updated comments based on your comment here.

Open in Gerrit

Related details

Attention is currently required from:

Balazs Engedy
Florian Jacky
Peter Beverloo
Tom Sepez

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Guido Urdaneta (Gerrit)

unread,

Feb 12, 2024, 4:35:04 AM2/12/24

to Sunggook Chue, edgecapab...@microsoft.com, android-web...@chromium.org, asvitkine...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, blundell+...@chromium.org, chfreme...@chromium.org, devtools-re...@chromium.org, devtools...@chromium.org, dominickn+wat...@chromium.org, dullweb...@chromium.org, feature-co...@chromium.org, feature-me...@chromium.org, iclella...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org, michaelpg+wa...@chromium.org, msrame...@chromium.org, permissio...@chromium.org, Chromium IPC Reviews, Tom Sepez, Balazs Engedy, Florian Jacky, Peter Beverloo, Chromium LUCI CQ, Tricium, Chromium Metrics Reviews, chromium...@chromium.org, Luna Lu, Rijubrata Bhaumik

Attention needed from Balazs Engedy, Florian Jacky, Peter Beverloo, Sunggook Chue and Tom Sepez

Guido Urdaneta voted and added 1 comment

Votes added by Guido Urdaneta

Code-Review

+1

1 comment

File android_webview/browser/aw_permission_manager.cc

Line 599, Patchset 4 (Latest): case PermissionType::SPEAKER - SELECTION:

Guido Urdaneta . unresolved

Still broken here.

Open in Gerrit

Related details

Attention is currently required from:

Balazs Engedy
Florian Jacky
Peter Beverloo

Sunggook Chue
Tom Sepez

satisfied_requirement

unsatisfied_requirement

open

diffy

Balazs Engedy (Gerrit)

unread,

Feb 12, 2024, 6:02:21 AM2/12/24

to Sunggook Chue, edgecapab...@microsoft.com, android-web...@chromium.org, asvitkine...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, blundell+...@chromium.org, chfreme...@chromium.org, devtools-re...@chromium.org, devtools...@chromium.org, dominickn+wat...@chromium.org, dullweb...@chromium.org, feature-co...@chromium.org, feature-me...@chromium.org, iclella...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org, michaelpg+wa...@chromium.org, msrame...@chromium.org, permissio...@chromium.org, Guido Urdaneta, Chromium IPC Reviews, Tom Sepez, Florian Jacky, Peter Beverloo, Chromium LUCI CQ, Tricium, Chromium Metrics Reviews, chromium...@chromium.org, Luna Lu, Rijubrata Bhaumik

Attention needed from Florian Jacky, Peter Beverloo, Sunggook Chue and Tom Sepez

Balazs Engedy added 1 comment

Patchset-level comments

File-level comment, Patchset 4 (Latest):

Balazs Engedy . resolved

Sunggook, can you please specify which set of files you would like each owner to review? I see that you have added Florian and myself as well, I wonder if he already has context on this change?

Open in Gerrit

Related details

Attention is currently required from:

satisfied_requirement

unsatisfied_requirement

open

diffy

Tom Sepez (Gerrit)

unread,

Feb 12, 2024, 11:35:11 AM2/12/24

to Sunggook Chue, edgecapab...@microsoft.com, android-web...@chromium.org, asvitkine...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, blundell+...@chromium.org, chfreme...@chromium.org, devtools-re...@chromium.org, devtools...@chromium.org, dominickn+wat...@chromium.org, dullweb...@chromium.org, feature-co...@chromium.org, feature-me...@chromium.org, iclella...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org, michaelpg+wa...@chromium.org, msrame...@chromium.org, permissio...@chromium.org, Guido Urdaneta, Chromium IPC Reviews, Balazs Engedy, Florian Jacky, Peter Beverloo, Chromium LUCI CQ, Tricium, Chromium Metrics Reviews, chromium...@chromium.org, Luna Lu, Rijubrata Bhaumik

Attention needed from Peter Beverloo and Sunggook Chue

Tom Sepez voted and added 1 comment

Votes added by Tom Sepez

Code-Review

+1

1 comment

Patchset-level comments

File-level comment, Patchset 4 (Latest):

Tom Sepez . resolved

MOJO LG but it looks like there's a typo preventing this from building.

Open in Gerrit

Related details

Attention is currently required from:

Peter Beverloo
Sunggook Chue

Submit Requirements:

Code-Coverage
Code-Owners

Code-Review

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Sunggook Chue (Gerrit)

unread,

Feb 12, 2024, 5:10:59 PM2/12/24

to edgecapab...@microsoft.com, android-web...@chromium.org, asvitkine...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, blundell+...@chromium.org, chfreme...@chromium.org, devtools-re...@chromium.org, devtools...@chromium.org, dominickn+wat...@chromium.org, dullweb...@chromium.org, feature-co...@chromium.org, feature-me...@chromium.org, iclella...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org, michaelpg+wa...@chromium.org, msrame...@chromium.org, permissio...@chromium.org

Attention needed from Peter Beverloo and Sunggook Chue

Sunggook Chue uploaded new patchset

Sunggook Chue uploaded patch set #5 to this change.

Open in Gerrit

Related details

Attention is currently required from:

Peter Beverloo
Sunggook Chue

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Sunggook Chue (Gerrit)

unread,

Feb 12, 2024, 5:29:55 PM2/12/24

to edgecapab...@microsoft.com, android-web...@chromium.org, asvitkine...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, blundell+...@chromium.org, chfreme...@chromium.org, devtools-re...@chromium.org, devtools...@chromium.org, dominickn+wat...@chromium.org, dullweb...@chromium.org, feature-co...@chromium.org, feature-me...@chromium.org, iclella...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org, michaelpg+wa...@chromium.org, msrame...@chromium.org, permissio...@chromium.org

Attention needed from Peter Beverloo and Sunggook Chue

Sunggook Chue uploaded new patchset

Sunggook Chue uploaded patch set #6 to this change.

Open in Gerrit

Related details

Attention is currently required from:

Peter Beverloo
Sunggook Chue

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Sunggook Chue (Gerrit)

unread,

Feb 12, 2024, 5:41:50 PM2/12/24

to Theodore Olsauskas-Warren, Fredrik Söderquist, Andrey Kosyakov, Sami Kyöstilä, Florian Jacky, edgecapab...@microsoft.com, android-web...@chromium.org, asvitkine...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, blundell+...@chromium.org, chfreme...@chromium.org, devtools-re...@chromium.org, devtools...@chromium.org, dominickn+wat...@chromium.org, dullweb...@chromium.org, feature-co...@chromium.org, feature-me...@chromium.org, iclella...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org, michaelpg+wa...@chromium.org, msrame...@chromium.org, permissio...@chromium.org

Attention needed from Andrey Kosyakov, Fredrik Söderquist, Peter Beverloo, Sami Kyöstilä and Theodore Olsauskas-Warren

Sunggook Chue has uploaded the change for review

Sunggook Chue would like Theodore Olsauskas-Warren, Fredrik Söderquist, Andrey Kosyakov and Sami Kyöstilä to review this change.

Sunggook Chue removed Florian Jacky from reviewers of this change.

Open in Gerrit

Related details

Attention is currently required from:

Andrey Kosyakov
Fredrik Söderquist
Peter Beverloo
Sami Kyöstilä
Theodore Olsauskas-Warren

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Sunggook Chue (Gerrit)

unread,

Feb 12, 2024, 5:41:57 PM2/12/24

to edgecapab...@microsoft.com, android-web...@chromium.org, asvitkine...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, blundell+...@chromium.org, chfreme...@chromium.org, devtools-re...@chromium.org, devtools...@chromium.org, dominickn+wat...@chromium.org, dullweb...@chromium.org, feature-co...@chromium.org, feature-me...@chromium.org, iclella...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org, michaelpg+wa...@chromium.org, msrame...@chromium.org, permissio...@chromium.org, Fredrik Söderquist, Theodore Olsauskas-Warren, Andrey Kosyakov, Sami Kyöstilä, Tom Sepez, Guido Urdaneta, Chromium IPC Reviews, Balazs Engedy, Peter Beverloo, Chromium LUCI CQ, Tricium, Chromium Metrics Reviews, chromium...@chromium.org, Luna Lu, Rijubrata Bhaumik

Attention needed from Andrey Kosyakov, Fredrik Söderquist, Peter Beverloo, Sami Kyöstilä and Theodore Olsauskas-Warren

Sunggook Chue added 2 comments

Patchset-level comments

File-level comment, Patchset 6 (Latest):

Sunggook Chue . resolved

add reviewers:

Andrey: t/b/p/d/browser_protocol.pdl, c/b/d/p/browser_handler.cc
Peter B: a/b/aw_permission_manager.cc, t/m/h/m/histogram_suffixes_list.xml
Theodore: c/b/u/w/s/site_settings_helper.cc
Balazs: c/b/p/permission_controller_* , c/c/c/b/content_settings_registry.*, permission_util.*, third_party/blink/renderer/modules/permissions/ permission_utils.cc

Sami K: t/b/r/p/runtime_enabled_features.json5
Tom S: c/c/c/c/content_settings_types.mojom

File android_webview/browser/aw_permission_manager.cc

Line 599, Patchset 4: case PermissionType::SPEAKER - SELECTION:

Guido Urdaneta . resolved

Still broken here.

Sunggook Chue

Done

Open in Gerrit

Related details

Attention is currently required from:

Andrey Kosyakov
Fredrik Söderquist
Peter Beverloo
Sami Kyöstilä
Theodore Olsauskas-Warren

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Andrey Kosyakov (Gerrit)

unread,

Feb 12, 2024, 5:42:43 PM2/12/24

to Sunggook Chue, edgecapab...@microsoft.com, android-web...@chromium.org, asvitkine...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, blundell+...@chromium.org, chfreme...@chromium.org, devtools-re...@chromium.org, devtools...@chromium.org, dominickn+wat...@chromium.org, dullweb...@chromium.org, feature-co...@chromium.org, feature-me...@chromium.org, iclella...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org, michaelpg+wa...@chromium.org, msrame...@chromium.org, permissio...@chromium.org, Fredrik Söderquist, Theodore Olsauskas-Warren, Sami Kyöstilä, Tom Sepez, Guido Urdaneta, Chromium IPC Reviews, Balazs Engedy, Peter Beverloo, Chromium LUCI CQ, Tricium, Chromium Metrics Reviews, chromium...@chromium.org, Luna Lu, Rijubrata Bhaumik

Attention needed from Fredrik Söderquist, Peter Beverloo, Sami Kyöstilä, Sunggook Chue and Theodore Olsauskas-Warren

Andrey Kosyakov voted and added 1 comment

Votes added by Andrey Kosyakov

Code-Review

+1

1 comment

Patchset-level comments

File-level comment, Patchset 6 (Latest):

Andrey Kosyakov . resolved

devtools/ rslgtm

Open in Gerrit

Related details

Attention is currently required from:

Fredrik Söderquist
Peter Beverloo
Sami Kyöstilä

Sunggook Chue
Theodore Olsauskas-Warren

satisfied_requirement

unsatisfied_requirement

open

diffy

Fredrik Söderquist (Gerrit)

unread,

Feb 13, 2024, 5:00:05 AM2/13/24

to Sunggook Chue, edgecapab...@microsoft.com, android-web...@chromium.org, asvitkine...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, blundell+...@chromium.org, chfreme...@chromium.org, devtools-re...@chromium.org, devtools...@chromium.org, dominickn+wat...@chromium.org, dullweb...@chromium.org, feature-co...@chromium.org, feature-me...@chromium.org, iclella...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org, michaelpg+wa...@chromium.org, msrame...@chromium.org, permissio...@chromium.org, Andrey Kosyakov, Theodore Olsauskas-Warren, Sami Kyöstilä, Tom Sepez, Guido Urdaneta, Chromium IPC Reviews, Balazs Engedy, Peter Beverloo, Chromium LUCI CQ, Tricium, Chromium Metrics Reviews, chromium...@chromium.org, Luna Lu, Rijubrata Bhaumik

Attention needed from Peter Beverloo, Sami Kyöstilä, Sunggook Chue and Theodore Olsauskas-Warren

Fredrik Söderquist added 2 comments

Patchset-level comments

File-level comment, Patchset 6 (Latest):

Fredrik Söderquist . resolved

Does this have an associated intent?

File third_party/blink/renderer/modules/permissions/permission_utils.cc

Line 131, Patchset 6 (Latest): return "speaker_selection";

Fredrik Söderquist . unresolved

It looks like this ought to be "speaker-selection" to match what's in PermissionName - since it's exposed via the `PermissionStatus` IDL interface and there expected to match the enumeration. (I see that the spec is loosey-goosey about this and just has a DOMString, but it seems a UA should at the very least be self-consistent at the very least.)

Open in Gerrit

Related details

Attention is currently required from:

satisfied_requirement

unsatisfied_requirement

open

diffy

Balazs Engedy (Gerrit)

unread,

Feb 13, 2024, 6:10:17 AM2/13/24

to Sunggook Chue, edgecapab...@microsoft.com, android-web...@chromium.org, asvitkine...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, blundell+...@chromium.org, chfreme...@chromium.org, devtools-re...@chromium.org, devtools...@chromium.org, dominickn+wat...@chromium.org, dullweb...@chromium.org, feature-co...@chromium.org, feature-me...@chromium.org, iclella...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org, michaelpg+wa...@chromium.org, msrame...@chromium.org, permissio...@chromium.org, Andrey Kosyakov, Fredrik Söderquist, Theodore Olsauskas-Warren, Sami Kyöstilä, Tom Sepez, Guido Urdaneta, Chromium IPC Reviews, Peter Beverloo, Chromium LUCI CQ, Tricium, Chromium Metrics Reviews, chromium...@chromium.org, Luna Lu, Rijubrata Bhaumik

Attention needed from Fredrik Söderquist, Peter Beverloo, Sami Kyöstilä, Sunggook Chue and Theodore Olsauskas-Warren

Balazs Engedy voted and added 3 comments

Votes added by Balazs Engedy

Code-Review

+1

3 comments

Patchset-level comments

File-level comment, Patchset 6 (Latest):

Balazs Engedy . resolved

permissions and content_settings LGTM % comments below.

File components/permissions/contexts/speaker_selection_permission_context.cc

Line 19, Patchset 6 (Latest):void SpeakerSelectionPermissionContext::UpdateContentSetting(

Balazs Engedy . unresolved

Can you please instead follow suit with the approach in chrome/browser/display_capture/display_capture_permission_context.cc that overrides all methods necessary for not crashing if PermissionController::RequestPermission* is called?

File third_party/blink/renderer/modules/permissions/permission_utils.cc

Line 131, Patchset 6 (Latest): return "speaker_selection";

Fredrik Söderquist . unresolved

It looks like this ought to be "speaker-selection" to match what's in PermissionName - since it's exposed via the `PermissionStatus` IDL interface and there expected to match the enumeration. (I see that the spec is loosey-goosey about this and just has a DOMString, but it seems a UA should at the very least be self-consistent at the very least.)

Balazs Engedy

Indeed, please use dashes for consistency (as pointed out in the TODO above). There are indeed some historical mistakes, the reason they are still there is because they are hard to change due to web compat risk.

Open in Gerrit

Related details

Attention is currently required from:

Fredrik Söderquist

satisfied_requirement

unsatisfied_requirement

open

diffy

Theodore Olsauskas-Warren (Gerrit)

unread,

Feb 13, 2024, 7:42:51 AM2/13/24

to Sunggook Chue, edgecapab...@microsoft.com, android-web...@chromium.org, asvitkine...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, blundell+...@chromium.org, chfreme...@chromium.org, devtools-re...@chromium.org, devtools...@chromium.org, dominickn+wat...@chromium.org, dullweb...@chromium.org, feature-co...@chromium.org, feature-me...@chromium.org, iclella...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org, michaelpg+wa...@chromium.org, msrame...@chromium.org, permissio...@chromium.org, Balazs Engedy, Andrey Kosyakov, Fredrik Söderquist, Sami Kyöstilä, Tom Sepez, Guido Urdaneta, Chromium IPC Reviews, Peter Beverloo, Chromium LUCI CQ, Tricium, Chromium Metrics Reviews, chromium...@chromium.org, Luna Lu, Rijubrata Bhaumik

Attention needed from Fredrik Söderquist, Peter Beverloo, Sami Kyöstilä and Sunggook Chue

Theodore Olsauskas-Warren voted and added 1 comment

Votes added by Theodore Olsauskas-Warren

Code-Review

+1

1 comment

Patchset-level comments

File-level comment, Patchset 6 (Latest):

Theodore Olsauskas-Warren . resolved

chrome/browser/ui/webui/settings/site_settings_helper.cc LGTM

Open in Gerrit

Related details

Attention is currently required from:

Fredrik Söderquist
Peter Beverloo
Sami Kyöstilä
Sunggook Chue

satisfied_requirement

unsatisfied_requirement

open

diffy

Sunggook Chue (Gerrit)

unread,

Feb 13, 2024, 8:32:16 PM2/13/24

to edgecapab...@microsoft.com, android-web...@chromium.org, asvitkine...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, blundell+...@chromium.org, chfreme...@chromium.org, devtools-re...@chromium.org, devtools...@chromium.org, dominickn+wat...@chromium.org, dullweb...@chromium.org, feature-co...@chromium.org, feature-me...@chromium.org, iclella...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org, michaelpg+wa...@chromium.org, msrame...@chromium.org, permissio...@chromium.org

Attention needed from Andrey Kosyakov, Balazs Engedy, Fredrik Söderquist, Guido Urdaneta, Peter Beverloo, Sami Kyöstilä, Sunggook Chue, Theodore Olsauskas-Warren and Tom Sepez

Sunggook Chue uploaded new patchset

Sunggook Chue uploaded patch set #7 to this change.

Following approvals got outdated and were removed:

Code-Review: +1 by Balazs Engedy, +1 by Guido Urdaneta, +1 by Theodore Olsauskas-Warren, +1 by Andrey Kosyakov, +1 by Tom Sepez

Open in Gerrit

Related details

Attention is currently required from:

Andrey Kosyakov
Balazs Engedy
Fredrik Söderquist
Guido Urdaneta

Peter Beverloo
Sami Kyöstilä
Sunggook Chue

Theodore Olsauskas-Warren
Tom Sepez

Submit Requirements:

Code-Coverage
Code-Owners

Code-Review

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Sunggook Chue (Gerrit)

unread,

Feb 14, 2024, 1:39:15 AM2/14/24

to edgecapab...@microsoft.com, android-web...@chromium.org, asvitkine...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, blundell+...@chromium.org, chfreme...@chromium.org, devtools-re...@chromium.org, devtools...@chromium.org, dominickn+wat...@chromium.org, dullweb...@chromium.org, feature-co...@chromium.org, feature-me...@chromium.org, iclella...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org, michaelpg+wa...@chromium.org, msrame...@chromium.org, permissio...@chromium.org

Attention needed from Andrey Kosyakov, Balazs Engedy, Fredrik Söderquist, Guido Urdaneta, Peter Beverloo, Sami Kyöstilä, Sunggook Chue, Theodore Olsauskas-Warren and Tom Sepez

Sunggook Chue uploaded new patchset

Sunggook Chue uploaded patch set #8 to this change.

Open in Gerrit

Related details

Attention is currently required from:

Andrey Kosyakov
Balazs Engedy
Fredrik Söderquist
Guido Urdaneta
Peter Beverloo
Sami Kyöstilä
Sunggook Chue
Theodore Olsauskas-Warren
Tom Sepez

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Sunggook Chue (Gerrit)

unread,

Feb 14, 2024, 1:39:58 AM2/14/24

to edgecapab...@microsoft.com, android-web...@chromium.org, asvitkine...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, blundell+...@chromium.org, chfreme...@chromium.org, devtools-re...@chromium.org, devtools...@chromium.org, dominickn+wat...@chromium.org, dullweb...@chromium.org, feature-co...@chromium.org, feature-me...@chromium.org, iclella...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org, michaelpg+wa...@chromium.org, msrame...@chromium.org, permissio...@chromium.org, Theodore Olsauskas-Warren, Balazs Engedy, Andrey Kosyakov, Fredrik Söderquist, Sami Kyöstilä, Tom Sepez, Guido Urdaneta, Chromium IPC Reviews, Peter Beverloo, Chromium LUCI CQ, Tricium, Chromium Metrics Reviews, chromium...@chromium.org, Luna Lu, Rijubrata Bhaumik

Attention needed from Andrey Kosyakov, Balazs Engedy, Fredrik Söderquist, Guido Urdaneta, Peter Beverloo, Sami Kyöstilä, Theodore Olsauskas-Warren and Tom Sepez

Sunggook Chue added 3 comments

Patchset-level comments

File-level comment, Patchset 7:

Sunggook Chue . resolved

Thanks for review!

File components/permissions/contexts/speaker_selection_permission_context.cc

Line 19, Patchset 6:void SpeakerSelectionPermissionContext::UpdateContentSetting(

Balazs Engedy . resolved

Can you please instead follow suit with the approach in chrome/browser/display_capture/display_capture_permission_context.cc that overrides all methods necessary for not crashing if PermissionController::RequestPermission* is called?

Sunggook Chue

Done

File third_party/blink/renderer/modules/permissions/permission_utils.cc

Line 131, Patchset 6: return "speaker_selection";

Fredrik Söderquist . resolved

It looks like this ought to be "speaker-selection" to match what's in PermissionName - since it's exposed via the `PermissionStatus` IDL interface and there expected to match the enumeration. (I see that the spec is loosey-goosey about this and just has a DOMString, but it seems a UA should at the very least be self-consistent at the very least.)

Balazs Engedy

Indeed, please use dashes for consistency (as pointed out in the TODO above). There are indeed some historical mistakes, the reason they are still there is because they are hard to change due to web compat risk.

Sunggook Chue

Done

Open in Gerrit

Related details

Attention is currently required from:

Andrey Kosyakov
Balazs Engedy
Fredrik Söderquist
Guido Urdaneta
Peter Beverloo
Sami Kyöstilä

Theodore Olsauskas-Warren
Tom Sepez

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Sunggook Chue (Gerrit)

unread,

Feb 14, 2024, 2:26:35 PM2/14/24

to edgecapab...@microsoft.com, android-web...@chromium.org, asvitkine...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, blundell+...@chromium.org, chfreme...@chromium.org, devtools-re...@chromium.org, devtools...@chromium.org, dominickn+wat...@chromium.org, dullweb...@chromium.org, feature-co...@chromium.org, feature-me...@chromium.org, iclella...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org, michaelpg+wa...@chromium.org, msrame...@chromium.org, permissio...@chromium.org, Theodore Olsauskas-Warren, Balazs Engedy, Andrey Kosyakov, Fredrik Söderquist, Sami Kyöstilä, Tom Sepez, Guido Urdaneta, Chromium IPC Reviews, Peter Beverloo, Chromium LUCI CQ, Tricium, Chromium Metrics Reviews, chromium...@chromium.org, Luna Lu, Rijubrata Bhaumik

Attention needed from Andrey Kosyakov, Balazs Engedy, Fredrik Söderquist, Guido Urdaneta, Peter Beverloo, Sami Kyöstilä, Theodore Olsauskas-Warren and Tom Sepez

Sunggook Chue added 1 comment

Patchset-level comments

File-level comment, Patchset 8 (Latest):

Sunggook Chue . resolved

Somehow, all votes are reset, can you take a look this change again?

Open in Gerrit

Related details

Attention is currently required from:

Andrey Kosyakov
Balazs Engedy
Fredrik Söderquist
Guido Urdaneta
Peter Beverloo
Sami Kyöstilä
Theodore Olsauskas-Warren
Tom Sepez

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Balazs Engedy (Gerrit)

unread,

Feb 14, 2024, 5:29:21 PM2/14/24

to Sunggook Chue, edgecapab...@microsoft.com, android-web...@chromium.org, asvitkine...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, blundell+...@chromium.org, chfreme...@chromium.org, devtools-re...@chromium.org, devtools...@chromium.org, dominickn+wat...@chromium.org, dullweb...@chromium.org, feature-co...@chromium.org, feature-me...@chromium.org, iclella...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org, michaelpg+wa...@chromium.org, msrame...@chromium.org, permissio...@chromium.org, Theodore Olsauskas-Warren, Andrey Kosyakov, Fredrik Söderquist, Sami Kyöstilä, Tom Sepez, Guido Urdaneta, Chromium IPC Reviews, Peter Beverloo, Chromium LUCI CQ, Tricium, Chromium Metrics Reviews, chromium...@chromium.org, Luna Lu, Rijubrata Bhaumik

Attention needed from Andrey Kosyakov, Fredrik Söderquist, Guido Urdaneta, Peter Beverloo, Sami Kyöstilä, Sunggook Chue, Theodore Olsauskas-Warren and Tom Sepez

Balazs Engedy voted and added 1 comment

Votes added by Balazs Engedy

Code-Review

+1

1 comment

Patchset-level comments

File-level comment, Patchset 8 (Latest):

Balazs Engedy . resolved

permissions still LGTM

Open in Gerrit

Related details

Attention is currently required from:

Andrey Kosyakov

Fredrik Söderquist
Guido Urdaneta
Peter Beverloo
Sami Kyöstilä

Sunggook Chue
Theodore Olsauskas-Warren
Tom Sepez

satisfied_requirement

unsatisfied_requirement

open

diffy

Andrey Kosyakov (Gerrit)

unread,

Feb 14, 2024, 8:40:00 PM2/14/24

to Sunggook Chue, edgecapab...@microsoft.com, android-web...@chromium.org, asvitkine...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, blundell+...@chromium.org, chfreme...@chromium.org, devtools-re...@chromium.org, devtools...@chromium.org, dominickn+wat...@chromium.org, dullweb...@chromium.org, feature-co...@chromium.org, feature-me...@chromium.org, iclella...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org, michaelpg+wa...@chromium.org, msrame...@chromium.org, permissio...@chromium.org, Balazs Engedy, Theodore Olsauskas-Warren, Fredrik Söderquist, Sami Kyöstilä, Tom Sepez, Guido Urdaneta, Chromium IPC Reviews, Peter Beverloo, Chromium LUCI CQ, Tricium, Chromium Metrics Reviews, chromium...@chromium.org, Luna Lu, Rijubrata Bhaumik

Attention needed from Fredrik Söderquist, Guido Urdaneta, Peter Beverloo, Sami Kyöstilä, Sunggook Chue, Theodore Olsauskas-Warren and Tom Sepez

Andrey Kosyakov voted and added 1 comment

Votes added by Andrey Kosyakov

Code-Review

+1

1 comment

Patchset-level comments

File-level comment, Patchset 8 (Latest):

Andrey Kosyakov . resolved

re-stamping for devtools/

Open in Gerrit

Related details

Attention is currently required from:

Fredrik Söderquist
Guido Urdaneta
Peter Beverloo
Sami Kyöstilä
Sunggook Chue
Theodore Olsauskas-Warren
Tom Sepez

Submit Requirements:

Code-Coverage
Code-Owners

Code-Review

satisfied_requirement

unsatisfied_requirement

open

diffy

Guido Urdaneta (Gerrit)

unread,

Feb 15, 2024, 3:16:15 AM2/15/24

to Sunggook Chue, edgecapab...@microsoft.com, android-web...@chromium.org, asvitkine...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, blundell+...@chromium.org, chfreme...@chromium.org, devtools-re...@chromium.org, devtools...@chromium.org, dominickn+wat...@chromium.org, dullweb...@chromium.org, feature-co...@chromium.org, feature-me...@chromium.org, iclella...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org, michaelpg+wa...@chromium.org, msrame...@chromium.org, permissio...@chromium.org, Andrey Kosyakov, Balazs Engedy, Theodore Olsauskas-Warren, Fredrik Söderquist, Sami Kyöstilä, Tom Sepez, Chromium IPC Reviews, Peter Beverloo, Chromium LUCI CQ, Tricium, Chromium Metrics Reviews, chromium...@chromium.org, Luna Lu, Rijubrata Bhaumik

Attention needed from Fredrik Söderquist, Peter Beverloo, Sami Kyöstilä, Sunggook Chue, Theodore Olsauskas-Warren and Tom Sepez

Guido Urdaneta voted Code-Review+1

Code-Review

+1

Open in Gerrit

Related details

Attention is currently required from:

Fredrik Söderquist

satisfied_requirement

unsatisfied_requirement

open

diffy

Theodore Olsauskas-Warren (Gerrit)

unread,

Feb 15, 2024, 3:45:13 AM2/15/24

to Sunggook Chue, edgecapab...@microsoft.com, android-web...@chromium.org, asvitkine...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, blundell+...@chromium.org, chfreme...@chromium.org, devtools-re...@chromium.org, devtools...@chromium.org, dominickn+wat...@chromium.org, dullweb...@chromium.org, feature-co...@chromium.org, feature-me...@chromium.org, iclella...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org, michaelpg+wa...@chromium.org, msrame...@chromium.org, permissio...@chromium.org, Guido Urdaneta, Andrey Kosyakov, Balazs Engedy, Fredrik Söderquist, Sami Kyöstilä, Tom Sepez, Chromium IPC Reviews, Peter Beverloo, Chromium LUCI CQ, Tricium, Chromium Metrics Reviews, chromium...@chromium.org, Luna Lu, Rijubrata Bhaumik

Attention needed from Fredrik Söderquist, Peter Beverloo, Sami Kyöstilä, Sunggook Chue and Tom Sepez

Theodore Olsauskas-Warren voted Code-Review+1

Code-Review

+1

Open in Gerrit

Related details

Attention is currently required from:

Fredrik Söderquist
Peter Beverloo
Sami Kyöstilä
Sunggook Chue

Tom Sepez

satisfied_requirement

unsatisfied_requirement

open

diffy

Tom Sepez (Gerrit)

unread,

Feb 15, 2024, 1:08:07 PM2/15/24

to Sunggook Chue, edgecapab...@microsoft.com, android-web...@chromium.org, asvitkine...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, blundell+...@chromium.org, chfreme...@chromium.org, devtools-re...@chromium.org, devtools...@chromium.org, dominickn+wat...@chromium.org, dullweb...@chromium.org, feature-co...@chromium.org, feature-me...@chromium.org, iclella...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org, michaelpg+wa...@chromium.org, msrame...@chromium.org, permissio...@chromium.org, Theodore Olsauskas-Warren, Guido Urdaneta, Andrey Kosyakov, Balazs Engedy, Fredrik Söderquist, Sami Kyöstilä, Chromium IPC Reviews, Peter Beverloo, Chromium LUCI CQ, Tricium, Chromium Metrics Reviews, chromium...@chromium.org, Luna Lu, Rijubrata Bhaumik

Attention needed from Fredrik Söderquist, Guido Urdaneta, Peter Beverloo, Sami Kyöstilä and Sunggook Chue

Tom Sepez voted and added 1 comment

Votes added by Tom Sepez

Code-Review

+1

1 comment

File components/content_settings/core/common/content_settings_types.mojom

Line 398, Patchset 8 (Latest): // Whether an application can enumerate audio output device.

Tom Sepez . unresolved

nit: devices.

Open in Gerrit

Related details

Attention is currently required from:

Fredrik Söderquist
Guido Urdaneta

Peter Beverloo
Sami Kyöstilä
Sunggook Chue

satisfied_requirement

unsatisfied_requirement

open

diffy

Peter Beverloo (Gerrit)

unread,

Feb 15, 2024, 2:15:27 PM2/15/24

to Sunggook Chue, edgecapab...@microsoft.com, android-web...@chromium.org, asvitkine...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, blundell+...@chromium.org, chfreme...@chromium.org, devtools-re...@chromium.org, devtools...@chromium.org, dominickn+wat...@chromium.org, dullweb...@chromium.org, feature-co...@chromium.org, feature-me...@chromium.org, iclella...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org, michaelpg+wa...@chromium.org, msrame...@chromium.org, permissio...@chromium.org, Peter Beverloo, Tom Sepez, Theodore Olsauskas-Warren, Guido Urdaneta, Andrey Kosyakov, Balazs Engedy, Fredrik Söderquist, Sami Kyöstilä, Chromium IPC Reviews, Chromium LUCI CQ, Tricium, Chromium Metrics Reviews, chromium...@chromium.org, Luna Lu, Rijubrata Bhaumik

Attention needed from Fredrik Söderquist, Guido Urdaneta, Sami Kyöstilä and Sunggook Chue

Peter Beverloo voted Code-Review+1

Code-Review

+1

Open in Gerrit

Related details

Attention is currently required from:

Fredrik Söderquist
Guido Urdaneta
Sami Kyöstilä
Sunggook Chue

satisfied_requirement

unsatisfied_requirement

open

diffy

Sunggook Chue (Gerrit)

unread,

Feb 15, 2024, 2:44:10 PM2/15/24

to Fredrik Söderquist, edgecapab...@microsoft.com, android-web...@chromium.org, asvitkine...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, blundell+...@chromium.org, chfreme...@chromium.org, devtools-re...@chromium.org, devtools...@chromium.org, dominickn+wat...@chromium.org, dullweb...@chromium.org, feature-co...@chromium.org, feature-me...@chromium.org, iclella...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org, michaelpg+wa...@chromium.org, msrame...@chromium.org, permissio...@chromium.org, Peter Beverloo, Tom Sepez, Theodore Olsauskas-Warren, Guido Urdaneta, Andrey Kosyakov, Balazs Engedy, Sami Kyöstilä, Chromium IPC Reviews, Chromium LUCI CQ, Tricium, Chromium Metrics Reviews, chromium...@chromium.org, Luna Lu, Rijubrata Bhaumik

Attention needed from Guido Urdaneta, Sami Kyöstilä and Sunggook Chue

Sunggook Chue removed Fredrik Söderquist from this change

Deleted Reviewers:

Fredrik Söderquist

Open in Gerrit

Related details

Attention is currently required from:

Guido Urdaneta
Sami Kyöstilä
Sunggook Chue

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Sunggook Chue (Gerrit)

unread,

Feb 15, 2024, 2:44:24 PM2/15/24

to Sami Kyöstilä, edgecapab...@microsoft.com, android-web...@chromium.org, asvitkine...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, blundell+...@chromium.org, chfreme...@chromium.org, devtools-re...@chromium.org, devtools...@chromium.org, dominickn+wat...@chromium.org, dullweb...@chromium.org, feature-co...@chromium.org, feature-me...@chromium.org, iclella...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org, michaelpg+wa...@chromium.org, msrame...@chromium.org, permissio...@chromium.org, Peter Beverloo, Tom Sepez, Theodore Olsauskas-Warren, Guido Urdaneta, Andrey Kosyakov, Balazs Engedy, Chromium IPC Reviews, Chromium LUCI CQ, Tricium, Chromium Metrics Reviews, chromium...@chromium.org, Luna Lu, Rijubrata Bhaumik

Attention needed from Guido Urdaneta and Sunggook Chue

Sunggook Chue removed Sami Kyöstilä from this change

Deleted Reviewers:

Sami Kyöstilä

Open in Gerrit

Related details

Attention is currently required from:

Guido Urdaneta
Sunggook Chue

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Sunggook Chue (Gerrit)

unread,

Feb 15, 2024, 3:12:55 PM2/15/24

to Daniel Bratell, Ian Clelland, Nicola Tommasi, edgecapab...@microsoft.com, android-web...@chromium.org, asvitkine...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, blundell+...@chromium.org, chfreme...@chromium.org, devtools-re...@chromium.org, devtools...@chromium.org, dominickn+wat...@chromium.org, dullweb...@chromium.org, feature-co...@chromium.org, feature-me...@chromium.org, iclella...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org, michaelpg+wa...@chromium.org, msrame...@chromium.org, permissio...@chromium.org

Attention needed from Daniel Bratell, Ian Clelland and Nicola Tommasi

Sunggook Chue has uploaded the change for review

Sunggook Chue would like Daniel Bratell, Ian Clelland and Nicola Tommasi to review this change.

Commit message

Add placeholder of speaker-selection permission.

Incorporating the ‘speaker-selection’ permission from
'https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Permissions-Policy/speaker-selection'
this addition aligns with Chromium’s permission enhancements
(https://source.chromium.org/chromium/chromium/src/+/main:components/permissions/add_new_permission.md)

However, it’s important to note that this feature is not yet implemented in
the chrome://settings/content page. Additionally, the need for a permission
prompt remains uncertain (not edited related code), and use 'test' permission
policy to prevent unintended prompts outside of testing scenarios."

Bug: 41492674

Change-Id: Ia4025021424c2dc317e92bc026e2d3c8077950c7

Change diff

Change information

Files:

M android_webview/browser/aw_permission_manager.cc
M chrome/android/javatests/src/org/chromium/chrome/browser/site_settings/WebsitePermissionsFetcherTest.java

M chrome/browser/permissions/permission_manager_factory.cc
M chrome/browser/ui/webui/settings/site_settings_helper.cc
M components/content_settings/core/browser/content_settings_registry.cc
M components/content_settings/core/browser/content_settings_uma_util.cc
M components/content_settings/core/common/content_settings_types.mojom
M components/permissions/BUILD.gn
A components/permissions/contexts/speaker_selection_permission_context.cc
A components/permissions/contexts/speaker_selection_permission_context.h
M components/permissions/permission_util.cc
M content/browser/devtools/protocol/browser_handler.cc
M content/browser/permissions/permission_controller_impl.cc
M content/shell/browser/shell_permission_manager.cc
M third_party/blink/common/permissions/permission_utils.cc
M third_party/blink/public/common/permissions/permission_utils.h
M third_party/blink/public/devtools_protocol/browser_protocol.pdl
M third_party/blink/public/mojom/permissions/permission.mojom
M third_party/blink/public/mojom/permissions_policy/permissions_policy_feature.mojom
M third_party/blink/renderer/core/permissions_policy/feature_policy.dict
M third_party/blink/renderer/core/permissions_policy/permissions_policy_features.json5
M third_party/blink/renderer/modules/permissions/permission_descriptor.idl
M third_party/blink/renderer/modules/permissions/permission_utils.cc
M third_party/blink/renderer/platform/runtime_enabled_features.json5

M third_party/blink/web_tests/external/wpt/permissions/all-permissions-expected.txt
M third_party/blink/web_tests/http/tests/resources/permissions-helper.js
M third_party/blink/web_tests/webexposed/feature-policy-features-expected.txt
M tools/metrics/histograms/metadata/histogram_suffixes_list.xml

Change size: M

Delta: 28 files changed, 175 insertions(+), 7 deletions(-)

Open in Gerrit

Related details

Attention is currently required from:

Daniel Bratell
Ian Clelland
Nicola Tommasi

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Sunggook Chue (Gerrit)

unread,

Feb 15, 2024, 3:13:02 PM2/15/24

to edgecapab...@microsoft.com, android-web...@chromium.org, asvitkine...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, blundell+...@chromium.org, chfreme...@chromium.org, devtools-re...@chromium.org, devtools...@chromium.org, dominickn+wat...@chromium.org, dullweb...@chromium.org, feature-co...@chromium.org, feature-me...@chromium.org, iclella...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org, michaelpg+wa...@chromium.org, msrame...@chromium.org, permissio...@chromium.org, Nicola Tommasi, Ian Clelland, Daniel Bratell, Peter Beverloo, Tom Sepez, Theodore Olsauskas-Warren, Guido Urdaneta, Andrey Kosyakov, Balazs Engedy, Chromium IPC Reviews, Chromium LUCI CQ, Tricium, Chromium Metrics Reviews, chromium...@chromium.org, Luna Lu, Rijubrata Bhaumik

Attention needed from Daniel Bratell, Ian Clelland and Nicola Tommasi

Sunggook Chue added 1 comment

Patchset-level comments

File-level comment, Patchset 8 (Latest):

Sunggook Chue . resolved

Nicola for WebsitePermissionsFetcherTest.java,
Ian for Permissions_policy_features.json5, feature_policy.dict
Daniel for t/b/r/p/runtime_enabled_features.json5.

Thanks,

Open in Gerrit

Related details

Attention is currently required from:

Daniel Bratell
Ian Clelland
Nicola Tommasi

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Daniel Bratell (Gerrit)

unread,

Feb 15, 2024, 3:26:20 PM2/15/24

to Sunggook Chue, edgecapab...@microsoft.com, android-web...@chromium.org, asvitkine...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, blundell+...@chromium.org, chfreme...@chromium.org, devtools-re...@chromium.org, devtools...@chromium.org, dominickn+wat...@chromium.org, dullweb...@chromium.org, feature-co...@chromium.org, feature-me...@chromium.org, iclella...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org, michaelpg+wa...@chromium.org, msrame...@chromium.org, permissio...@chromium.org, Nicola Tommasi, Ian Clelland, Peter Beverloo, Tom Sepez, Theodore Olsauskas-Warren, Guido Urdaneta, Andrey Kosyakov, Balazs Engedy, Chromium IPC Reviews, Chromium LUCI CQ, Tricium, Chromium Metrics Reviews, chromium...@chromium.org, Luna Lu, Rijubrata Bhaumik

Attention needed from Ian Clelland, Nicola Tommasi and Sunggook Chue

Daniel Bratell voted Code-Review+1

Code-Review

+1

Open in Gerrit

Related details

Attention is currently required from:

Ian Clelland
Nicola Tommasi
Sunggook Chue

satisfied_requirement

unsatisfied_requirement

open

diffy

Nicola Tommasi (Gerrit)

unread,

Feb 16, 2024, 3:14:01 AM2/16/24

to Sunggook Chue, edgecapab...@microsoft.com, android-web...@chromium.org, asvitkine...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, blundell+...@chromium.org, chfreme...@chromium.org, devtools-re...@chromium.org, devtools...@chromium.org, dominickn+wat...@chromium.org, dullweb...@chromium.org, feature-co...@chromium.org, feature-me...@chromium.org, iclella...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org, michaelpg+wa...@chromium.org, msrame...@chromium.org, permissio...@chromium.org, Nicola Tommasi, Daniel Bratell, Ian Clelland, Peter Beverloo, Tom Sepez, Theodore Olsauskas-Warren, Guido Urdaneta, Andrey Kosyakov, Balazs Engedy, Chromium IPC Reviews, Chromium LUCI CQ, Tricium, Chromium Metrics Reviews, chromium...@chromium.org, Luna Lu, Rijubrata Bhaumik

Attention needed from Ian Clelland and Sunggook Chue

Nicola Tommasi voted Code-Review+1

Code-Review

+1

Open in Gerrit

Related details

Attention is currently required from:

Ian Clelland
Sunggook Chue

satisfied_requirement

unsatisfied_requirement

open

diffy

Sunggook Chue (Gerrit)

unread,

Feb 16, 2024, 2:13:37 PM2/16/24

to Ari Chivukula, Kevin Ellis, Ian Clelland, edgecapab...@microsoft.com, android-web...@chromium.org, asvitkine...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, blundell+...@chromium.org, chfreme...@chromium.org, devtools-re...@chromium.org, devtools...@chromium.org, dominickn+wat...@chromium.org, dullweb...@chromium.org, feature-co...@chromium.org, feature-me...@chromium.org, iclella...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org, michaelpg+wa...@chromium.org, msrame...@chromium.org, permissio...@chromium.org

Attention needed from Ari Chivukula and Kevin Ellis

Sunggook Chue has uploaded the change for review

Sunggook Chue would like Ari Chivukula and Kevin Ellis to review this change.

Sunggook Chue removed Ian Clelland from reviewers of this change.

Open in Gerrit

Related details

Attention is currently required from:

Ari Chivukula
Kevin Ellis

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Sunggook Chue (Gerrit)

unread,

Feb 16, 2024, 2:13:43 PM2/16/24

to edgecapab...@microsoft.com, android-web...@chromium.org, asvitkine...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, blundell+...@chromium.org, chfreme...@chromium.org, devtools-re...@chromium.org, devtools...@chromium.org, dominickn+wat...@chromium.org, dullweb...@chromium.org, feature-co...@chromium.org, feature-me...@chromium.org, iclella...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org, michaelpg+wa...@chromium.org, msrame...@chromium.org, permissio...@chromium.org, Kevin Ellis, Ari Chivukula, Nicola Tommasi, Daniel Bratell, Peter Beverloo, Tom Sepez, Theodore Olsauskas-Warren, Guido Urdaneta, Andrey Kosyakov, Balazs Engedy, Chromium IPC Reviews, Chromium LUCI CQ, Tricium, Chromium Metrics Reviews, chromium...@chromium.org, Luna Lu, Rijubrata Bhaumik

Attention needed from Ari Chivukula and Kevin Ellis

Sunggook Chue added 1 comment

Patchset-level comments

File-level comment, Patchset 8 (Latest):

Sunggook Chue . resolved

Adding Ari and Kevin for t/b/r/c/p/feature_policy.dict. Thanks.

Open in Gerrit

Related details

Attention is currently required from:

Ari Chivukula
Kevin Ellis

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Ari Chivukula (Gerrit)

unread,

Feb 16, 2024, 3:21:29 PM2/16/24

to Sunggook Chue, edgecapab...@microsoft.com, android-web...@chromium.org, asvitkine...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, blundell+...@chromium.org, chfreme...@chromium.org, devtools-re...@chromium.org, devtools...@chromium.org, dominickn+wat...@chromium.org, dullweb...@chromium.org, feature-co...@chromium.org, feature-me...@chromium.org, iclella...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org, michaelpg+wa...@chromium.org, msrame...@chromium.org, permissio...@chromium.org, Kevin Ellis, Nicola Tommasi, Daniel Bratell, Peter Beverloo, Tom Sepez, Theodore Olsauskas-Warren, Guido Urdaneta, Andrey Kosyakov, Balazs Engedy, Chromium IPC Reviews, Chromium LUCI CQ, Tricium, Chromium Metrics Reviews, chromium...@chromium.org, Luna Lu, Rijubrata Bhaumik

Attention needed from Kevin Ellis and Sunggook Chue

Ari Chivukula voted and added 1 comment

Votes added by Ari Chivukula

Code-Review

+1

1 comment

Patchset-level comments

File-level comment, Patchset 8 (Latest):

Ari Chivukula . resolved

LGTM for feature dict specific changes, did not look elsewhere (trusting others on overall design)

Open in Gerrit

Related details

Attention is currently required from:

Kevin Ellis
Sunggook Chue

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review

satisfied_requirement

open

diffy

Sunggook Chue (Gerrit)

unread,

Feb 16, 2024, 3:25:56 PM2/16/24

to edgecapab...@microsoft.com, android-web...@chromium.org, asvitkine...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, blundell+...@chromium.org, chfreme...@chromium.org, devtools-re...@chromium.org, devtools...@chromium.org, dominickn+wat...@chromium.org, dullweb...@chromium.org, feature-co...@chromium.org, feature-me...@chromium.org, iclella...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org, michaelpg+wa...@chromium.org, msrame...@chromium.org, permissio...@chromium.org, Ari Chivukula, Kevin Ellis, Nicola Tommasi, Daniel Bratell, Peter Beverloo, Tom Sepez, Theodore Olsauskas-Warren, Guido Urdaneta, Andrey Kosyakov, Balazs Engedy, Chromium IPC Reviews, Chromium LUCI CQ, Tricium, Chromium Metrics Reviews, chromium...@chromium.org, Luna Lu, Rijubrata Bhaumik

Attention needed from Kevin Ellis

Sunggook Chue voted Commit-Queue+2

Commit-Queue

+2

Open in Gerrit

Related details

Attention is currently required from:

Kevin Ellis

satisfied_requirement

open

diffy

Chromium LUCI CQ (Gerrit)

unread,

Feb 16, 2024, 4:28:06 PM2/16/24

to Sunggook Chue, edgecapab...@microsoft.com, android-web...@chromium.org, asvitkine...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, blundell+...@chromium.org, chfreme...@chromium.org, devtools-re...@chromium.org, devtools...@chromium.org, dominickn+wat...@chromium.org, dullweb...@chromium.org, feature-co...@chromium.org, feature-me...@chromium.org, iclella...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org, michaelpg+wa...@chromium.org, msrame...@chromium.org, permissio...@chromium.org, Ari Chivukula, Kevin Ellis, Nicola Tommasi, Daniel Bratell, Peter Beverloo, Tom Sepez, Theodore Olsauskas-Warren, Guido Urdaneta, Andrey Kosyakov, Balazs Engedy, Chromium IPC Reviews, Tricium, Chromium Metrics Reviews, chromium...@chromium.org, Luna Lu, Rijubrata Bhaumik

Chromium LUCI CQ submitted the change

Change information

Commit message:

Add placeholder of speaker-selection permission.

Incorporating the ‘speaker-selection’ permission from
'https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Permissions-Policy/speaker-selection'
this addition aligns with Chromium’s permission enhancements
(https://source.chromium.org/chromium/chromium/src/+/main:components/permissions/add_new_permission.md)

However, it’s important to note that this feature is not yet implemented in
the chrome://settings/content page. Additionally, the need for a permission
prompt remains uncertain (not edited related code), and use 'test' permission
policy to prevent unintended prompts outside of testing scenarios."

Bug: 41492674

Change-Id: Ia4025021424c2dc317e92bc026e2d3c8077950c7

Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5278243

Reviewed-by: Theodore Olsauskas-Warren <sau...@google.com>

Reviewed-by: Nicola Tommasi <tomm...@chromium.org>

Reviewed-by: Balazs Engedy <eng...@chromium.org>

Reviewed-by: Peter Beverloo <pe...@chromium.org>

Reviewed-by: Guido Urdaneta <gui...@chromium.org>

Reviewed-by: Andrey Kosyakov <ca...@chromium.org>

Commit-Queue: Sunggook Chue <sun...@microsoft.com>

Reviewed-by: Ari Chivukula <ari...@chromium.org>

Reviewed-by: Daniel Bratell <brat...@gmail.com>

Reviewed-by: Tom Sepez <tse...@chromium.org>

Cr-Commit-Position: refs/heads/main@{#1261900}

Branch: refs/heads/main

Submit Requirements:

Code-Review: +1 by Peter Beverloo, +1 by Andrey Kosyakov, +1 by Balazs Engedy, +1 by Guido Urdaneta, +1 by Theodore Olsauskas-Warren, +1 by Nicola Tommasi, +1 by Daniel Bratell, +1 by Ari Chivukula, +1 by Tom Sepez

Open in Gerrit

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

open

diffy

satisfied_requirement

Reply all

Reply to author

Forward

Add placeholder of speaker-selection permission. [chromium/src : main]

Sunggook Chue (Gerrit)

Sunggook Chue has uploaded the change for review

Commit message

Change diff

Change information

Related details

Sunggook Chue (Gerrit)

Sunggook Chue added 1 comment

Related details

Sunggook Chue (Gerrit)

Sunggook Chue uploaded new patchset

Related details

Guido Urdaneta (Gerrit)

Guido Urdaneta added 5 comments

Related details

Sunggook Chue (Gerrit)

Sunggook Chue uploaded new patchset

Related details

Sunggook Chue (Gerrit)

Sunggook Chue added 5 comments

Related details

Guido Urdaneta (Gerrit)

Guido Urdaneta added 1 comment

Related details

Guido Urdaneta (Gerrit)

Guido Urdaneta voted and added 1 comment

Votes added by Guido Urdaneta

1 comment

Sunggook Chue (Gerrit)

Sunggook Chue has uploaded the change for review

Commit message

Change diff

Change information

Related details

gwsq (Gerrit)

Sunggook Chue has uploaded the change for review

Related details

gwsq (Gerrit)

Sunggook Chue has uploaded the change for review

Related details

gwsq (Gerrit)

Message from gwsq

Related details

Sunggook Chue (Gerrit)

Sunggook Chue uploaded new patchset

Related details

Sunggook Chue (Gerrit)

Sunggook Chue added 2 comments

Related details

Guido Urdaneta (Gerrit)

Guido Urdaneta voted and added 1 comment

Votes added by Guido Urdaneta

1 comment

Related details

Balazs Engedy (Gerrit)

Balazs Engedy added 1 comment

Related details

Tom Sepez (Gerrit)

Tom Sepez voted and added 1 comment

Votes added by Tom Sepez

1 comment

Related details

Sunggook Chue (Gerrit)

Sunggook Chue uploaded new patchset

Related details

Sunggook Chue (Gerrit)

Sunggook Chue uploaded new patchset

Related details

Sunggook Chue (Gerrit)

Sunggook Chue has uploaded the change for review

Related details

Sunggook Chue (Gerrit)

Sunggook Chue added 2 comments

Related details

Andrey Kosyakov (Gerrit)

Andrey Kosyakov voted and added 1 comment

Votes added by Andrey Kosyakov

1 comment

Related details