New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 780344 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
OOO until 4th Feb
Closed: Mar 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 1
Type: Bug
Team-Security-UX



Sign in to add a comment

Automatically grant notifications to the Default Search Engine

Project Member Reported by raymes@chromium.org, Nov 1 2017

Issue description

We want the DSE to be automatically be granted notifications permission. The behavior should be the same as geolocation.
 
Cc: peter@chromium.org
I think the following changes need to be made:
1) Rename search_geolocation_service* to search_permissions_service*
2) Add support for notifications to SearchPermissionsService (perhaps paramaterize the functions on ContentSettingsType
3) Update NotificationPermissionContext::GetPermissionStatusInternal to query SearchPermissionsService (as we do with geolocation: https://cs.chromium.org/chromium/src/chrome/browser/geolocation/geolocation_permission_context_android.cc?rcl=29b0469acd5c529d525195bd18181192d6c38d6d&l=107)
4) Update the code which changes the notifications content setting to change the DSE setting when needed (as we do with geolocation: https://cs.chromium.org/chromium/src/chrome/android/java/src/org/chromium/chrome/browser/preferences/website/Website.java?rcl=29b0469acd5c529d525195bd18181192d6c38d6d&l=251)
5) Handle the 1 case I know of where we don't query the notifications setting through the PermissionManager on Android (https://cs.chromium.org/chromium/src/chrome/browser/notifications/platform_notification_service_impl.cc?rcl=5e9d340366992d63fc03a2f814b218f349e5e551&l=262)


The above won't work as suggested because of notification channels on Android. I wrote up a doc (emailed to you) with more details.
Components: Privacy
Cc: msramek@chromium.org
Project Member

Comment 6 by bugdroid1@chromium.org, Nov 6 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/01251b3ef1bd31417a3282d24184bc8a3a1e8d43

commit 01251b3ef1bd31417a3282d24184bc8a3a1e8d43
Author: Raymes Khoury <raymes@chromium.org>
Date: Mon Nov 06 22:31:27 2017

Rename SearchGeolocationService to SearchPermissionService

This renames SearchGeolocationService to SearchPermissionService to
reflect the fact that this code will soon be used to grant the
notifications permission to the default search engine in addition to
geolocation.

This also renames associated files:
search_geolocation_service* -> search_permissions_service*
c/b/a/search_geolocation -> c/b/a/search_permissions

Bug:  780344 
Change-Id: I7937844f4206a9b9207b1767d78e61f74650de53
Reviewed-on: https://chromium-review.googlesource.com/748401
Commit-Queue: Raymes Khoury <raymes@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Reviewed-by: Ben Wells <benwells@chromium.org>
Cr-Commit-Position: refs/heads/master@{#514260}
[modify] https://crrev.com/01251b3ef1bd31417a3282d24184bc8a3a1e8d43/chrome/browser/BUILD.gn
[modify] https://crrev.com/01251b3ef1bd31417a3282d24184bc8a3a1e8d43/chrome/browser/android/preferences/browser_prefs_android.cc
[modify] https://crrev.com/01251b3ef1bd31417a3282d24184bc8a3a1e8d43/chrome/browser/android/preferences/website_preference_bridge.cc
[rename] https://crrev.com/01251b3ef1bd31417a3282d24184bc8a3a1e8d43/chrome/browser/android/search_permissions/OWNERS
[rename] https://crrev.com/01251b3ef1bd31417a3282d24184bc8a3a1e8d43/chrome/browser/android/search_permissions/search_geolocation_disclosure_infobar_delegate.cc
[rename] https://crrev.com/01251b3ef1bd31417a3282d24184bc8a3a1e8d43/chrome/browser/android/search_permissions/search_geolocation_disclosure_infobar_delegate.h
[rename] https://crrev.com/01251b3ef1bd31417a3282d24184bc8a3a1e8d43/chrome/browser/android/search_permissions/search_geolocation_disclosure_tab_helper.cc
[rename] https://crrev.com/01251b3ef1bd31417a3282d24184bc8a3a1e8d43/chrome/browser/android/search_permissions/search_geolocation_disclosure_tab_helper.h
[rename] https://crrev.com/01251b3ef1bd31417a3282d24184bc8a3a1e8d43/chrome/browser/android/search_permissions/search_permissions_service.cc
[rename] https://crrev.com/01251b3ef1bd31417a3282d24184bc8a3a1e8d43/chrome/browser/android/search_permissions/search_permissions_service.h
[rename] https://crrev.com/01251b3ef1bd31417a3282d24184bc8a3a1e8d43/chrome/browser/android/search_permissions/search_permissions_service_unittest.cc
[modify] https://crrev.com/01251b3ef1bd31417a3282d24184bc8a3a1e8d43/chrome/browser/geolocation/geolocation_permission_context_android.cc
[modify] https://crrev.com/01251b3ef1bd31417a3282d24184bc8a3a1e8d43/chrome/browser/geolocation/geolocation_permission_context_unittest.cc
[modify] https://crrev.com/01251b3ef1bd31417a3282d24184bc8a3a1e8d43/chrome/browser/profiles/chrome_browser_main_extra_parts_profiles.cc
[modify] https://crrev.com/01251b3ef1bd31417a3282d24184bc8a3a1e8d43/chrome/browser/ui/android/infobars/search_geolocation_disclosure_infobar.cc
[modify] https://crrev.com/01251b3ef1bd31417a3282d24184bc8a3a1e8d43/chrome/browser/ui/android/page_info/page_info_popup_android.cc
[modify] https://crrev.com/01251b3ef1bd31417a3282d24184bc8a3a1e8d43/chrome/browser/ui/android/page_info/page_info_popup_android.h
[modify] https://crrev.com/01251b3ef1bd31417a3282d24184bc8a3a1e8d43/chrome/browser/ui/tab_helpers.cc
[modify] https://crrev.com/01251b3ef1bd31417a3282d24184bc8a3a1e8d43/chrome/test/BUILD.gn

Project Member

Comment 7 by bugdroid1@chromium.org, Nov 7 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9e59c47de22f40cc3283b27320fc18560f2b5f62

commit 9e59c47de22f40cc3283b27320fc18560f2b5f62
Author: Raymes Khoury <raymes@chromium.org>
Date: Tue Nov 07 08:32:48 2017

Remove logic for deprecated Google-only Default Search Engine behavior

The kGoogleDSEGeolocationSettingDeprecated pref was deprecated in either
M60 or M61. This removes it from M64 which will have been at least 3
milestones after it was deprecated which should be sufficient time for
users to migrate.

If users haven't migrated yet, they will simply get the out-of-the-box
behavior for default search.

Bug:  780344 
Change-Id: I1fe4033ae21ab6a72d6bce7f1ccf06bb3921fc02
Reviewed-on: https://chromium-review.googlesource.com/750561
Commit-Queue: Raymes Khoury <raymes@chromium.org>
Reviewed-by: Ben Wells <benwells@chromium.org>
Cr-Commit-Position: refs/heads/master@{#514421}
[modify] https://crrev.com/9e59c47de22f40cc3283b27320fc18560f2b5f62/chrome/browser/android/search_permissions/search_permissions_service.cc
[modify] https://crrev.com/9e59c47de22f40cc3283b27320fc18560f2b5f62/chrome/browser/android/search_permissions/search_permissions_service_unittest.cc
[modify] https://crrev.com/9e59c47de22f40cc3283b27320fc18560f2b5f62/chrome/common/pref_names.cc
[modify] https://crrev.com/9e59c47de22f40cc3283b27320fc18560f2b5f62/chrome/common/pref_names.h

Project Member

Comment 8 by bugdroid1@chromium.org, Nov 9 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/cdf40eef972eea49a3764fb9c6b59af908fdeecb

commit cdf40eef972eea49a3764fb9c6b59af908fdeecb
Author: Raymes Khoury <raymes@chromium.org>
Date: Thu Nov 09 00:37:00 2017

Remove redundant search engine permission UI logic on Android

Currently there is logic which ensures the geolocation permission is
displayed in Site Details for the default search engine when the
location link is clicked from Settings>Search Engines. Since we've
enabled location consistency, this logic isn't needed as the location
setting will always be set and displayed appropriately.

Bug:  780344 
Change-Id: Ie04f096194b4ee3ebd6ecb601db2fa8f744cb663
Tbr: finnur@chromium.org
Reviewed-on: https://chromium-review.googlesource.com/758017
Commit-Queue: Raymes Khoury <raymes@chromium.org>
Reviewed-by: Ben Wells <benwells@chromium.org>
Cr-Commit-Position: refs/heads/master@{#515035}
[modify] https://crrev.com/cdf40eef972eea49a3764fb9c6b59af908fdeecb/chrome/android/java/src/org/chromium/chrome/browser/preferences/SearchEngineAdapter.java
[modify] https://crrev.com/cdf40eef972eea49a3764fb9c6b59af908fdeecb/chrome/android/java/src/org/chromium/chrome/browser/preferences/website/SingleWebsitePreferences.java

Comment 9 by est...@chromium.org, Nov 10 2017

Labels: Hotlist-EnamelAndFriendsFixIt
Project Member

Comment 10 by bugdroid1@chromium.org, Nov 27 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/12fe1c1febde50a12c11c61e3090e2b5f6223fe1

commit 12fe1c1febde50a12c11c61e3090e2b5f6223fe1
Author: Raymes Khoury <raymes@chromium.org>
Date: Mon Nov 27 05:33:49 2017

Add function to HCSM to get the current user-modifiable ContentSetting

In order for DSE permissions to be properly migrated when the user
changes DSE, we need to be able to obtain the current user-defined
content setting even in the presence of enterprise policy or other
policy which would generally override the user-defined setting. This CL
adds a function to HCSM to obtain the user-defined setting.

Bug:  780344 
Change-Id: I1cc75489ac92676475165ced0ef1fa233ab6364f
Reviewed-on: https://chromium-review.googlesource.com/786875
Commit-Queue: Raymes Khoury <raymes@chromium.org>
Reviewed-by: Martin Šrámek <msramek@chromium.org>
Cr-Commit-Position: refs/heads/master@{#519222}
[modify] https://crrev.com/12fe1c1febde50a12c11c61e3090e2b5f6223fe1/chrome/browser/content_settings/host_content_settings_map_unittest.cc
[modify] https://crrev.com/12fe1c1febde50a12c11c61e3090e2b5f6223fe1/components/content_settings/core/browser/host_content_settings_map.cc
[modify] https://crrev.com/12fe1c1febde50a12c11c61e3090e2b5f6223fe1/components/content_settings/core/browser/host_content_settings_map.h

Project Member

Comment 11 by bugdroid1@chromium.org, Dec 4 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/82049ea7a898657c4aad7492ffa5e765ef4c08a9

commit 82049ea7a898657c4aad7492ffa5e765ef4c08a9
Author: Raymes Khoury <raymes@chromium.org>
Date: Mon Dec 04 02:37:29 2017

Automatically grant notifications to the Default Search Engine

This adds support to automatically grant the Default Search Engine
the notifications permission on Android. The behavior is similar to
the way the geolocation permission works for the DSE.

Although the behavior is mostly the same, this CL changes the way
permissions (geolocation and notifications) are transferred between
DSEs when the user changes their DSE. The new approach stores the
permissions that the user had set on the DSE origin prior to it
becoming the DSE. When the user changes the DSE, we restore the old
settings.

No UI for the notifications part has yet been implemented.

Bug:  780344 
Change-Id: Ifa57e395ad4a647f42debb78146d50574ee1b636
Reviewed-on: https://chromium-review.googlesource.com/756657
Commit-Queue: Raymes Khoury <raymes@chromium.org>
Reviewed-by: Jesse Doherty <jwd@chromium.org>
Reviewed-by: Ben Wells <benwells@chromium.org>
Reviewed-by: Bernhard Bauer <bauerb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#521256}
[modify] https://crrev.com/82049ea7a898657c4aad7492ffa5e765ef4c08a9/chrome/android/java/src/org/chromium/chrome/browser/omnibox/geo/GeolocationHeader.java
[modify] https://crrev.com/82049ea7a898657c4aad7492ffa5e765ef4c08a9/chrome/android/java/src/org/chromium/chrome/browser/page_info/PageInfoPopup.java
[modify] https://crrev.com/82049ea7a898657c4aad7492ffa5e765ef4c08a9/chrome/android/java/src/org/chromium/chrome/browser/preferences/SearchEngineAdapter.java
[modify] https://crrev.com/82049ea7a898657c4aad7492ffa5e765ef4c08a9/chrome/android/java/src/org/chromium/chrome/browser/preferences/website/SingleWebsitePreferences.java
[modify] https://crrev.com/82049ea7a898657c4aad7492ffa5e765ef4c08a9/chrome/android/java/src/org/chromium/chrome/browser/preferences/website/Website.java
[modify] https://crrev.com/82049ea7a898657c4aad7492ffa5e765ef4c08a9/chrome/android/java/src/org/chromium/chrome/browser/preferences/website/WebsitePreferenceBridge.java
[modify] https://crrev.com/82049ea7a898657c4aad7492ffa5e765ef4c08a9/chrome/android/javatests/src/org/chromium/chrome/browser/engagement/SiteEngagementServiceTest.java
[modify] https://crrev.com/82049ea7a898657c4aad7492ffa5e765ef4c08a9/chrome/android/javatests/src/org/chromium/chrome/browser/infobar/SearchGeolocationDisclosureInfoBarTest.java
[modify] https://crrev.com/82049ea7a898657c4aad7492ffa5e765ef4c08a9/chrome/android/javatests/src/org/chromium/chrome/browser/omnibox/geo/GeolocationHeaderTest.java
[modify] https://crrev.com/82049ea7a898657c4aad7492ffa5e765ef4c08a9/chrome/android/javatests/src/org/chromium/chrome/browser/preferences/PreferencesTest.java
[modify] https://crrev.com/82049ea7a898657c4aad7492ffa5e765ef4c08a9/chrome/android/javatests/src/org/chromium/chrome/browser/preferences/privacy/ClearBrowsingDataPreferencesTest.java
[modify] https://crrev.com/82049ea7a898657c4aad7492ffa5e765ef4c08a9/chrome/android/junit/src/org/chromium/chrome/browser/omnibox/geo/GeolocationHeaderUnitTest.java
[modify] https://crrev.com/82049ea7a898657c4aad7492ffa5e765ef4c08a9/chrome/browser/android/preferences/website_preference_bridge.cc
[modify] https://crrev.com/82049ea7a898657c4aad7492ffa5e765ef4c08a9/chrome/browser/android/search_permissions/search_geolocation_disclosure_tab_helper.cc
[modify] https://crrev.com/82049ea7a898657c4aad7492ffa5e765ef4c08a9/chrome/browser/android/search_permissions/search_permissions_service.cc
[modify] https://crrev.com/82049ea7a898657c4aad7492ffa5e765ef4c08a9/chrome/browser/android/search_permissions/search_permissions_service.h
[modify] https://crrev.com/82049ea7a898657c4aad7492ffa5e765ef4c08a9/chrome/browser/android/search_permissions/search_permissions_service_unittest.cc
[modify] https://crrev.com/82049ea7a898657c4aad7492ffa5e765ef4c08a9/chrome/browser/geolocation/geolocation_permission_context_android.cc
[modify] https://crrev.com/82049ea7a898657c4aad7492ffa5e765ef4c08a9/chrome/browser/geolocation/geolocation_permission_context_android.h
[modify] https://crrev.com/82049ea7a898657c4aad7492ffa5e765ef4c08a9/chrome/browser/geolocation/geolocation_permission_context_unittest.cc
[modify] https://crrev.com/82049ea7a898657c4aad7492ffa5e765ef4c08a9/chrome/browser/ui/android/page_info/page_info_popup_android.cc
[modify] https://crrev.com/82049ea7a898657c4aad7492ffa5e765ef4c08a9/chrome/browser/ui/android/page_info/page_info_popup_android.h
[modify] https://crrev.com/82049ea7a898657c4aad7492ffa5e765ef4c08a9/chrome/common/chrome_features.cc
[modify] https://crrev.com/82049ea7a898657c4aad7492ffa5e765ef4c08a9/chrome/common/chrome_features.h
[modify] https://crrev.com/82049ea7a898657c4aad7492ffa5e765ef4c08a9/chrome/common/pref_names.cc
[modify] https://crrev.com/82049ea7a898657c4aad7492ffa5e765ef4c08a9/chrome/common/pref_names.h
[modify] https://crrev.com/82049ea7a898657c4aad7492ffa5e765ef4c08a9/tools/metrics/histograms/histograms.xml

Project Member

Comment 12 by bugdroid1@chromium.org, Dec 4 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/91377e4719901c1ac14d253f15add8be748f37c4

commit 91377e4719901c1ac14d253f15add8be748f37c4
Author: Raymes Khoury <raymes@chromium.org>
Date: Mon Dec 04 06:20:00 2017

Handle DSE permission changes when the default search engine is disabled

The default search engine can be disabled by enterprise. In this case,
we restore the permission settings for the old DSE. We also delete the
underlying pref that tracks settings, so that if the DSE is re-enabled,
the DSE permissions will be re-initialized for the new DSE as if being
initialized for the first time.

Bug:  780344 
Change-Id: I6df13d6b085dbe8164fe003c3406245e5b8efb2f
Reviewed-on: https://chromium-review.googlesource.com/765566
Commit-Queue: Raymes Khoury <raymes@chromium.org>
Reviewed-by: Ben Wells <benwells@chromium.org>
Cr-Commit-Position: refs/heads/master@{#521278}
[modify] https://crrev.com/91377e4719901c1ac14d253f15add8be748f37c4/chrome/browser/android/search_permissions/search_permissions_service.cc
[modify] https://crrev.com/91377e4719901c1ac14d253f15add8be748f37c4/chrome/browser/android/search_permissions/search_permissions_service.h
[modify] https://crrev.com/91377e4719901c1ac14d253f15add8be748f37c4/chrome/browser/android/search_permissions/search_permissions_service_unittest.cc

Project Member

Comment 13 by bugdroid1@chromium.org, Dec 4 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/30472ee0a5c981ce0e53ba8f73363aaed176607e

commit 30472ee0a5c981ce0e53ba8f73363aaed176607e
Author: Raymes Khoury <raymes@chromium.org>
Date: Mon Dec 04 07:20:33 2017

Reset Default Search Engine permissions properly

After the refactor in
https://chromium-review.googlesource.com/c/chromium/src/+/756657/
which changes the DSE permissions to be reflected in the user's content
settings, we need to make sure these are reset to their default values
when the user clicks "Clear and Reset" or "Clear Browsing Data".

Bug:  780344 
Change-Id: I8e18ecc0d60b9ec0442ab2e5291c55cfd2480b61
Reviewed-on: https://chromium-review.googlesource.com/770592
Commit-Queue: Raymes Khoury <raymes@chromium.org>
Reviewed-by: Bernhard Bauer <bauerb@chromium.org>
Reviewed-by: Martin Šrámek <msramek@chromium.org>
Reviewed-by: Ben Wells <benwells@chromium.org>
Cr-Commit-Position: refs/heads/master@{#521281}
[modify] https://crrev.com/30472ee0a5c981ce0e53ba8f73363aaed176607e/chrome/android/java/src/org/chromium/chrome/browser/ChromeFeatureList.java
[modify] https://crrev.com/30472ee0a5c981ce0e53ba8f73363aaed176607e/chrome/android/java/src/org/chromium/chrome/browser/preferences/website/WebsitePreferenceBridge.java
[modify] https://crrev.com/30472ee0a5c981ce0e53ba8f73363aaed176607e/chrome/android/java_sources.gni
[modify] https://crrev.com/30472ee0a5c981ce0e53ba8f73363aaed176607e/chrome/android/javatests/src/org/chromium/chrome/browser/omnibox/geo/GeolocationHeaderTest.java
[add] https://crrev.com/30472ee0a5c981ce0e53ba8f73363aaed176607e/chrome/android/javatests/src/org/chromium/chrome/browser/preferences/website/PermissionInfoTest.java
[modify] https://crrev.com/30472ee0a5c981ce0e53ba8f73363aaed176607e/chrome/browser/android/preferences/website_preference_bridge.cc
[modify] https://crrev.com/30472ee0a5c981ce0e53ba8f73363aaed176607e/chrome/browser/android/search_permissions/search_permissions_service.cc
[modify] https://crrev.com/30472ee0a5c981ce0e53ba8f73363aaed176607e/chrome/browser/android/search_permissions/search_permissions_service.h
[modify] https://crrev.com/30472ee0a5c981ce0e53ba8f73363aaed176607e/chrome/browser/android/search_permissions/search_permissions_service_unittest.cc
[modify] https://crrev.com/30472ee0a5c981ce0e53ba8f73363aaed176607e/chrome/browser/browsing_data/chrome_browsing_data_remover_delegate.cc
[modify] https://crrev.com/30472ee0a5c981ce0e53ba8f73363aaed176607e/chrome/browser/browsing_data/chrome_browsing_data_remover_delegate_unittest.cc

Project Member

Comment 14 by bugdroid1@chromium.org, Dec 6 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6567c2d70508bd466f59e658f92da81e9911bb1d

commit 6567c2d70508bd466f59e658f92da81e9911bb1d
Author: Raymes Khoury <raymes@chromium.org>
Date: Wed Dec 06 02:51:05 2017

Change SearchPermissionService to work correctly in the presence of enterpise policy

This makes SearchPermissionService preserve the correct content setting
values when content setting enterprise policies are in effect. It is
changed to use GetUserModifiableContentSetting which will always return
the user defined content setting, even when an enterprise policy is in
place.

Bug:  780344 
Change-Id: I8b9362ddd6580c602270f7b5d8d9dafbb467a896
Reviewed-on: https://chromium-review.googlesource.com/807865
Reviewed-by: Ben Wells <benwells@chromium.org>
Commit-Queue: Raymes Khoury <raymes@chromium.org>
Cr-Commit-Position: refs/heads/master@{#521970}
[modify] https://crrev.com/6567c2d70508bd466f59e658f92da81e9911bb1d/chrome/browser/android/search_permissions/search_permissions_service.cc
[modify] https://crrev.com/6567c2d70508bd466f59e658f92da81e9911bb1d/chrome/browser/android/search_permissions/search_permissions_service_unittest.cc

Project Member

Comment 15 by bugdroid1@chromium.org, Dec 6 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f3af6cde35cfe72e60a152a53833a72111dc592c

commit f3af6cde35cfe72e60a152a53833a72111dc592c
Author: Raymes Khoury <raymes@chromium.org>
Date: Wed Dec 06 03:02:35 2017

Change ArePermissionsControlledByDSE to IsPermissionControlledByDSE

This changes the function which checks whether permissions are
controlled by the DSE to take an extra argument for the
ContentSettingsType which allows querying whether a particular type is
allowed. This simplifies code at callsites since we can centralise the
check for the Notifications feature flag and it also means that we can
have a central location for the list of supported permissions.

As a side effect of this change, the Page Info bubble will now
correctly display the text "Allow/Block for the current search engine"
next to the notifications permission, when the GrantNotificationsToDSE
feature is enabled.

Bug:  780344 
Change-Id: I0300397e337c47e4eee83a10dd4900d8ca7d86f0
Reviewed-on: https://chromium-review.googlesource.com/795520
Reviewed-by: Ben Wells <benwells@chromium.org>
Reviewed-by: Ted Choc <tedchoc@chromium.org>
Commit-Queue: Raymes Khoury <raymes@chromium.org>
Cr-Commit-Position: refs/heads/master@{#521973}
[modify] https://crrev.com/f3af6cde35cfe72e60a152a53833a72111dc592c/chrome/android/java/src/org/chromium/chrome/browser/omnibox/geo/GeolocationHeader.java
[modify] https://crrev.com/f3af6cde35cfe72e60a152a53833a72111dc592c/chrome/android/java/src/org/chromium/chrome/browser/page_info/PageInfoPopup.java
[modify] https://crrev.com/f3af6cde35cfe72e60a152a53833a72111dc592c/chrome/android/java/src/org/chromium/chrome/browser/preferences/website/SingleWebsitePreferences.java
[modify] https://crrev.com/f3af6cde35cfe72e60a152a53833a72111dc592c/chrome/android/java/src/org/chromium/chrome/browser/preferences/website/WebsitePreferenceBridge.java
[modify] https://crrev.com/f3af6cde35cfe72e60a152a53833a72111dc592c/chrome/android/junit/src/org/chromium/chrome/browser/omnibox/geo/GeolocationHeaderUnitTest.java
[modify] https://crrev.com/f3af6cde35cfe72e60a152a53833a72111dc592c/chrome/browser/android/preferences/website_preference_bridge.cc
[modify] https://crrev.com/f3af6cde35cfe72e60a152a53833a72111dc592c/chrome/browser/android/search_permissions/search_geolocation_disclosure_tab_helper.cc
[modify] https://crrev.com/f3af6cde35cfe72e60a152a53833a72111dc592c/chrome/browser/android/search_permissions/search_permissions_service.cc
[modify] https://crrev.com/f3af6cde35cfe72e60a152a53833a72111dc592c/chrome/browser/android/search_permissions/search_permissions_service.h
[modify] https://crrev.com/f3af6cde35cfe72e60a152a53833a72111dc592c/chrome/browser/android/search_permissions/search_permissions_service_unittest.cc

Project Member

Comment 16 by bugdroid1@chromium.org, Dec 12 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6deaae488ed3eb805a385c453507a1ebac0edf80

commit 6deaae488ed3eb805a385c453507a1ebac0edf80
Author: Raymes Khoury <raymes@chromium.org>
Date: Tue Dec 12 23:43:05 2017

Update the UI to reflect the notifications permission for the DSE

This updates 2 UI surfaces to reflect notifications permissions set for
the DSE. Specifically:
1) In android Site Details, when toggle the DSE permission for
notififications, it will show the options "Allow/Block for the current
search engine". 2 cases are handled here: one when that setting links
directly into Notification Channels on Android O+ and the other in the
case where the permission is handled directly in Chrome.

2) In Settings>Search Engines, underneath the currently selected search
engine, it will display a link indicating whether
notifications/geolocation are allowed. The link will take the user to
the Site Details for the search engine. Several changes have been made
to the behavior here:
-The link used to indicate if location was disabled. Now, if
notifications are location are disabled, nothing will be shown in the
text for those permissions.
-The link used to take users directly to Android Location Settings when
the text indicated that system location was disabled. But now due to the
link handling combined permissions, it always takes the user directly to
Site Details for the search engine, where the user can follow a link
into Android Location Settings.

Bug:  780344 
Change-Id: I41e0dd1028ad289141d1632875d30373b528f17c
Reviewed-on: https://chromium-review.googlesource.com/799534
Commit-Queue: Raymes Khoury <raymes@chromium.org>
Reviewed-by: Ben Wells <benwells@chromium.org>
Reviewed-by: Ted Choc <tedchoc@chromium.org>
Cr-Commit-Position: refs/heads/master@{#523606}
[modify] https://crrev.com/6deaae488ed3eb805a385c453507a1ebac0edf80/chrome/android/java/src/org/chromium/chrome/browser/preferences/SearchEngineAdapter.java
[modify] https://crrev.com/6deaae488ed3eb805a385c453507a1ebac0edf80/chrome/android/java/src/org/chromium/chrome/browser/preferences/website/SingleWebsitePreferences.java
[modify] https://crrev.com/6deaae488ed3eb805a385c453507a1ebac0edf80/chrome/android/java/strings/android_chrome_strings.grd

Project Member

Comment 17 by bugdroid1@chromium.org, Dec 12 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/582b431390ac564d3074b3cbdce146c201cb8c7b

commit 582b431390ac564d3074b3cbdce146c201cb8c7b
Author: Raymes Khoury <raymes@chromium.org>
Date: Tue Dec 12 23:54:30 2017

Add features::kGrantNotificationsToDSE to about:flags

This adds features::kGrantNotificationsToDSE to about flags for easy
toggling for testing purposes.

Bug:  780344 
Change-Id: Iffbfe175fbafea5aef368255a226a7780cc3ab44
Reviewed-on: https://chromium-review.googlesource.com/821991
Commit-Queue: Raymes Khoury <raymes@chromium.org>
Reviewed-by: Ben Wells <benwells@chromium.org>
Cr-Commit-Position: refs/heads/master@{#523611}
[modify] https://crrev.com/582b431390ac564d3074b3cbdce146c201cb8c7b/chrome/browser/about_flags.cc
[modify] https://crrev.com/582b431390ac564d3074b3cbdce146c201cb8c7b/chrome/browser/flag_descriptions.cc
[modify] https://crrev.com/582b431390ac564d3074b3cbdce146c201cb8c7b/chrome/browser/flag_descriptions.h
[modify] https://crrev.com/582b431390ac564d3074b3cbdce146c201cb8c7b/tools/metrics/histograms/enums.xml

Labels: -Hotlist-EnamelAndFriendsFixIt
Status: Fixed (was: Assigned)

Sign in to add a comment