Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add permission lifetime backend. #8275

Merged
merged 6 commits into from
Apr 2, 2021
Merged

Add permission lifetime backend. #8275

merged 6 commits into from
Apr 2, 2021

Conversation

goodov
Copy link
Member

@goodov goodov commented Mar 17, 2021

Implement permission lifetime logic to track permissions lifetime and revoke them when required.

This change implements time-related aspect of brave/brave-browser#14126, i.e. it handles different lifetime options (24 hrs, 7 days, etc), but does not include support for "permission for this session" aka "revoke permission when all WebContents with eTLD+1 are closed".

The overall logic consists of these steps:

  1. Observe UI interactions with PermissionPromptBubble to set permission lifetime in all PermissionRequests currently presented to a user.
  2. Pass a permission decision to PermissionLifetimeManager (with all permission data, including content type, origins, lifetime).
  3. Store permission origins and lifetime to prefs (PermissionExpirations).
  4. Start/update permission revoke timer for a closest permission to be revoked (PermissionLifetimeManager).
  5. Revoke permissions on timer hit and reschedule it for a next closest permission to revoke (PermissionLifetimeManager).
  6. Observe HostContentSettingsMap to handle any permission changes not triggered via permission prompt dialogs (e.g. manually via Settings or via Page Info UI). This is required to properly update/stop the timer and clean up prefs for permissions we don't need to control anymore (PermissionLifetimeManager).

brave/brave-browser#14126

Submitter Checklist:

  • I confirm that no security/privacy review is needed, or that I have requested one
  • There is a ticket for my issue
  • Used Github auto-closing keywords in the PR description above
  • Wrote a good PR/commit description
  • Added appropriate labels (QA/Yes or QA/No; release-notes/include or release-notes/exclude; OS/...) to the associated issue
  • Checked the PR locally: npm run test -- brave_browser_tests, npm run test -- brave_unit_tests, npm run lint, npm run gn_check, npm run tslint
  • Ran git rebase master (if needed)

Reviewer Checklist:

  • A security review is not needed, or a link to one is included in the PR description
  • New files have MPL-2.0 license header
  • Adequate test coverage exists to prevent regressions
  • Major classes, functions and non-trivial code blocks are well-commented
  • Changes in component dependencies are properly reflected in gn
  • Code follows the style guide
  • Test plan is specified in PR before merging

After-merge Checklist:

Test Plan:

@goodov goodov requested a review from iefremov March 17, 2021 13:30
@goodov goodov marked this pull request as ready for review March 17, 2021 13:30
@goodov goodov requested review from bridiver and a team as code owners March 17, 2021 13:30
@goodov goodov force-pushed the issues/14126 branch 4 times, most recently from 54af4f5 to 23aaccb Compare March 18, 2021 10:42
@simonhong
Copy link
Member

@goodov Sorry for the late response. I'll review today!

@goodov
Copy link
Member Author

goodov commented Mar 22, 2021

PTAL. It's ready.

Copy link
Member

@simonhong simonhong left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

++ for me. Only have trivial nits. 👍

browser/permissions/BUILD.gn Outdated Show resolved Hide resolved
browser/permissions/permission_lifetime_manager_factory.cc Outdated Show resolved Hide resolved
components/permissions/permission_lifetime_pref_names.h Outdated Show resolved Hide resolved
Copy link
Collaborator

@bridiver bridiver left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

patches/chromium_src looks ok, but there are some missing dep issues

@goodov goodov requested a review from bridiver April 1, 2021 12:30
@goodov goodov merged commit 578925e into master Apr 2, 2021
@goodov goodov deleted the issues/14126 branch April 2, 2021 05:16
@goodov goodov added this to the 1.24.x - Nightly milestone Apr 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants