Automatic Fullscreen: Prototype Permissions API query support [chromium/src : main]

78 views
Skip to first unread message

Mike Wasserman (Gerrit)

unread,
Jun 14, 2024, 5:18:00 PMJun 14
to Brad Triebwasser, Mike West, Bo Liu, Kentaro Hara, Elias Klim, AyeAye, Chromium LUCI CQ, Takumi Fujimoto, chromium...@chromium.org, devtools...@chromium.org, Permissions Reviews, Peter Beverloo, devtools-re...@chromium.org, ipc-securi...@chromium.org, blink-revie...@chromium.org, dullweb...@chromium.org, msrame...@chromium.org, michaelpg+wa...@chromium.org, kuragin+web-ap...@chromium.org, dominickn+watch-...@chromium.org, alancutter...@chromium.org, mgiuca...@chromium.org, philli...@chromium.org, loyso...@chromium.org, ericwillige...@chromium.org, dmurph+watc...@chromium.org, dibyapal+wa...@chromium.org, webap...@microsoft.com, zelin+watch-we...@chromium.org, android-web...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, blundell+...@chromium.org, dominickn+wat...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org

Mike Wasserman voted and added 1 comment

Votes added by Mike Wasserman

Code-Review+0

1 comment

Patchset-level comments
File-level comment, Patchset 19 (Latest):
Mike Wasserman . resolved

elklm, please take the first look at this revised prototype, thanks!

Open in Gerrit

Related details

Attention set is empty
Submit Requirements:
  • requirement satisfiedCode-Coverage
  • requirement is not satisfiedCode-Review
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
Gerrit-MessageType: comment
Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: If9adac187ef712a9064855055953d3882305bf05
Gerrit-Change-Number: 5583182
Gerrit-PatchSet: 19
Gerrit-Owner: Mike Wasserman <m...@chromium.org>
Gerrit-Reviewer: Elias Klim <el...@chromium.org>
Gerrit-Reviewer: Mike Wasserman <m...@chromium.org>
Gerrit-CC: Bo Liu <bo...@chromium.org>
Gerrit-CC: Brad Triebwasser <btr...@chromium.org>
Gerrit-CC: Kentaro Hara <har...@chromium.org>
Gerrit-CC: Mike West <mk...@chromium.org>
Gerrit-CC: Permissions Reviews <permissio...@chromium.org>
Gerrit-CC: Peter Beverloo <pe...@chromium.org>
Gerrit-CC: Takumi Fujimoto <tak...@chromium.org>
Gerrit-Comment-Date: Fri, 14 Jun 2024 21:17:50 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
satisfied_requirement
unsatisfied_requirement
open
diffy

Elias Klim (Gerrit)

unread,
Jun 17, 2024, 8:39:49 AM (12 days ago) Jun 17
to Mike Wasserman, Brad Triebwasser, Mike West, Bo Liu, Kentaro Hara, AyeAye, Chromium LUCI CQ, Takumi Fujimoto, chromium...@chromium.org, devtools...@chromium.org, Permissions Reviews, Peter Beverloo, devtools-re...@chromium.org, ipc-securi...@chromium.org, blink-revie...@chromium.org, dullweb...@chromium.org, msrame...@chromium.org, michaelpg+wa...@chromium.org, kuragin+web-ap...@chromium.org, dominickn+watch-...@chromium.org, alancutter...@chromium.org, mgiuca...@chromium.org, philli...@chromium.org, loyso...@chromium.org, ericwillige...@chromium.org, dmurph+watc...@chromium.org, dibyapal+wa...@chromium.org, webap...@microsoft.com, zelin+watch-we...@chromium.org, android-web...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, blundell+...@chromium.org, dominickn+wat...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org
Attention needed from Mike Wasserman

Elias Klim voted and added 1 comment

Votes added by Elias Klim

Code-Review+1

1 comment

Patchset-level comments
Elias Klim . resolved

Thank you, lgtm

Open in Gerrit

Related details

Attention is currently required from:
  • Mike Wasserman
Submit Requirements:
  • requirement satisfiedCode-Coverage
  • requirement satisfiedCode-Review
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
Gerrit-MessageType: comment
Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: If9adac187ef712a9064855055953d3882305bf05
Gerrit-Change-Number: 5583182
Gerrit-PatchSet: 19
Gerrit-Owner: Mike Wasserman <m...@chromium.org>
Gerrit-Reviewer: Elias Klim <el...@chromium.org>
Gerrit-Reviewer: Mike Wasserman <m...@chromium.org>
Gerrit-CC: Bo Liu <bo...@chromium.org>
Gerrit-CC: Brad Triebwasser <btr...@chromium.org>
Gerrit-CC: Kentaro Hara <har...@chromium.org>
Gerrit-CC: Mike West <mk...@chromium.org>
Gerrit-CC: Permissions Reviews <permissio...@chromium.org>
Gerrit-CC: Peter Beverloo <pe...@chromium.org>
Gerrit-CC: Takumi Fujimoto <tak...@chromium.org>
Gerrit-Attention: Mike Wasserman <m...@chromium.org>
Gerrit-Comment-Date: Mon, 17 Jun 2024 12:39:37 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
satisfied_requirement
open
diffy

Mike Wasserman (Gerrit)

unread,
Jun 17, 2024, 3:56:16 PM (12 days ago) Jun 17
to Mike West, Bo Liu, Andrey Kosyakov, Elias Klim, Brad Triebwasser, Kentaro Hara, AyeAye, Chromium LUCI CQ, Takumi Fujimoto, chromium...@chromium.org, devtools...@chromium.org, Permissions Reviews, Peter Beverloo, devtools-re...@chromium.org, ipc-securi...@chromium.org, blink-revie...@chromium.org, dullweb...@chromium.org, msrame...@chromium.org, michaelpg+wa...@chromium.org, kuragin+web-ap...@chromium.org, dominickn+watch-...@chromium.org, alancutter...@chromium.org, mgiuca...@chromium.org, philli...@chromium.org, loyso...@chromium.org, ericwillige...@chromium.org, dmurph+watc...@chromium.org, dibyapal+wa...@chromium.org, webap...@microsoft.com, zelin+watch-we...@chromium.org, android-web...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, blundell+...@chromium.org, dominickn+wat...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org
Attention needed from Andrey Kosyakov, Bo Liu and Mike West

Mike Wasserman voted and added 1 comment

Votes added by Mike Wasserman

Auto-Submit+1
Commit-Queue+1

1 comment

Patchset-level comments
File-level comment, Patchset 19:
Mike Wasserman . resolved

Thanks elklm. Other owners, please review the updated descriptor shape:
mkwst: shell_permission_manager.cc, permission.mojom, runtime_enabled_features.json5
boliu: aw_permission_manager.cc, browser_handler.cc
caseq: browser_protocol.pdl
Thanks!

Open in Gerrit

Related details

Attention is currently required from:
  • Andrey Kosyakov
  • Bo Liu
  • Mike West
Submit Requirements:
  • requirement satisfiedCode-Coverage
  • requirement satisfiedCode-Review
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
Gerrit-MessageType: comment
Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: If9adac187ef712a9064855055953d3882305bf05
Gerrit-Change-Number: 5583182
Gerrit-PatchSet: 20
Gerrit-Owner: Mike Wasserman <m...@chromium.org>
Gerrit-Reviewer: Andrey Kosyakov <ca...@chromium.org>
Gerrit-Reviewer: Bo Liu <bo...@chromium.org>
Gerrit-Reviewer: Elias Klim <el...@chromium.org>
Gerrit-Reviewer: Mike Wasserman <m...@chromium.org>
Gerrit-Reviewer: Mike West <mk...@chromium.org>
Gerrit-CC: Brad Triebwasser <btr...@chromium.org>
Gerrit-CC: Kentaro Hara <har...@chromium.org>
Gerrit-CC: Permissions Reviews <permissio...@chromium.org>
Gerrit-CC: Peter Beverloo <pe...@chromium.org>
Gerrit-CC: Takumi Fujimoto <tak...@chromium.org>
Gerrit-Attention: Bo Liu <bo...@chromium.org>
Gerrit-Attention: Andrey Kosyakov <ca...@chromium.org>
Gerrit-Attention: Mike West <mk...@chromium.org>
Gerrit-Comment-Date: Mon, 17 Jun 2024 19:56:03 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
satisfied_requirement
open
diffy

Bo Liu (Gerrit)

unread,
Jun 17, 2024, 4:10:20 PM (12 days ago) Jun 17
to Mike Wasserman, Bo Liu, Mike West, Andrey Kosyakov, Elias Klim, Brad Triebwasser, Kentaro Hara, AyeAye, Chromium LUCI CQ, Takumi Fujimoto, chromium...@chromium.org, devtools...@chromium.org, Permissions Reviews, Peter Beverloo, devtools-re...@chromium.org, ipc-securi...@chromium.org, blink-revie...@chromium.org, dullweb...@chromium.org, msrame...@chromium.org, michaelpg+wa...@chromium.org, kuragin+web-ap...@chromium.org, dominickn+watch-...@chromium.org, alancutter...@chromium.org, mgiuca...@chromium.org, philli...@chromium.org, loyso...@chromium.org, ericwillige...@chromium.org, dmurph+watc...@chromium.org, dibyapal+wa...@chromium.org, webap...@microsoft.com, zelin+watch-we...@chromium.org, android-web...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, blundell+...@chromium.org, dominickn+wat...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org
Attention needed from Andrey Kosyakov, Mike Wasserman and Mike West

Bo Liu voted and added 1 comment

Votes added by Bo Liu

Code-Review+1
Commit-Queue+2

1 comment

Patchset-level comments
Open in Gerrit

Related details

Attention is currently required from:
  • Andrey Kosyakov
  • Mike Wasserman
  • Mike West
Gerrit-Attention: Andrey Kosyakov <ca...@chromium.org>
Gerrit-Attention: Mike West <mk...@chromium.org>
Gerrit-Attention: Mike Wasserman <m...@chromium.org>
Gerrit-Comment-Date: Mon, 17 Jun 2024 20:10:05 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
satisfied_requirement
open
diffy

Bo Liu (Gerrit)

unread,
Jun 17, 2024, 4:10:35 PM (12 days ago) Jun 17
to Mike Wasserman, Bo Liu, Mike West, Andrey Kosyakov, Elias Klim, Brad Triebwasser, Kentaro Hara, AyeAye, Chromium LUCI CQ, Takumi Fujimoto, chromium...@chromium.org, devtools...@chromium.org, Permissions Reviews, Peter Beverloo, devtools-re...@chromium.org, ipc-securi...@chromium.org, blink-revie...@chromium.org, dullweb...@chromium.org, msrame...@chromium.org, michaelpg+wa...@chromium.org, kuragin+web-ap...@chromium.org, dominickn+watch-...@chromium.org, alancutter...@chromium.org, mgiuca...@chromium.org, philli...@chromium.org, loyso...@chromium.org, ericwillige...@chromium.org, dmurph+watc...@chromium.org, dibyapal+wa...@chromium.org, webap...@microsoft.com, zelin+watch-we...@chromium.org, android-web...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, blundell+...@chromium.org, dominickn+wat...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org
Attention needed from Andrey Kosyakov, Mike Wasserman and Mike West

Bo Liu voted Commit-Queue+0

Commit-Queue+0
Gerrit-Comment-Date: Mon, 17 Jun 2024 20:10:18 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
satisfied_requirement
open
diffy

Andrey Kosyakov (Gerrit)

unread,
Jun 17, 2024, 4:14:36 PM (12 days ago) Jun 17
to Mike Wasserman, Bo Liu, Mike West, Elias Klim, Brad Triebwasser, Kentaro Hara, AyeAye, Chromium LUCI CQ, Takumi Fujimoto, chromium...@chromium.org, devtools...@chromium.org, Permissions Reviews, Peter Beverloo, devtools-re...@chromium.org, ipc-securi...@chromium.org, blink-revie...@chromium.org, dullweb...@chromium.org, msrame...@chromium.org, michaelpg+wa...@chromium.org, kuragin+web-ap...@chromium.org, dominickn+watch-...@chromium.org, alancutter...@chromium.org, mgiuca...@chromium.org, philli...@chromium.org, loyso...@chromium.org, ericwillige...@chromium.org, dmurph+watc...@chromium.org, dibyapal+wa...@chromium.org, webap...@microsoft.com, zelin+watch-we...@chromium.org, android-web...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, blundell+...@chromium.org, dominickn+wat...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org
Attention needed from Mike Wasserman and Mike West

Andrey Kosyakov added 3 comments

File content/browser/devtools/protocol/browser_handler.cc
Line 193, Patchset 20 (Latest): if (descriptor->GetAllowWithoutGesture(false)) {
Andrey Kosyakov . unresolved

Is there a point in requiring this dance on part of the client? Can we just imply `allowWithoutGesture: false` here, or is 'allowWithoutGesture: true` coming in the future?

Line 196, Patchset 20 (Latest): "Fullscreen Permission only supports allowWithoutGesture:true");
Andrey Kosyakov . unresolved

That's the opposite of what you're actually checking though.

File third_party/blink/public/devtools_protocol/browser_protocol.pdl
Line 1366, Patchset 20 (Latest): # For "fullscreen" permission, may specify allowWithoutGesture.
Andrey Kosyakov . unresolved

Looks more like "must" on the implementation side to me ;-)

Open in Gerrit

Related details

Attention is currently required from:
  • Mike Wasserman
  • Mike West
Submit Requirements:
    • requirement satisfiedCode-Coverage
    • requirement satisfiedCode-Review
    • requirement is not satisfiedNo-Unresolved-Comments
    Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
    Gerrit-MessageType: comment
    Gerrit-Project: chromium/src
    Gerrit-Branch: main
    Gerrit-Change-Id: If9adac187ef712a9064855055953d3882305bf05
    Gerrit-Change-Number: 5583182
    Gerrit-PatchSet: 20
    Gerrit-Owner: Mike Wasserman <m...@chromium.org>
    Gerrit-Reviewer: Bo Liu <bo...@chromium.org>
    Gerrit-Reviewer: Elias Klim <el...@chromium.org>
    Gerrit-Reviewer: Mike Wasserman <m...@chromium.org>
    Gerrit-Reviewer: Mike West <mk...@chromium.org>
    Gerrit-CC: Andrey Kosyakov <ca...@chromium.org>
    Gerrit-CC: Brad Triebwasser <btr...@chromium.org>
    Gerrit-CC: Kentaro Hara <har...@chromium.org>
    Gerrit-CC: Permissions Reviews <permissio...@chromium.org>
    Gerrit-CC: Peter Beverloo <pe...@chromium.org>
    Gerrit-CC: Takumi Fujimoto <tak...@chromium.org>
    Gerrit-Attention: Mike West <mk...@chromium.org>
    Gerrit-Attention: Mike Wasserman <m...@chromium.org>
    Gerrit-Comment-Date: Mon, 17 Jun 2024 20:14:18 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: No
    satisfied_requirement
    unsatisfied_requirement
    open
    diffy

    Mike Wasserman (Gerrit)

    unread,
    Jun 17, 2024, 4:14:47 PM (12 days ago) Jun 17
    to Andrey Kosyakov, Bo Liu, Mike West, Elias Klim, Brad Triebwasser, Kentaro Hara, AyeAye, Chromium LUCI CQ, Takumi Fujimoto, chromium...@chromium.org, devtools...@chromium.org, Permissions Reviews, Peter Beverloo, devtools-re...@chromium.org, ipc-securi...@chromium.org, blink-revie...@chromium.org, dullweb...@chromium.org, msrame...@chromium.org, michaelpg+wa...@chromium.org, kuragin+web-ap...@chromium.org, dominickn+watch-...@chromium.org, alancutter...@chromium.org, mgiuca...@chromium.org, philli...@chromium.org, loyso...@chromium.org, ericwillige...@chromium.org, dmurph+watc...@chromium.org, dibyapal+wa...@chromium.org, webap...@microsoft.com, zelin+watch-we...@chromium.org, android-web...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, blundell+...@chromium.org, dominickn+wat...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org
    Attention needed from Mike Wasserman and Mike West

    Mike Wasserman voted and added 1 comment

    Votes added by Mike Wasserman

    Auto-Submit+0
    Commit-Queue+1

    1 comment

    Patchset-level comments
    Mike Wasserman . resolved

    Moving caseq to CC and asking mkwst to also review browser_protocol.pdl.
    Thanks for unchecking CQ+2, boliu! mkwst should re-review before landing.

    Gerrit-Comment-Date: Mon, 17 Jun 2024 20:14:31 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: Yes
    satisfied_requirement
    unsatisfied_requirement
    open
    diffy

    Mike Wasserman (Gerrit)

    unread,
    Jun 17, 2024, 4:20:49 PM (12 days ago) Jun 17
    to Andrey Kosyakov, Bo Liu, Mike West, Elias Klim, Brad Triebwasser, Kentaro Hara, AyeAye, Chromium LUCI CQ, Takumi Fujimoto, chromium...@chromium.org, devtools...@chromium.org, Permissions Reviews, Peter Beverloo, devtools-re...@chromium.org, ipc-securi...@chromium.org, blink-revie...@chromium.org, dullweb...@chromium.org, msrame...@chromium.org, michaelpg+wa...@chromium.org, kuragin+web-ap...@chromium.org, dominickn+watch-...@chromium.org, alancutter...@chromium.org, mgiuca...@chromium.org, philli...@chromium.org, loyso...@chromium.org, ericwillige...@chromium.org, dmurph+watc...@chromium.org, dibyapal+wa...@chromium.org, webap...@microsoft.com, zelin+watch-we...@chromium.org, android-web...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, blundell+...@chromium.org, dominickn+wat...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org
    Attention needed from Andrey Kosyakov and Mike West

    Mike Wasserman added 4 comments

    Patchset-level comments
    Mike Wasserman . resolved

    caseq: thanks for the prompt review, please take another look or ping me to chat, thanks.

    File content/browser/devtools/protocol/browser_handler.cc
    Line 193, Patchset 20 (Latest): if (descriptor->GetAllowWithoutGesture(false)) {
    Andrey Kosyakov . unresolved

    Is there a point in requiring this dance on part of the client? Can we just imply `allowWithoutGesture: false` here, or is 'allowWithoutGesture: true` coming in the future?

    Mike Wasserman

    `allowWithoutGesture: true` is the only supported descriptor for now.

    Line 196, Patchset 20 (Latest): "Fullscreen Permission only supports allowWithoutGesture:true");
    Andrey Kosyakov . unresolved

    That's the opposite of what you're actually checking though.

    Mike Wasserman

    Sorry, can you clarify? The intent here is to get the caller-specified `allowWithoutGesture`, with a `false` default value, even though `false` is not supported. i.e. the caller must specify `allowWithoutGesture: true`.

    File third_party/blink/public/devtools_protocol/browser_protocol.pdl
    Line 1366, Patchset 20 (Latest): # For "fullscreen" permission, may specify allowWithoutGesture.
    Andrey Kosyakov . unresolved

    Looks more like "must" on the implementation side to me ;-)

    Mike Wasserman

    Indeed, the false default value is not supported for now. I can update the comment if appropriate.

    Open in Gerrit

    Related details

    Attention is currently required from:
    • Andrey Kosyakov
    • Mike West
    Submit Requirements:
    • requirement satisfiedCode-Coverage
    • requirement satisfiedCode-Review
    • requirement is not satisfiedNo-Unresolved-Comments
    Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
    Gerrit-MessageType: comment
    Gerrit-Project: chromium/src
    Gerrit-Branch: main
    Gerrit-Change-Id: If9adac187ef712a9064855055953d3882305bf05
    Gerrit-Change-Number: 5583182
    Gerrit-PatchSet: 20
    Gerrit-Owner: Mike Wasserman <m...@chromium.org>
    Gerrit-Reviewer: Andrey Kosyakov <ca...@chromium.org>
    Gerrit-Reviewer: Bo Liu <bo...@chromium.org>
    Gerrit-Reviewer: Elias Klim <el...@chromium.org>
    Gerrit-Reviewer: Mike Wasserman <m...@chromium.org>
    Gerrit-Reviewer: Mike West <mk...@chromium.org>
    Gerrit-CC: Brad Triebwasser <btr...@chromium.org>
    Gerrit-CC: Kentaro Hara <har...@chromium.org>
    Gerrit-CC: Permissions Reviews <permissio...@chromium.org>
    Gerrit-CC: Peter Beverloo <pe...@chromium.org>
    Gerrit-CC: Takumi Fujimoto <tak...@chromium.org>
    Gerrit-Attention: Andrey Kosyakov <ca...@chromium.org>
    Gerrit-Attention: Mike West <mk...@chromium.org>
    Gerrit-Comment-Date: Mon, 17 Jun 2024 20:20:35 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: No
    Comment-In-Reply-To: Andrey Kosyakov <ca...@chromium.org>
    satisfied_requirement
    unsatisfied_requirement
    open
    diffy

    Andrey Kosyakov (Gerrit)

    unread,
    Jun 17, 2024, 4:30:18 PM (12 days ago) Jun 17
    to Mike Wasserman, Bo Liu, Mike West, Elias Klim, Brad Triebwasser, Kentaro Hara, AyeAye, Chromium LUCI CQ, Takumi Fujimoto, chromium...@chromium.org, devtools...@chromium.org, Permissions Reviews, Peter Beverloo, devtools-re...@chromium.org, ipc-securi...@chromium.org, blink-revie...@chromium.org, dullweb...@chromium.org, msrame...@chromium.org, michaelpg+wa...@chromium.org, kuragin+web-ap...@chromium.org, dominickn+watch-...@chromium.org, alancutter...@chromium.org, mgiuca...@chromium.org, philli...@chromium.org, loyso...@chromium.org, ericwillige...@chromium.org, dmurph+watc...@chromium.org, dibyapal+wa...@chromium.org, webap...@microsoft.com, zelin+watch-we...@chromium.org, android-web...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, blundell+...@chromium.org, dominickn+wat...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org
    Attention needed from Mike Wasserman and Mike West

    Andrey Kosyakov added 1 comment

    File content/browser/devtools/protocol/browser_handler.cc
    Line 196, Patchset 20 (Latest): "Fullscreen Permission only supports allowWithoutGesture:true");
    Andrey Kosyakov . unresolved

    That's the opposite of what you're actually checking though.

    Mike Wasserman

    Sorry, can you clarify? The intent here is to get the caller-specified `allowWithoutGesture`, with a `false` default value, even though `false` is not supported. i.e. the caller must specify `allowWithoutGesture: true`.

    Andrey Kosyakov

    The intent sounds good, however `descriptor->GetAllowWithoutGesture(false)` will return true in case `allowWithoutGesture: true` is specified, and you appear to return an error just in this case.
    Which convenient brings us to the subject of test coverage for this code ;-)

    Open in Gerrit

    Related details

    Attention is currently required from:
    • Mike Wasserman
    • Mike West
    Gerrit-Attention: Mike West <mk...@chromium.org>
    Gerrit-Attention: Mike Wasserman <m...@chromium.org>
    Gerrit-Comment-Date: Mon, 17 Jun 2024 20:30:02 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: No
    Comment-In-Reply-To: Andrey Kosyakov <ca...@chromium.org>
    Comment-In-Reply-To: Mike Wasserman <m...@chromium.org>
    satisfied_requirement
    unsatisfied_requirement
    open
    diffy

    Andrey Kosyakov (Gerrit)

    unread,
    Jun 17, 2024, 4:30:47 PM (12 days ago) Jun 17
    to Mike Wasserman, Bo Liu, Mike West, Elias Klim, Brad Triebwasser, Kentaro Hara, AyeAye, Chromium LUCI CQ, Takumi Fujimoto, chromium...@chromium.org, devtools...@chromium.org, Permissions Reviews, Peter Beverloo, devtools-re...@chromium.org, ipc-securi...@chromium.org, blink-revie...@chromium.org, dullweb...@chromium.org, msrame...@chromium.org, michaelpg+wa...@chromium.org, kuragin+web-ap...@chromium.org, dominickn+watch-...@chromium.org, alancutter...@chromium.org, mgiuca...@chromium.org, philli...@chromium.org, loyso...@chromium.org, ericwillige...@chromium.org, dmurph+watc...@chromium.org, dibyapal+wa...@chromium.org, webap...@microsoft.com, zelin+watch-we...@chromium.org, android-web...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, blundell+...@chromium.org, dominickn+wat...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org
    Attention needed from Mike Wasserman and Mike West

    Andrey Kosyakov added 1 comment

    File content/browser/devtools/protocol/browser_handler.cc
    Line 196, Patchset 20 (Latest): "Fullscreen Permission only supports allowWithoutGesture:true");
    Andrey Kosyakov . unresolved

    That's the opposite of what you're actually checking though.

    Mike Wasserman

    Sorry, can you clarify? The intent here is to get the caller-specified `allowWithoutGesture`, with a `false` default value, even though `false` is not supported. i.e. the caller must specify `allowWithoutGesture: true`.

    Andrey Kosyakov

    The intent sounds good, however `descriptor->GetAllowWithoutGesture(false)` will return true in case `allowWithoutGesture: true` is specified, and you appear to return an error just in this case.
    Which convenient brings us to the subject of test coverage for this code ;-)

    Andrey Kosyakov

    *conveniently

    Gerrit-Comment-Date: Mon, 17 Jun 2024 20:30:31 +0000
    satisfied_requirement
    unsatisfied_requirement
    open
    diffy

    Mike Wasserman (Gerrit)

    unread,
    Jun 17, 2024, 6:51:09 PM (12 days ago) Jun 17
    to Andrey Kosyakov, Bo Liu, Mike West, Elias Klim, Brad Triebwasser, Kentaro Hara, AyeAye, Chromium LUCI CQ, Takumi Fujimoto, chromium...@chromium.org, devtools...@chromium.org, Permissions Reviews, Peter Beverloo, blink-revie...@chromium.org, devtools-re...@chromium.org, ipc-securi...@chromium.org, blink-revie...@chromium.org, dullweb...@chromium.org, msrame...@chromium.org, michaelpg+wa...@chromium.org, kuragin+web-ap...@chromium.org, dominickn+watch-...@chromium.org, alancutter...@chromium.org, mgiuca...@chromium.org, philli...@chromium.org, loyso...@chromium.org, ericwillige...@chromium.org, dmurph+watc...@chromium.org, dibyapal+wa...@chromium.org, webap...@microsoft.com, zelin+watch-we...@chromium.org, android-web...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, blundell+...@chromium.org, dominickn+wat...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org
    Attention needed from Andrey Kosyakov

    Mike Wasserman voted and added 2 comments

    Votes added by Mike Wasserman

    Commit-Queue+1

    2 comments

    Patchset-level comments
    File-level comment, Patchset 23 (Latest):
    Mike Wasserman . resolved

    caseq: please take another look, thanks!

    File content/browser/devtools/protocol/browser_handler.cc
    Line 196, Patchset 20: "Fullscreen Permission only supports allowWithoutGesture:true");
    Andrey Kosyakov . unresolved

    That's the opposite of what you're actually checking though.

    Mike Wasserman

    Sorry, can you clarify? The intent here is to get the caller-specified `allowWithoutGesture`, with a `false` default value, even though `false` is not supported. i.e. the caller must specify `allowWithoutGesture: true`.

    Andrey Kosyakov

    The intent sounds good, however `descriptor->GetAllowWithoutGesture(false)` will return true in case `allowWithoutGesture: true` is specified, and you appear to return an error just in this case.
    Which convenient brings us to the subject of test coverage for this code ;-)

    Andrey Kosyakov

    *conveniently

    Mike Wasserman

    Good catch! I fixed the inverted check, added browser-set-permission.js coverage, and added a WPT, although test_driver.set_permission apparently does not exercise this codepath. Please take another look and lmk if you recommend following any other test coverage patterns for this new permission descriptor, thanks.

    Open in Gerrit

    Related details

    Attention is currently required from:
    • Andrey Kosyakov
    Submit Requirements:
    • requirement satisfiedCode-Coverage
    • requirement is not satisfiedCode-Review
    • requirement is not satisfiedNo-Unresolved-Comments
    Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
    Gerrit-MessageType: comment
    Gerrit-Project: chromium/src
    Gerrit-Branch: main
    Gerrit-Change-Id: If9adac187ef712a9064855055953d3882305bf05
    Gerrit-Change-Number: 5583182
    Gerrit-PatchSet: 23
    Gerrit-Owner: Mike Wasserman <m...@chromium.org>
    Gerrit-Reviewer: Andrey Kosyakov <ca...@chromium.org>
    Gerrit-Reviewer: Bo Liu <bo...@chromium.org>
    Gerrit-Reviewer: Elias Klim <el...@chromium.org>
    Gerrit-Reviewer: Mike Wasserman <m...@chromium.org>
    Gerrit-Reviewer: Mike West <mk...@chromium.org>
    Gerrit-CC: Brad Triebwasser <btr...@chromium.org>
    Gerrit-CC: Kentaro Hara <har...@chromium.org>
    Gerrit-CC: Permissions Reviews <permissio...@chromium.org>
    Gerrit-CC: Peter Beverloo <pe...@chromium.org>
    Gerrit-CC: Takumi Fujimoto <tak...@chromium.org>
    Gerrit-Attention: Andrey Kosyakov <ca...@chromium.org>
    Gerrit-Comment-Date: Mon, 17 Jun 2024 22:50:57 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: Yes
    satisfied_requirement
    unsatisfied_requirement
    open
    diffy

    Andrey Kosyakov (Gerrit)

    unread,
    Jun 17, 2024, 6:55:11 PM (12 days ago) Jun 17
    to Mike Wasserman, Bo Liu, Mike West, Elias Klim, Brad Triebwasser, Kentaro Hara, AyeAye, Chromium LUCI CQ, Takumi Fujimoto, chromium...@chromium.org, devtools...@chromium.org, Permissions Reviews, Peter Beverloo, blink-revie...@chromium.org, devtools-re...@chromium.org, ipc-securi...@chromium.org, blink-revie...@chromium.org, dullweb...@chromium.org, msrame...@chromium.org, michaelpg+wa...@chromium.org, kuragin+web-ap...@chromium.org, dominickn+watch-...@chromium.org, alancutter...@chromium.org, mgiuca...@chromium.org, philli...@chromium.org, loyso...@chromium.org, ericwillige...@chromium.org, dmurph+watc...@chromium.org, dibyapal+wa...@chromium.org, webap...@microsoft.com, zelin+watch-we...@chromium.org, android-web...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, blundell+...@chromium.org, dominickn+wat...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org
    Attention needed from Mike Wasserman

    Andrey Kosyakov voted and added 2 comments

    Votes added by Andrey Kosyakov

    Code-Review+1

    2 comments

    Patchset-level comments
    Andrey Kosyakov . resolved

    Thanks, devtools/ lgtm!

    File third_party/blink/public/devtools_protocol/browser_protocol.pdl
    Line 1366, Patchset 20: # For "fullscreen" permission, may specify allowWithoutGesture.
    Andrey Kosyakov . unresolved

    Looks more like "must" on the implementation side to me ;-)

    Mike Wasserman

    Indeed, the false default value is not supported for now. I can update the comment if appropriate.

    Andrey Kosyakov

    Yep, let's document that this is required for this permission.

    Open in Gerrit

    Related details

    Attention is currently required from:
    • Mike Wasserman
    Submit Requirements:
    • requirement satisfiedCode-Coverage
    • requirement satisfiedCode-Review
    • requirement is not satisfiedNo-Unresolved-Comments
    Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
    Gerrit-MessageType: comment
    Gerrit-Project: chromium/src
    Gerrit-Branch: main
    Gerrit-Change-Id: If9adac187ef712a9064855055953d3882305bf05
    Gerrit-Change-Number: 5583182
    Gerrit-PatchSet: 23
    Gerrit-Owner: Mike Wasserman <m...@chromium.org>
    Gerrit-Reviewer: Andrey Kosyakov <ca...@chromium.org>
    Gerrit-Reviewer: Bo Liu <bo...@chromium.org>
    Gerrit-Reviewer: Elias Klim <el...@chromium.org>
    Gerrit-Reviewer: Mike Wasserman <m...@chromium.org>
    Gerrit-Reviewer: Mike West <mk...@chromium.org>
    Gerrit-CC: Brad Triebwasser <btr...@chromium.org>
    Gerrit-CC: Kentaro Hara <har...@chromium.org>
    Gerrit-CC: Permissions Reviews <permissio...@chromium.org>
    Gerrit-CC: Peter Beverloo <pe...@chromium.org>
    Gerrit-CC: Takumi Fujimoto <tak...@chromium.org>
    Gerrit-Attention: Mike Wasserman <m...@chromium.org>
    Gerrit-Comment-Date: Mon, 17 Jun 2024 22:54:58 +0000
    satisfied_requirement
    unsatisfied_requirement
    open
    diffy

    Mike Wasserman (Gerrit)

    unread,
    Jun 17, 2024, 7:50:32 PM (12 days ago) Jun 17
    to Brad Triebwasser, Andrey Kosyakov, Bo Liu, Mike West, Elias Klim, Kentaro Hara, AyeAye, Chromium LUCI CQ, Takumi Fujimoto, chromium...@chromium.org, devtools...@chromium.org, Permissions Reviews, Peter Beverloo, blink-revie...@chromium.org, devtools-re...@chromium.org, ipc-securi...@chromium.org, blink-revie...@chromium.org, dullweb...@chromium.org, msrame...@chromium.org, michaelpg+wa...@chromium.org, kuragin+web-ap...@chromium.org, dominickn+watch-...@chromium.org, alancutter...@chromium.org, mgiuca...@chromium.org, philli...@chromium.org, loyso...@chromium.org, ericwillige...@chromium.org, dmurph+watc...@chromium.org, dibyapal+wa...@chromium.org, webap...@microsoft.com, zelin+watch-we...@chromium.org, android-web...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, blundell+...@chromium.org, dominickn+wat...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org
    Attention needed from Bo Liu, Elias Klim and Mike West

    Mike Wasserman added 4 comments

    Patchset-level comments
    File-level comment, Patchset 25 (Latest):
    Mike Wasserman . resolved

    Thanks, caseq! Owners, please take another look (I think your CR+1s were reset?):
    elklm: overall (just minor diffs since your PS19 CR+1 was reset).
    boliu: aw_permission_manager.cc
    mkwst: shell_permission_manager.cc, permission.mojom, runtime_enabled_features.json5
    Thanks everyone!

    File content/browser/devtools/protocol/browser_handler.cc
    Line 193, Patchset 20: if (descriptor->GetAllowWithoutGesture(false)) {
    Andrey Kosyakov . resolved

    Is there a point in requiring this dance on part of the client? Can we just imply `allowWithoutGesture: false` here, or is 'allowWithoutGesture: true` coming in the future?

    Mike Wasserman

    `allowWithoutGesture: true` is the only supported descriptor for now.

    Mike Wasserman

    Acknowledged

    Line 196, Patchset 20: "Fullscreen Permission only supports allowWithoutGesture:true");
    Andrey Kosyakov . resolved

    That's the opposite of what you're actually checking though.

    Mike Wasserman

    Sorry, can you clarify? The intent here is to get the caller-specified `allowWithoutGesture`, with a `false` default value, even though `false` is not supported. i.e. the caller must specify `allowWithoutGesture: true`.

    Andrey Kosyakov

    The intent sounds good, however `descriptor->GetAllowWithoutGesture(false)` will return true in case `allowWithoutGesture: true` is specified, and you appear to return an error just in this case.
    Which convenient brings us to the subject of test coverage for this code ;-)

    Andrey Kosyakov

    *conveniently

    Mike Wasserman

    Good catch! I fixed the inverted check, added browser-set-permission.js coverage, and added a WPT, although test_driver.set_permission apparently does not exercise this codepath. Please take another look and lmk if you recommend following any other test coverage patterns for this new permission descriptor, thanks.

    Mike Wasserman

    Acknowledged

    File third_party/blink/public/devtools_protocol/browser_protocol.pdl
    Line 1366, Patchset 20: # For "fullscreen" permission, may specify allowWithoutGesture.
    Andrey Kosyakov . resolved

    Looks more like "must" on the implementation side to me ;-)

    Mike Wasserman

    Indeed, the false default value is not supported for now. I can update the comment if appropriate.

    Andrey Kosyakov

    Yep, let's document that this is required for this permission.

    Mike Wasserman

    Done

    Open in Gerrit

    Related details

    Attention is currently required from:
    • Bo Liu
    • Elias Klim
    • Mike West
    Submit Requirements:
    • requirement satisfiedCode-Coverage
    • requirement satisfiedCode-Review
    Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
    Gerrit-MessageType: comment
    Gerrit-Project: chromium/src
    Gerrit-Branch: main
    Gerrit-Change-Id: If9adac187ef712a9064855055953d3882305bf05
    Gerrit-Change-Number: 5583182
    Gerrit-PatchSet: 25
    Gerrit-Owner: Mike Wasserman <m...@chromium.org>
    Gerrit-Reviewer: Andrey Kosyakov <ca...@chromium.org>
    Gerrit-Reviewer: Bo Liu <bo...@chromium.org>
    Gerrit-Reviewer: Brad Triebwasser <btr...@chromium.org>
    Gerrit-Reviewer: Elias Klim <el...@chromium.org>
    Gerrit-Reviewer: Mike Wasserman <m...@chromium.org>
    Gerrit-Reviewer: Mike West <mk...@chromium.org>
    Gerrit-CC: Kentaro Hara <har...@chromium.org>
    Gerrit-CC: Permissions Reviews <permissio...@chromium.org>
    Gerrit-CC: Peter Beverloo <pe...@chromium.org>
    Gerrit-CC: Takumi Fujimoto <tak...@chromium.org>
    Gerrit-Attention: Bo Liu <bo...@chromium.org>
    Gerrit-Attention: Mike West <mk...@chromium.org>
    Gerrit-Attention: Elias Klim <el...@chromium.org>
    Gerrit-Comment-Date: Mon, 17 Jun 2024 23:50:21 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: No
    satisfied_requirement
    open
    diffy

    Mike Wasserman (Gerrit)

    unread,
    Jun 17, 2024, 7:59:32 PM (12 days ago) Jun 17
    to Brad Triebwasser, Andrey Kosyakov, Bo Liu, Mike West, Elias Klim, Kentaro Hara, AyeAye, Chromium LUCI CQ, Takumi Fujimoto, chromium...@chromium.org, devtools...@chromium.org, Permissions Reviews, Peter Beverloo, blink-revie...@chromium.org, devtools-re...@chromium.org, ipc-securi...@chromium.org, blink-revie...@chromium.org, dullweb...@chromium.org, msrame...@chromium.org, michaelpg+wa...@chromium.org, kuragin+web-ap...@chromium.org, dominickn+watch-...@chromium.org, alancutter...@chromium.org, mgiuca...@chromium.org, philli...@chromium.org, loyso...@chromium.org, ericwillige...@chromium.org, dmurph+watc...@chromium.org, dibyapal+wa...@chromium.org, webap...@microsoft.com, zelin+watch-we...@chromium.org, android-web...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, blundell+...@chromium.org, dominickn+wat...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org
    Attention needed from Mike West

    Mike Wasserman added 1 comment

    Patchset-level comments
    Mike Wasserman . resolved

    Thanks, caseq! Owners, please take another look (I think your CR+1s were reset?):
    elklm: overall (just minor diffs since your PS19 CR+1 was reset).
    boliu: aw_permission_manager.cc
    mkwst: shell_permission_manager.cc, permission.mojom, runtime_enabled_features.json5
    Thanks everyone!

    Mike Wasserman

    TIL Code-Owners state isn't cleared when CR+1s are reset.
    I'm satisfied with just mkwst's re-review; removing others from attention set.

    Open in Gerrit

    Related details

    Attention is currently required from:
    • Mike West
    Gerrit-Attention: Mike West <mk...@chromium.org>
    Gerrit-Comment-Date: Mon, 17 Jun 2024 23:59:19 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: No
    Comment-In-Reply-To: Mike Wasserman <m...@chromium.org>
    satisfied_requirement
    open
    diffy

    Blink W3C Test Autoroller (Gerrit)

    unread,
    Jun 17, 2024, 8:00:40 PM (12 days ago) Jun 17
    to Mike Wasserman, Brad Triebwasser, Andrey Kosyakov, Bo Liu, Mike West, Elias Klim, Kentaro Hara, AyeAye, Chromium LUCI CQ, Takumi Fujimoto, chromium...@chromium.org, devtools...@chromium.org, Permissions Reviews, Peter Beverloo, blink-revie...@chromium.org, devtools-re...@chromium.org, ipc-securi...@chromium.org, blink-revie...@chromium.org, dullweb...@chromium.org, msrame...@chromium.org, michaelpg+wa...@chromium.org, kuragin+web-ap...@chromium.org, dominickn+watch-...@chromium.org, alancutter...@chromium.org, mgiuca...@chromium.org, philli...@chromium.org, loyso...@chromium.org, ericwillige...@chromium.org, dmurph+watc...@chromium.org, dibyapal+wa...@chromium.org, webap...@microsoft.com, zelin+watch-we...@chromium.org, android-web...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, blundell+...@chromium.org, dominickn+wat...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org
    Attention needed from Mike West

    Message from Blink W3C Test Autoroller

    Exportable changes to web-platform-tests were detected in this CL and a pull request in the upstream repo has been made: https://github.com/web-platform-tests/wpt/pull/46804.

    When this CL lands, the bot will automatically merge the PR on GitHub if the required GitHub checks pass; otherwise, ecosystem-infra@ team will triage the failures and may contact you.

    WPT Export docs:
    https://chromium.googlesource.com/chromium/src/+/main/docs/testing/web_platform_tests.md#Automatic-export-process

    Open in Gerrit

    Related details

    Attention is currently required from:
    • Mike West
    Submit Requirements:
    • requirement satisfiedCode-Coverage
    • requirement satisfiedCode-Review
    Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
    Gerrit-MessageType: comment
    Gerrit-Project: chromium/src
    Gerrit-Branch: main
    Gerrit-Change-Id: If9adac187ef712a9064855055953d3882305bf05
    Gerrit-Change-Number: 5583182
    Gerrit-PatchSet: 25
    Gerrit-Owner: Mike Wasserman <m...@chromium.org>
    Gerrit-Reviewer: Andrey Kosyakov <ca...@chromium.org>
    Gerrit-Reviewer: Bo Liu <bo...@chromium.org>
    Gerrit-Reviewer: Brad Triebwasser <btr...@chromium.org>
    Gerrit-Reviewer: Elias Klim <el...@chromium.org>
    Gerrit-Reviewer: Mike Wasserman <m...@chromium.org>
    Gerrit-Reviewer: Mike West <mk...@chromium.org>
    Gerrit-CC: Blink W3C Test Autoroller <blink-w3c-te...@chromium.org>
    Gerrit-CC: Kentaro Hara <har...@chromium.org>
    Gerrit-CC: Permissions Reviews <permissio...@chromium.org>
    Gerrit-CC: Peter Beverloo <pe...@chromium.org>
    Gerrit-CC: Takumi Fujimoto <tak...@chromium.org>
    Gerrit-Attention: Mike West <mk...@chromium.org>
    Gerrit-Comment-Date: Tue, 18 Jun 2024 00:00:20 +0000
    Gerrit-HasComments: No
    Gerrit-Has-Labels: No
    satisfied_requirement
    open
    diffy

    Elias Klim (Gerrit)

    unread,
    Jun 18, 2024, 6:09:49 AM (11 days ago) Jun 18
    to Mike Wasserman, Blink W3C Test Autoroller, Brad Triebwasser, Andrey Kosyakov, Bo Liu, Mike West, Kentaro Hara, AyeAye, Chromium LUCI CQ, Takumi Fujimoto, chromium...@chromium.org, devtools...@chromium.org, Permissions Reviews, Peter Beverloo, blink-revie...@chromium.org, devtools-re...@chromium.org, ipc-securi...@chromium.org, blink-revie...@chromium.org, dullweb...@chromium.org, msrame...@chromium.org, michaelpg+wa...@chromium.org, kuragin+web-ap...@chromium.org, dominickn+watch-...@chromium.org, alancutter...@chromium.org, mgiuca...@chromium.org, philli...@chromium.org, loyso...@chromium.org, ericwillige...@chromium.org, dmurph+watc...@chromium.org, dibyapal+wa...@chromium.org, webap...@microsoft.com, zelin+watch-we...@chromium.org, android-web...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, blundell+...@chromium.org, dominickn+wat...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org
    Attention needed from Mike Wasserman and Mike West

    Elias Klim voted Code-Review+1

    Code-Review+1
    Open in Gerrit

    Related details

    Attention is currently required from:
    • Mike Wasserman
    • Mike West
    Gerrit-Attention: Mike Wasserman <m...@chromium.org>
    Gerrit-Comment-Date: Tue, 18 Jun 2024 10:09:38 +0000
    Gerrit-HasComments: No
    Gerrit-Has-Labels: Yes
    satisfied_requirement
    open
    diffy

    Mike West (Gerrit)

    unread,
    Jun 19, 2024, 3:19:00 AM (11 days ago) Jun 19
    to Mike Wasserman, Elias Klim, Blink W3C Test Autoroller, Brad Triebwasser, Andrey Kosyakov, Bo Liu, Kentaro Hara, AyeAye, Chromium LUCI CQ, Takumi Fujimoto, chromium...@chromium.org, devtools...@chromium.org, Permissions Reviews, Peter Beverloo, blink-revie...@chromium.org, devtools-re...@chromium.org, ipc-securi...@chromium.org, blink-revie...@chromium.org, dullweb...@chromium.org, msrame...@chromium.org, michaelpg+wa...@chromium.org, kuragin+web-ap...@chromium.org, dominickn+watch-...@chromium.org, alancutter...@chromium.org, mgiuca...@chromium.org, philli...@chromium.org, loyso...@chromium.org, ericwillige...@chromium.org, dmurph+watc...@chromium.org, dibyapal+wa...@chromium.org, webap...@microsoft.com, zelin+watch-we...@chromium.org, android-web...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, blundell+...@chromium.org, dominickn+wat...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org
    Attention needed from Mike Wasserman

    Mike West added 1 comment

    Patchset-level comments
    Mike West . resolved

    I'll defer to Elias on the shape of the permissions integration. The implementation of that shape in mojo, //content/shell, and //third_party/blink LGTM.

    Open in Gerrit

    Related details

    Attention is currently required from:
    • Mike Wasserman
    Gerrit-Attention: Mike Wasserman <m...@chromium.org>
    Gerrit-Comment-Date: Wed, 19 Jun 2024 07:18:46 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: No
    satisfied_requirement
    open
    diffy

    Mike West (Gerrit)

    unread,
    Jun 19, 2024, 3:19:08 AM (11 days ago) Jun 19
    to Mike Wasserman, Elias Klim, Blink W3C Test Autoroller, Brad Triebwasser, Andrey Kosyakov, Bo Liu, Kentaro Hara, AyeAye, Chromium LUCI CQ, Takumi Fujimoto, chromium...@chromium.org, devtools...@chromium.org, Permissions Reviews, Peter Beverloo, blink-revie...@chromium.org, devtools-re...@chromium.org, ipc-securi...@chromium.org, blink-revie...@chromium.org, dullweb...@chromium.org, msrame...@chromium.org, michaelpg+wa...@chromium.org, kuragin+web-ap...@chromium.org, dominickn+watch-...@chromium.org, alancutter...@chromium.org, mgiuca...@chromium.org, philli...@chromium.org, loyso...@chromium.org, ericwillige...@chromium.org, dmurph+watc...@chromium.org, dibyapal+wa...@chromium.org, webap...@microsoft.com, zelin+watch-we...@chromium.org, android-web...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, blundell+...@chromium.org, dominickn+wat...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org
    Attention needed from Mike Wasserman

    Mike West voted Code-Review+1

    Code-Review+1
    Gerrit-Comment-Date: Wed, 19 Jun 2024 07:18:53 +0000
    Gerrit-HasComments: No
    Gerrit-Has-Labels: Yes
    satisfied_requirement
    open
    diffy

    Mike Wasserman (Gerrit)

    unread,
    Jun 19, 2024, 2:31:04 PM (10 days ago) Jun 19
    to Mike West, Elias Klim, Blink W3C Test Autoroller, Brad Triebwasser, Andrey Kosyakov, Bo Liu, Kentaro Hara, AyeAye, Chromium LUCI CQ, Takumi Fujimoto, chromium...@chromium.org, devtools...@chromium.org, Permissions Reviews, Peter Beverloo, blink-revie...@chromium.org, devtools-re...@chromium.org, ipc-securi...@chromium.org, blink-revie...@chromium.org, dullweb...@chromium.org, msrame...@chromium.org, michaelpg+wa...@chromium.org, kuragin+web-ap...@chromium.org, dominickn+watch-...@chromium.org, alancutter...@chromium.org, mgiuca...@chromium.org, philli...@chromium.org, loyso...@chromium.org, ericwillige...@chromium.org, dmurph+watc...@chromium.org, dibyapal+wa...@chromium.org, webap...@microsoft.com, zelin+watch-we...@chromium.org, android-web...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, blundell+...@chromium.org, dominickn+wat...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org

    Mike Wasserman voted Commit-Queue+2

    Commit-Queue+2
    Open in Gerrit

    Related details

    Attention set is empty
    Gerrit-Comment-Date: Wed, 19 Jun 2024 18:30:49 +0000
    Gerrit-HasComments: No
    Gerrit-Has-Labels: Yes
    satisfied_requirement
    open
    diffy

    Chromium LUCI CQ (Gerrit)

    unread,
    Jun 19, 2024, 3:39:55 PM (10 days ago) Jun 19
    to Mike Wasserman, Mike West, Elias Klim, Blink W3C Test Autoroller, Brad Triebwasser, Andrey Kosyakov, Bo Liu, Kentaro Hara, AyeAye, Takumi Fujimoto, chromium...@chromium.org, devtools...@chromium.org, Permissions Reviews, Peter Beverloo, blink-revie...@chromium.org, devtools-re...@chromium.org, ipc-securi...@chromium.org, blink-revie...@chromium.org, dullweb...@chromium.org, msrame...@chromium.org, michaelpg+wa...@chromium.org, kuragin+web-ap...@chromium.org, dominickn+watch-...@chromium.org, alancutter...@chromium.org, mgiuca...@chromium.org, philli...@chromium.org, loyso...@chromium.org, ericwillige...@chromium.org, dmurph+watc...@chromium.org, dibyapal+wa...@chromium.org, webap...@microsoft.com, zelin+watch-we...@chromium.org, android-web...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, blundell+...@chromium.org, dominickn+wat...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org

    Chromium LUCI CQ submitted the change

    Change information

    Commit message:
    Automatic Fullscreen: Prototype Permissions API query support

    Add navigator.permissions.query support for a new descriptor:
      {name:'fullscreen', allowWithoutGesture:true}
    Gate on RunTimeEnabledFeature AutomaticFullscreenPermissionsQuery.

    Enhances feature detection and debuggability, per guidance:
    https://groups.google.com/a/chromium.org/g/blink-dev/c/WOch5LPq9RY

    Yield `TypeError` for allowWithoutGesture:false, like unknown names.
    Descriptor supports potential future fullscreen permission iteration.
    http://doc/1ojUXRUcciyjxAgXOuTZoYREzI4Xp_dUd9NiFlYJxMew

    Associate permission with `fullscreen` permissions policy feature.
    (yield 'denied' for frames without `fullscreen` permissions policy)

    Add interactive_ui_tests, tentative WPT, and inspector-protocol tests.
    Refine supporting test functionality.

    https://chromestatus.com/feature/6218822004768768
    https://github.com/explainers-by-googlers/html-fullscreen-without-a-gesture
    http://go/automatic-fullscreen-content-setting
    Bug: 40941384
    Test: Automated; Permissions API query WAI with flag enabled.
    Change-Id: If9adac187ef712a9064855055953d3882305bf05
    Commit-Queue: Mike Wasserman <m...@chromium.org>
    Reviewed-by: Elias Klim <el...@chromium.org>
    Reviewed-by: Mike West <mk...@chromium.org>
    Reviewed-by: Andrey Kosyakov <ca...@chromium.org>
    Cr-Commit-Position: refs/heads/main@{#1317128}
    Files:
    • M android_webview/browser/aw_permission_manager.cc
    • M chrome/browser/permissions/permission_manager_factory.cc
    • M chrome/browser/ui/exclusive_access/fullscreen_controller_interactive_browsertest.cc
    • M chrome/browser/ui/web_applications/test/isolated_web_app_test_utils.cc
    • M chrome/browser/ui/webui/settings/site_settings_helper_unittest.cc
    • M components/permissions/BUILD.gn
    • A components/permissions/contexts/automatic_fullscreen_permission_context.cc
    • A components/permissions/contexts/automatic_fullscreen_permission_context.h
    • M components/permissions/permission_util.cc
    • M components/permissions/test/permission_test_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/renderer/bindings/generated_in_modules.gni
    • M third_party/blink/renderer/bindings/idl_in_modules.gni
    • A third_party/blink/renderer/modules/permissions/fullscreen_permission_descriptor.idl
    • 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/modules/permissions/permission_utils.h
    • M third_party/blink/renderer/modules/permissions/permissions.cc
    • M third_party/blink/renderer/platform/runtime_enabled_features.json5
    • A third_party/blink/web_tests/external/wpt/fullscreen/api/permission.tentative.https.html
    • M third_party/blink/web_tests/http/tests/inspector-protocol/browser-set-permission-expected.txt
    • M third_party/blink/web_tests/http/tests/inspector-protocol/browser-set-permission.js
    Change size: L
    Delta: 28 files changed, 305 insertions(+), 24 deletions(-)
    Branch: refs/heads/main
    Submit Requirements:
    • requirement satisfiedCode-Review: +1 by Andrey Kosyakov, +1 by Mike West, +1 by Elias Klim
    Open in Gerrit
    Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
    Gerrit-MessageType: merged
    Gerrit-Project: chromium/src
    Gerrit-Branch: main
    Gerrit-Change-Id: If9adac187ef712a9064855055953d3882305bf05
    Gerrit-Change-Number: 5583182
    Gerrit-PatchSet: 26
    Gerrit-Owner: Mike Wasserman <m...@chromium.org>
    Gerrit-Reviewer: Andrey Kosyakov <ca...@chromium.org>
    Gerrit-Reviewer: Bo Liu <bo...@chromium.org>
    Gerrit-Reviewer: Brad Triebwasser <btr...@chromium.org>
    Gerrit-Reviewer: Chromium LUCI CQ <chromiu...@luci-project-accounts.iam.gserviceaccount.com>
    Gerrit-Reviewer: Elias Klim <el...@chromium.org>
    Gerrit-Reviewer: Mike Wasserman <m...@chromium.org>
    Gerrit-Reviewer: Mike West <mk...@chromium.org>
    Gerrit-CC: Blink W3C Test Autoroller <blink-w3c-te...@chromium.org>
    open
    diffy
    satisfied_requirement

    Blink W3C Test Autoroller (Gerrit)

    unread,
    Jun 19, 2024, 4:10:45 PM (10 days ago) Jun 19
    to Chromium LUCI CQ, Mike Wasserman, Mike West, Elias Klim, Brad Triebwasser, Andrey Kosyakov, Bo Liu, Kentaro Hara, AyeAye, Takumi Fujimoto, chromium...@chromium.org, devtools...@chromium.org, Permissions Reviews, Peter Beverloo, blink-revie...@chromium.org, devtools-re...@chromium.org, ipc-securi...@chromium.org, blink-revie...@chromium.org, dullweb...@chromium.org, msrame...@chromium.org, michaelpg+wa...@chromium.org, kuragin+web-ap...@chromium.org, dominickn+watch-...@chromium.org, alancutter...@chromium.org, mgiuca...@chromium.org, philli...@chromium.org, loyso...@chromium.org, ericwillige...@chromium.org, dmurph+watc...@chromium.org, dibyapal+wa...@chromium.org, webap...@microsoft.com, zelin+watch-we...@chromium.org, android-web...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, blundell+...@chromium.org, dominickn+wat...@chromium.org, jmedle...@chromium.org, kinuko...@chromium.org

    Message from Blink W3C Test Autoroller

    The WPT PR for this CL has been merged upstream! https://github.com/web-platform-tests/wpt/pull/46804

    Open in Gerrit

    Related details

    Attention set is empty
    Submit Requirements:
    • requirement satisfiedCode-Coverage
    • requirement satisfiedCode-Review
    Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
    Gerrit-MessageType: comment
    Gerrit-Project: chromium/src
    Gerrit-Branch: main
    Gerrit-Change-Id: If9adac187ef712a9064855055953d3882305bf05
    Gerrit-Change-Number: 5583182
    Gerrit-PatchSet: 26
    Gerrit-Owner: Mike Wasserman <m...@chromium.org>
    Gerrit-Reviewer: Andrey Kosyakov <ca...@chromium.org>
    Gerrit-Reviewer: Bo Liu <bo...@chromium.org>
    Gerrit-Reviewer: Brad Triebwasser <btr...@chromium.org>
    Gerrit-Reviewer: Chromium LUCI CQ <chromiu...@luci-project-accounts.iam.gserviceaccount.com>
    Gerrit-Reviewer: Elias Klim <el...@chromium.org>
    Gerrit-Reviewer: Mike Wasserman <m...@chromium.org>
    Gerrit-Reviewer: Mike West <mk...@chromium.org>
    Gerrit-CC: Blink W3C Test Autoroller <blink-w3c-te...@chromium.org>
    Gerrit-CC: Kentaro Hara <har...@chromium.org>
    Gerrit-CC: Permissions Reviews <permissio...@chromium.org>
    Gerrit-CC: Peter Beverloo <pe...@chromium.org>
    Gerrit-CC: Takumi Fujimoto <tak...@chromium.org>
    Gerrit-Comment-Date: Wed, 19 Jun 2024 20:10:36 +0000
    Gerrit-HasComments: No
    Gerrit-Has-Labels: No
    satisfied_requirement
    open
    diffy
    Reply all
    Reply to author
    Forward
    0 new messages