Add new ExtensionPermissionProvider [chromium/src : main]
Joel Hockey (Gerrit)
Joel Hockey added 5 comments![]( "Open in Gerrit")
Add new ContentSettingsApiPermissionProviderJoel Hockeynit: Add an empty line between the title and description of a CL
Done
ProviderType::kExtensionApiPermissionProvider,Joel HockeyCould you add unit tests for HostContentSettingsMap for extension URLs?
HostContentSettingsMapTest.ExtensionContentSetting already exists.
kExtensionApiPermissionProvider,Joel HockeyThe order of providers in this enum defines their priority. If users should be able to override permissions from this provider, then I think the position is good (Above default but below prefprovider). But please confirm this :)
I did place the provider at this position to allow any other providers to override if they already happen to. I don't think that any do for extensions, but this still feels like a good position.
std::unique_ptr<content_settings::Rule> GetRule(Joel HockeyCould you add unit tests for these methods?
Done
class ContentSettingsApiPermissionProvider finalJoel HockeyWhy do you call it ApiPermissionProvider? Should we call it ExtensionPermissionProvider?
Done
Related details
- Christian Dullweber
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Christian Dullweber (Gerrit)
Christian Dullweber added 2 comments![]( "Open in Gerrit")
ProviderType::kExtensionApiPermissionProvider,Joel HockeyCould you add unit tests for HostContentSettingsMap for extension URLs?
HostContentSettingsMapTest.ExtensionContentSetting already exists.
That test is checking that custom content settings are applied to extensions. I think it would be good to have a test that an install-time permission for extension is returned by HostContentSettingsMap?
https://source.chromium.org/chromium/chromium/src/+/main:chrome/browser/content_settings/host_content_settings_map_unittest.cc;l=2970;drc=0911060b87775470bd81b3e2fcc6a164124278e6
Related details
- Joel Hockey
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Christian Dullweber (Gerrit)
Christian Dullweber added 1 comment![]( "Open in Gerrit")
ProviderType::kExtensionApiPermissionProvider,Joel HockeyCould you add unit tests for HostContentSettingsMap for extension URLs?
Christian DullweberHostContentSettingsMapTest.ExtensionContentSetting already exists.
That test is checking that custom content settings are applied to extensions. I think it would be good to have a test that an install-time permission for extension is returned by HostContentSettingsMap?
https://source.chromium.org/chromium/chromium/src/+/main:chrome/browser/content_settings/host_content_settings_map_unittest.cc;l=2970;drc=0911060b87775470bd81b3e2fcc6a164124278e6
Maybe also that an install-time permission can be overriden by user permissions
Joel Hockey (Gerrit)
Joel Hockey voted and added 1 comment![]( "Open in Gerrit")
Votes added by Joel Hockey
| Commit-Queue | +1 |
1 comment
ProviderType::kExtensionApiPermissionProvider,Joel HockeyCould you add unit tests for HostContentSettingsMap for extension URLs?
Christian DullweberHostContentSettingsMapTest.ExtensionContentSetting already exists.
Christian DullweberThat test is checking that custom content settings are applied to extensions. I think it would be good to have a test that an install-time permission for extension is returned by HostContentSettingsMap?
https://source.chromium.org/chromium/chromium/src/+/main:chrome/browser/content_settings/host_content_settings_map_unittest.cc;l=2970;drc=0911060b87775470bd81b3e2fcc6a164124278e6
Maybe also that an install-time permission can be overriden by user permissions
- Christian Dullweber
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |