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/
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. | Gerrit |
Code-Review | +1 |
LGTM % nit
base::test::ScopedFeatureList scoped_feature_list;
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.
Commit-Queue | +1 |
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.
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. | Gerrit |
#if BUILDFLAG(IS_CHROMEOS)
Why is this ChromeOS-specific?
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. | Gerrit |
#if BUILDFLAG(IS_CHROMEOS)
Robbie McElrathWhy is this ChromeOS-specific?
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.
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. | Gerrit |
Code-Review | +1 |
#if BUILDFLAG(IS_CHROMEOS)
Robbie McElrathWhy is this ChromeOS-specific?
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.
#if BUILDFLAG(IS_CHROMEOS)
Robbie McElrathWhy is this ChromeOS-specific?
Reilly GrantI 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.
Thanks for investigating.
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.
#if BUILDFLAG(IS_CHROMEOS)
Robbie McElrathWhy is this ChromeOS-specific?
Reilly GrantI 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.
Robbie McElrathThanks for investigating.
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.
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
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. | Gerrit |