iwa: Ensure all tests work with IWA feature enabled [chromium/src : main]

0 views
Skip to first unread message

Robbie McElrath (Gerrit)

unread,
Jul 3, 2024, 2:05:47 PM (2 days ago) Jul 3
to Reilly Grant, Chung-sheng Wu, Sergii Bykov, Guido Urdaneta, Martin Šrámek, Dibyajyoti Pal, Chromium LUCI CQ, chromium...@chromium.org, Andrew Rayskiy, Rijubrata Bhaumik, Simon Hangl, alancutter...@chromium.org, blundell+...@chromium.org, chfreme...@chromium.org, chromium-a...@chromium.org, dibyapal+wa...@chromium.org, dmurph+watc...@chromium.org, dominickn+watch-...@chromium.org, ericwillige...@chromium.org, extension...@chromium.org, feature-me...@chromium.org, kuragin+web-ap...@chromium.org, loyso...@chromium.org, mattreyno...@chromium.org, mgiuca...@chromium.org, odejesu...@chromium.org, oshima...@chromium.org, philli...@chromium.org, webap...@microsoft.com, zelin+watch-we...@chromium.org
Attention needed from Chung-sheng Wu, Dibyajyoti Pal, Guido Urdaneta, Martin Šrámek, Reilly Grant and Sergii Bykov

Robbie McElrath added 1 comment

Patchset-level comments
File-level comment, Patchset 1 (Latest):
Robbie McElrath . resolved

Hello! Could you each please review the following paths?

reillyg: chrome/browser/chooser_controller/
chungsheng: chrome/browser/chromeos/extensions/telemetry/
sbykov: chrome/browser/device_api/
guidou: chrome/browser/media/
msramek: chrome/browser/ui/content_settings/
dibyapal: chrome/browser/web_applications/

Open in Gerrit

Related details

Attention is currently required from:
  • Chung-sheng Wu
  • Dibyajyoti Pal
  • Guido Urdaneta
  • Martin Šrámek
  • Reilly Grant
  • Sergii Bykov
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: I1ab9eabf130d85e8d9a65861c3036270ddc34e7c
Gerrit-Change-Number: 5673945
Gerrit-PatchSet: 1
Gerrit-Owner: Robbie McElrath <rmce...@chromium.org>
Gerrit-Reviewer: Chung-sheng Wu <chung...@google.com>
Gerrit-Reviewer: Dibyajyoti Pal <diby...@chromium.org>
Gerrit-Reviewer: Guido Urdaneta <gui...@chromium.org>
Gerrit-Reviewer: Martin Šrámek <msr...@chromium.org>
Gerrit-Reviewer: Reilly Grant <rei...@chromium.org>
Gerrit-Reviewer: Robbie McElrath <rmce...@chromium.org>
Gerrit-Reviewer: Sergii Bykov <sby...@google.com>
Gerrit-CC: Andrew Rayskiy <green...@google.com>
Gerrit-CC: Rijubrata Bhaumik <rijubrat...@intel.com>
Gerrit-CC: Simon Hangl <sim...@google.com>
Gerrit-Attention: Sergii Bykov <sby...@google.com>
Gerrit-Attention: Chung-sheng Wu <chung...@google.com>
Gerrit-Attention: Reilly Grant <rei...@chromium.org>
Gerrit-Attention: Martin Šrámek <msr...@chromium.org>
Gerrit-Attention: Dibyajyoti Pal <diby...@chromium.org>
Gerrit-Attention: Guido Urdaneta <gui...@chromium.org>
Gerrit-Comment-Date: Wed, 03 Jul 2024 18:05:37 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
satisfied_requirement
unsatisfied_requirement
open
diffy

Dibyajyoti Pal (Gerrit)

unread,
Jul 3, 2024, 2:42:52 PM (2 days ago) Jul 3
to Robbie McElrath, Reilly Grant, Chung-sheng Wu, Sergii Bykov, Guido Urdaneta, Martin Šrámek, Chromium LUCI CQ, chromium...@chromium.org, Andrew Rayskiy, Rijubrata Bhaumik, Simon Hangl, alancutter...@chromium.org, blundell+...@chromium.org, chfreme...@chromium.org, chromium-a...@chromium.org, dibyapal+wa...@chromium.org, dmurph+watc...@chromium.org, dominickn+watch-...@chromium.org, ericwillige...@chromium.org, extension...@chromium.org, feature-me...@chromium.org, kuragin+web-ap...@chromium.org, loyso...@chromium.org, mattreyno...@chromium.org, mgiuca...@chromium.org, odejesu...@chromium.org, oshima...@chromium.org, philli...@chromium.org, webap...@microsoft.com, zelin+watch-we...@chromium.org
Attention needed from Chung-sheng Wu, Guido Urdaneta, Martin Šrámek, Reilly Grant, Robbie McElrath and Sergii Bykov

Dibyajyoti Pal voted and added 2 comments

Votes added by Dibyajyoti Pal

Code-Review+1

2 comments

Patchset-level comments
Dibyajyoti Pal . resolved

LGTM % nit

File chrome/browser/web_applications/isolated_web_apps/isolated_web_app_features_unittest.cc
Line 47, Patchset 1 (Latest): base::test::ScopedFeatureList scoped_feature_list;
Dibyajyoti Pal . unresolved

subjective nit: Each of these use-cases could be small individual tests by itself, that way, it is easier to parse the `ScopedFeatureList` for readability to know which feature is being enabled and which is being disabled.

Open in Gerrit

Related details

Attention is currently required from:
  • Chung-sheng Wu
  • Guido Urdaneta
  • Martin Šrámek
  • Reilly Grant
  • Robbie McElrath
  • Sergii Bykov
Submit Requirements:
  • requirement satisfiedCode-Coverage
  • requirement satisfiedCode-Review
  • requirement is not satisfiedNo-Unresolved-Comments
Gerrit-Attention: Guido Urdaneta <gui...@chromium.org>
Gerrit-Attention: Robbie McElrath <rmce...@chromium.org>
Gerrit-Comment-Date: Wed, 03 Jul 2024 18:42:38 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
satisfied_requirement
unsatisfied_requirement
open
diffy

Martin Šrámek (Gerrit)

unread,
Jul 3, 2024, 2:44:08 PM (2 days ago) Jul 3
to Robbie McElrath, Reilly Grant, Chung-sheng Wu, Sergii Bykov, Guido Urdaneta, Chromium LUCI CQ, chromium...@chromium.org, Andrew Rayskiy, Rijubrata Bhaumik, Simon Hangl, alancutter...@chromium.org, blundell+...@chromium.org, chfreme...@chromium.org, chromium-a...@chromium.org, dibyapal+wa...@chromium.org, dmurph+watc...@chromium.org, dominickn+watch-...@chromium.org, ericwillige...@chromium.org, extension...@chromium.org, feature-me...@chromium.org, kuragin+web-ap...@chromium.org, loyso...@chromium.org, mattreyno...@chromium.org, mgiuca...@chromium.org, odejesu...@chromium.org, oshima...@chromium.org, philli...@chromium.org, webap...@microsoft.com, zelin+watch-we...@chromium.org
Attention needed from Chung-sheng Wu, Guido Urdaneta, Reilly Grant, Robbie McElrath and Sergii Bykov

Martin Šrámek voted and added 1 comment

Votes added by Martin Šrámek

Code-Review+1

1 comment

Patchset-level comments
Martin Šrámek . resolved

content_settings/ LGTM

Open in Gerrit

Related details

Attention is currently required from:
  • Chung-sheng Wu
  • Guido Urdaneta
Gerrit-Attention: Guido Urdaneta <gui...@chromium.org>
Gerrit-Attention: Robbie McElrath <rmce...@chromium.org>
Gerrit-Comment-Date: Wed, 03 Jul 2024 18:43:58 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
satisfied_requirement
unsatisfied_requirement
open
diffy

Guido Urdaneta (Gerrit)

unread,
Jul 3, 2024, 2:54:22 PM (2 days ago) Jul 3
to Robbie McElrath, Martin Šrámek, Reilly Grant, Chung-sheng Wu, Sergii Bykov, Chromium LUCI CQ, chromium...@chromium.org, Andrew Rayskiy, Rijubrata Bhaumik, Simon Hangl, alancutter...@chromium.org, blundell+...@chromium.org, chfreme...@chromium.org, chromium-a...@chromium.org, dibyapal+wa...@chromium.org, dmurph+watc...@chromium.org, dominickn+watch-...@chromium.org, ericwillige...@chromium.org, extension...@chromium.org, feature-me...@chromium.org, kuragin+web-ap...@chromium.org, loyso...@chromium.org, mattreyno...@chromium.org, mgiuca...@chromium.org, odejesu...@chromium.org, oshima...@chromium.org, philli...@chromium.org, webap...@microsoft.com, zelin+watch-we...@chromium.org
Attention needed from Chung-sheng Wu, Reilly Grant, Robbie McElrath and Sergii Bykov

Guido Urdaneta voted Code-Review+1

Code-Review+1
Open in Gerrit

Related details

Attention is currently required from:
  • Chung-sheng Wu
Gerrit-Attention: Robbie McElrath <rmce...@chromium.org>
Gerrit-Comment-Date: Wed, 03 Jul 2024 18:54:07 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
satisfied_requirement
unsatisfied_requirement
open
diffy

Robbie McElrath (Gerrit)

unread,
Jul 3, 2024, 4:12:47 PM (2 days ago) Jul 3
to Guido Urdaneta, Martin Šrámek, Reilly Grant, Chung-sheng Wu, Sergii Bykov, Chromium LUCI CQ, chromium...@chromium.org, Andrew Rayskiy, Rijubrata Bhaumik, Simon Hangl, alancutter...@chromium.org, blundell+...@chromium.org, chfreme...@chromium.org, chromium-a...@chromium.org, dibyapal+wa...@chromium.org, dmurph+watc...@chromium.org, dominickn+watch-...@chromium.org, ericwillige...@chromium.org, extension...@chromium.org, feature-me...@chromium.org, kuragin+web-ap...@chromium.org, loyso...@chromium.org, mattreyno...@chromium.org, mgiuca...@chromium.org, odejesu...@chromium.org, oshima...@chromium.org, philli...@chromium.org, webap...@microsoft.com, zelin+watch-we...@chromium.org
Attention needed from Chung-sheng Wu, Reilly Grant and Sergii Bykov

Robbie McElrath voted and added 1 comment

Votes added by Robbie McElrath

Commit-Queue+1

1 comment

File chrome/browser/web_applications/isolated_web_apps/isolated_web_app_features_unittest.cc
Line 47, Patchset 1: base::test::ScopedFeatureList scoped_feature_list;
Dibyajyoti Pal . resolved

subjective nit: Each of these use-cases could be small individual tests by itself, that way, it is easier to parse the `ScopedFeatureList` for readability to know which feature is being enabled and which is being disabled.

Robbie McElrath

Good idea, done

Open in Gerrit

Related details

Attention is currently required from:
  • Chung-sheng Wu
  • Reilly Grant
  • Sergii Bykov
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: I1ab9eabf130d85e8d9a65861c3036270ddc34e7c
Gerrit-Change-Number: 5673945
Gerrit-PatchSet: 2
Gerrit-Owner: Robbie McElrath <rmce...@chromium.org>
Gerrit-Reviewer: Chung-sheng Wu <chung...@google.com>
Gerrit-Reviewer: Dibyajyoti Pal <diby...@chromium.org>
Gerrit-Reviewer: Guido Urdaneta <gui...@chromium.org>
Gerrit-Reviewer: Martin Šrámek <msr...@chromium.org>
Gerrit-Reviewer: Reilly Grant <rei...@chromium.org>
Gerrit-Reviewer: Robbie McElrath <rmce...@chromium.org>
Gerrit-Reviewer: Sergii Bykov <sby...@google.com>
Gerrit-CC: Andrew Rayskiy <green...@google.com>
Gerrit-CC: Rijubrata Bhaumik <rijubrat...@intel.com>
Gerrit-CC: Simon Hangl <sim...@google.com>
Gerrit-Attention: Sergii Bykov <sby...@google.com>
Gerrit-Attention: Chung-sheng Wu <chung...@google.com>
Gerrit-Attention: Reilly Grant <rei...@chromium.org>
Gerrit-Comment-Date: Wed, 03 Jul 2024 20:12:33 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: Dibyajyoti Pal <diby...@chromium.org>
satisfied_requirement
open
diffy

Reilly Grant (Gerrit)

unread,
Jul 3, 2024, 4:48:42 PM (2 days ago) Jul 3
to Robbie McElrath, Guido Urdaneta, Martin Šrámek, Reilly Grant, Chung-sheng Wu, Sergii Bykov, Chromium LUCI CQ, chromium...@chromium.org, Andrew Rayskiy, Rijubrata Bhaumik, Simon Hangl, alancutter...@chromium.org, blundell+...@chromium.org, chfreme...@chromium.org, chromium-a...@chromium.org, dibyapal+wa...@chromium.org, dmurph+watc...@chromium.org, dominickn+watch-...@chromium.org, ericwillige...@chromium.org, extension...@chromium.org, feature-me...@chromium.org, kuragin+web-ap...@chromium.org, loyso...@chromium.org, mattreyno...@chromium.org, mgiuca...@chromium.org, odejesu...@chromium.org, oshima...@chromium.org, philli...@chromium.org, webap...@microsoft.com, zelin+watch-we...@chromium.org
Attention needed from Chung-sheng Wu, Robbie McElrath and Sergii Bykov

Reilly Grant added 1 comment

File chrome/browser/device_api/device_service_unittest.cc
Line 176, Patchset 2 (Latest):#if BUILDFLAG(IS_CHROMEOS)
Reilly Grant . unresolved

Why is this ChromeOS-specific?

Open in Gerrit

Related details

Attention is currently required from:
  • Chung-sheng Wu
  • Robbie McElrath
  • Sergii Bykov
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: I1ab9eabf130d85e8d9a65861c3036270ddc34e7c
    Gerrit-Change-Number: 5673945
    Gerrit-PatchSet: 2
    Gerrit-Owner: Robbie McElrath <rmce...@chromium.org>
    Gerrit-Reviewer: Chung-sheng Wu <chung...@google.com>
    Gerrit-Reviewer: Dibyajyoti Pal <diby...@chromium.org>
    Gerrit-Reviewer: Guido Urdaneta <gui...@chromium.org>
    Gerrit-Reviewer: Martin Šrámek <msr...@chromium.org>
    Gerrit-Reviewer: Reilly Grant <rei...@chromium.org>
    Gerrit-Reviewer: Robbie McElrath <rmce...@chromium.org>
    Gerrit-Reviewer: Sergii Bykov <sby...@google.com>
    Gerrit-CC: Andrew Rayskiy <green...@google.com>
    Gerrit-CC: Rijubrata Bhaumik <rijubrat...@intel.com>
    Gerrit-CC: Simon Hangl <sim...@google.com>
    Gerrit-Attention: Sergii Bykov <sby...@google.com>
    Gerrit-Attention: Chung-sheng Wu <chung...@google.com>
    Gerrit-Attention: Robbie McElrath <rmce...@chromium.org>
    Gerrit-Comment-Date: Wed, 03 Jul 2024 20:48:29 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: No
    satisfied_requirement
    unsatisfied_requirement
    open
    diffy

    Robbie McElrath (Gerrit)

    unread,
    Jul 3, 2024, 5:39:26 PM (2 days ago) Jul 3
    to Guido Urdaneta, Martin Šrámek, Reilly Grant, Chung-sheng Wu, Sergii Bykov, Chromium LUCI CQ, chromium...@chromium.org, Andrew Rayskiy, Rijubrata Bhaumik, Simon Hangl, alancutter...@chromium.org, blundell+...@chromium.org, chfreme...@chromium.org, chromium-a...@chromium.org, dibyapal+wa...@chromium.org, dmurph+watc...@chromium.org, dominickn+watch-...@chromium.org, ericwillige...@chromium.org, extension...@chromium.org, feature-me...@chromium.org, kuragin+web-ap...@chromium.org, loyso...@chromium.org, mattreyno...@chromium.org, mgiuca...@chromium.org, odejesu...@chromium.org, oshima...@chromium.org, philli...@chromium.org, webap...@microsoft.com, zelin+watch-we...@chromium.org
    Attention needed from Chung-sheng Wu, Reilly Grant and Sergii Bykov

    Robbie McElrath added 1 comment

    File chrome/browser/device_api/device_service_unittest.cc
    Line 176, Patchset 2 (Latest):#if BUILDFLAG(IS_CHROMEOS)
    Reilly Grant . resolved

    Why is this ChromeOS-specific?

    Robbie McElrath

    I followed the existing pattern here, where all of the IWA trust related code is gated on CrOS. I didn't actually review that code (crrev.com/c/5434058) so I'm not sure why it was gated this way, but I don't see why it needs to be.

    I sent crrev.com/c/5677469 to remove these.

    Open in Gerrit

    Related details

    Attention is currently required from:
    • Chung-sheng Wu
    • Reilly Grant
    • Sergii Bykov
    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: I1ab9eabf130d85e8d9a65861c3036270ddc34e7c
    Gerrit-Change-Number: 5673945
    Gerrit-PatchSet: 2
    Gerrit-Owner: Robbie McElrath <rmce...@chromium.org>
    Gerrit-Reviewer: Chung-sheng Wu <chung...@google.com>
    Gerrit-Reviewer: Dibyajyoti Pal <diby...@chromium.org>
    Gerrit-Reviewer: Guido Urdaneta <gui...@chromium.org>
    Gerrit-Reviewer: Martin Šrámek <msr...@chromium.org>
    Gerrit-Reviewer: Reilly Grant <rei...@chromium.org>
    Gerrit-Reviewer: Robbie McElrath <rmce...@chromium.org>
    Gerrit-Reviewer: Sergii Bykov <sby...@google.com>
    Gerrit-CC: Andrew Rayskiy <green...@google.com>
    Gerrit-CC: Rijubrata Bhaumik <rijubrat...@intel.com>
    Gerrit-CC: Simon Hangl <sim...@google.com>
    Gerrit-Attention: Sergii Bykov <sby...@google.com>
    Gerrit-Attention: Chung-sheng Wu <chung...@google.com>
    Gerrit-Attention: Reilly Grant <rei...@chromium.org>
    Gerrit-Comment-Date: Wed, 03 Jul 2024 21:39:14 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: No
    Comment-In-Reply-To: Reilly Grant <rei...@chromium.org>
    satisfied_requirement
    open
    diffy

    Reilly Grant (Gerrit)

    unread,
    Jul 3, 2024, 5:44:26 PM (2 days ago) Jul 3
    to Robbie McElrath, Reilly Grant, Guido Urdaneta, Martin Šrámek, Chung-sheng Wu, Sergii Bykov, Chromium LUCI CQ, chromium...@chromium.org, Andrew Rayskiy, Rijubrata Bhaumik, Simon Hangl, alancutter...@chromium.org, blundell+...@chromium.org, chfreme...@chromium.org, chromium-a...@chromium.org, dibyapal+wa...@chromium.org, dmurph+watc...@chromium.org, dominickn+watch-...@chromium.org, ericwillige...@chromium.org, extension...@chromium.org, feature-me...@chromium.org, kuragin+web-ap...@chromium.org, loyso...@chromium.org, mattreyno...@chromium.org, mgiuca...@chromium.org, odejesu...@chromium.org, oshima...@chromium.org, philli...@chromium.org, webap...@microsoft.com, zelin+watch-we...@chromium.org
    Attention needed from Chung-sheng Wu, Robbie McElrath and Sergii Bykov

    Reilly Grant voted and added 2 comments

    Votes added by Reilly Grant

    Code-Review+1

    2 comments

    Patchset-level comments
    File-level comment, Patchset 2 (Latest):
    Reilly Grant . resolved

    LGTM

    File chrome/browser/device_api/device_service_unittest.cc
    Line 176, Patchset 2 (Latest):#if BUILDFLAG(IS_CHROMEOS)
    Reilly Grant . resolved

    Why is this ChromeOS-specific?

    Robbie McElrath

    I followed the existing pattern here, where all of the IWA trust related code is gated on CrOS. I didn't actually review that code (crrev.com/c/5434058) so I'm not sure why it was gated this way, but I don't see why it needs to be.

    I sent crrev.com/c/5677469 to remove these.

    Reilly Grant

    Thanks for investigating.

    Open in Gerrit

    Related details

    Attention is currently required from:
    • Chung-sheng Wu
    • Robbie McElrath
    • Sergii Bykov
    Gerrit-Attention: Robbie McElrath <rmce...@chromium.org>
    Gerrit-Comment-Date: Wed, 03 Jul 2024 21:44:11 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: Yes
    Comment-In-Reply-To: Reilly Grant <rei...@chromium.org>
    Comment-In-Reply-To: Robbie McElrath <rmce...@chromium.org>
    satisfied_requirement
    open
    diffy

    Robbie McElrath (Gerrit)

    unread,
    Jul 3, 2024, 5:51:56 PM (2 days ago) Jul 3
    to Reilly Grant, Guido Urdaneta, Martin Šrámek, Chung-sheng Wu, Sergii Bykov, Chromium LUCI CQ, chromium...@chromium.org, Andrew Rayskiy, Rijubrata Bhaumik, Simon Hangl, alancutter...@chromium.org, blundell+...@chromium.org, chfreme...@chromium.org, chromium-a...@chromium.org, dibyapal+wa...@chromium.org, dmurph+watc...@chromium.org, dominickn+watch-...@chromium.org, ericwillige...@chromium.org, extension...@chromium.org, feature-me...@chromium.org, kuragin+web-ap...@chromium.org, loyso...@chromium.org, mattreyno...@chromium.org, mgiuca...@chromium.org, odejesu...@chromium.org, oshima...@chromium.org, philli...@chromium.org, webap...@microsoft.com, zelin+watch-we...@chromium.org
    Attention needed from Chung-sheng Wu and Sergii Bykov

    Robbie McElrath added 1 comment

    File chrome/browser/device_api/device_service_unittest.cc
    Line 176, Patchset 2 (Latest):#if BUILDFLAG(IS_CHROMEOS)
    Reilly Grant . resolved

    Why is this ChromeOS-specific?

    Robbie McElrath

    I followed the existing pattern here, where all of the IWA trust related code is gated on CrOS. I didn't actually review that code (crrev.com/c/5434058) so I'm not sure why it was gated this way, but I don't see why it needs to be.

    I sent crrev.com/c/5677469 to remove these.

    Reilly Grant

    Thanks for investigating.

    Robbie McElrath

    Nevermind, the underlying kIsolatedWebAppInstallForceList pref is CrOS only. It looks like all the IWA policy stuff is CrOS only, and I'd rather move un-gating all that to a separate task if it's what we want to do.

    https://source.chromium.org/chromium/chromium/src/+/main:chrome/common/pref_names.h;drc=67d0441446c6264c2921cf5c41098e6b4db467d3;l=2640

    Open in Gerrit

    Related details

    Attention is currently required from:
    • Chung-sheng Wu
    • Sergii Bykov
    Gerrit-Comment-Date: Wed, 03 Jul 2024 21:51:33 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: No
    satisfied_requirement
    open
    diffy

    Sergii Bykov (Gerrit)

    unread,
    Jul 4, 2024, 5:57:58 AM (22 hours ago) Jul 4
    to Robbie McElrath, Reilly Grant, Guido Urdaneta, Martin Šrámek, Chung-sheng Wu, Chromium LUCI CQ, chromium...@chromium.org, Andrew Rayskiy, Rijubrata Bhaumik, Simon Hangl, alancutter...@chromium.org, blundell+...@chromium.org, chfreme...@chromium.org, chromium-a...@chromium.org, dibyapal+wa...@chromium.org, dmurph+watc...@chromium.org, dominickn+watch-...@chromium.org, ericwillige...@chromium.org, extension...@chromium.org, feature-me...@chromium.org, kuragin+web-ap...@chromium.org, loyso...@chromium.org, mattreyno...@chromium.org, mgiuca...@chromium.org, odejesu...@chromium.org, oshima...@chromium.org, philli...@chromium.org, webap...@microsoft.com, zelin+watch-we...@chromium.org
    Attention needed from Chung-sheng Wu, Reilly Grant and Robbie McElrath

    Sergii Bykov added 1 comment

    File chrome/browser/device_api/device_service_unittest.cc
    Line 176, Patchset 2 (Latest):#if BUILDFLAG(IS_CHROMEOS)
    Reilly Grant . unresolved

    Why is this ChromeOS-specific?

    Robbie McElrath

    I followed the existing pattern here, where all of the IWA trust related code is gated on CrOS. I didn't actually review that code (crrev.com/c/5434058) so I'm not sure why it was gated this way, but I don't see why it needs to be.

    I sent crrev.com/c/5677469 to remove these.

    Reilly Grant

    Thanks for investigating.

    Robbie McElrath

    Nevermind, the underlying kIsolatedWebAppInstallForceList pref is CrOS only. It looks like all the IWA policy stuff is CrOS only, and I'd rather move un-gating all that to a separate task if it's what we want to do.

    https://source.chromium.org/chromium/chromium/src/+/main:chrome/common/pref_names.h;drc=67d0441446c6264c2921cf5c41098e6b4db467d3;l=2640

    Sergii Bykov

    Device Attributes API is not an IWA-specific API. It's a Web API (https://wicg.github.io/WebApiDevice/device_attributes/) that we launched on ChromeOS last year.

    It's currently not implemented on other platforms in case you need it:
    https://source.chromium.org/chromium/chromium/src/+/main:chrome/browser/device_api/device_attribute_api.cc;l=40

    Open in Gerrit

    Related details

    Attention is currently required from:
    • Chung-sheng Wu
    • Reilly Grant
    • Robbie McElrath
    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: I1ab9eabf130d85e8d9a65861c3036270ddc34e7c
      Gerrit-Change-Number: 5673945
      Gerrit-PatchSet: 2
      Gerrit-Owner: Robbie McElrath <rmce...@chromium.org>
      Gerrit-Reviewer: Chung-sheng Wu <chung...@google.com>
      Gerrit-Reviewer: Dibyajyoti Pal <diby...@chromium.org>
      Gerrit-Reviewer: Guido Urdaneta <gui...@chromium.org>
      Gerrit-Reviewer: Martin Šrámek <msr...@chromium.org>
      Gerrit-Reviewer: Reilly Grant <rei...@chromium.org>
      Gerrit-Reviewer: Robbie McElrath <rmce...@chromium.org>
      Gerrit-Reviewer: Sergii Bykov <sby...@google.com>
      Gerrit-CC: Andrew Rayskiy <green...@google.com>
      Gerrit-CC: Rijubrata Bhaumik <rijubrat...@intel.com>
      Gerrit-CC: Simon Hangl <sim...@google.com>
      Gerrit-Attention: Chung-sheng Wu <chung...@google.com>
      Gerrit-Attention: Reilly Grant <rei...@chromium.org>
      Gerrit-Attention: Robbie McElrath <rmce...@chromium.org>
      Gerrit-Comment-Date: Thu, 04 Jul 2024 09:57:47 +0000
      satisfied_requirement
      unsatisfied_requirement
      open
      diffy

      Chung-sheng Wu (Gerrit)

      unread,
      2:25 AM (2 hours ago) 2:25 AM
      to Robbie McElrath, Reilly Grant, Guido Urdaneta, Martin Šrámek, Sergii Bykov, Chromium LUCI CQ, chromium...@chromium.org, Andrew Rayskiy, Rijubrata Bhaumik, Simon Hangl, alancutter...@chromium.org, blundell+...@chromium.org, chfreme...@chromium.org, chromium-a...@chromium.org, dibyapal+wa...@chromium.org, dmurph+watc...@chromium.org, dominickn+watch-...@chromium.org, ericwillige...@chromium.org, extension...@chromium.org, feature-me...@chromium.org, kuragin+web-ap...@chromium.org, loyso...@chromium.org, mattreyno...@chromium.org, mgiuca...@chromium.org, odejesu...@chromium.org, oshima...@chromium.org, philli...@chromium.org, webap...@microsoft.com, zelin+watch-we...@chromium.org
      Attention needed from Reilly Grant and Robbie McElrath

      Chung-sheng Wu voted Code-Review+1

      Code-Review+1
      Open in Gerrit

      Related details

      Attention is currently required from:
      • Reilly Grant
      • Robbie McElrath
      Gerrit-Attention: Reilly Grant <rei...@chromium.org>
      Gerrit-Attention: Robbie McElrath <rmce...@chromium.org>
      Gerrit-Comment-Date: Fri, 05 Jul 2024 06:25:38 +0000
      Gerrit-HasComments: No
      Gerrit-Has-Labels: Yes
      satisfied_requirement
      unsatisfied_requirement
      open
      diffy

      Sergii Bykov (Gerrit)

      unread,
      4:12 AM (13 minutes ago) 4:12 AM
      to Robbie McElrath, Chung-sheng Wu, Reilly Grant, Guido Urdaneta, Martin Šrámek, Chromium LUCI CQ, chromium...@chromium.org, Andrew Rayskiy, Rijubrata Bhaumik, Simon Hangl, alancutter...@chromium.org, blundell+...@chromium.org, chfreme...@chromium.org, chromium-a...@chromium.org, dibyapal+wa...@chromium.org, dmurph+watc...@chromium.org, dominickn+watch-...@chromium.org, ericwillige...@chromium.org, extension...@chromium.org, feature-me...@chromium.org, kuragin+web-ap...@chromium.org, loyso...@chromium.org, mattreyno...@chromium.org, mgiuca...@chromium.org, odejesu...@chromium.org, oshima...@chromium.org, philli...@chromium.org, webap...@microsoft.com, zelin+watch-we...@chromium.org
      Attention needed from Reilly Grant and Robbie McElrath

      Sergii Bykov voted Code-Review+1

      Gerrit-Comment-Date: Fri, 05 Jul 2024 08:12:15 +0000
      Gerrit-HasComments: No
      Gerrit-Has-Labels: Yes
      satisfied_requirement
      unsatisfied_requirement
      open
      diffy
      Reply all
      Reply to author
      Forward
      0 new messages