Record Android revokes in Permissions.Action.Notification.

This CL modifies fixes a bug in which metrics were not being recorded on
Android when notification permissions were revoked. It adds a missing
ScopedRevocationReporter for Chrome-visible changes, and adds detection
code for when notification permissions are revoked in Android O+
system channel settings.

Bug: 782126
Change-Id: I609e909936d09e6dd948f0601bbf73ecdb6b8b75
Reviewed-on: https://chromium-review.googlesource.com/c/1324394
Commit-Queue: Joe DeBlasio <jdeblasio@chromium.org>
Reviewed-by: Theresa <twellington@chromium.org>
Reviewed-by: Dominick Ng <dominickn@chromium.org>
Reviewed-by: Bryan McQuade <bmcquade@chromium.org>
Reviewed-by: Finnur Thorarinsson <finnur@chromium.org>
Reviewed-by: Christopher Thompson <cthomp@chromium.org>
Cr-Commit-Position: refs/heads/master@{#606887}

chrome/android/java/src/org/chromium/chrome/browser/preferences/website/SingleWebsitePreferences.java

@@ -102,6 +102,10 @@ public class SingleWebsitePreferences extends PreferenceFragment
     // The number of chosen object permissions displayed.
     private int mObjectPermissionCount;
 
+    // Records previous notification permission on Android O+ to allow detection of permission
+    // revocation within the Android system permission activity.
+    private ContentSetting mPreviousNotificationPermission;
+
     private class SingleWebsitePermissionsPopulator
             implements WebsitePermissionsFetcher.WebsitePermissionsCallback {
         private final WebsiteAddress mSiteAddress;
@@ -409,6 +413,9 @@ public class SingleWebsitePreferences extends PreferenceFragment
     }
 
     private void launchOsChannelSettings(Context context, String channelId) {
+        // Store current value of permission to allow comparison against new value at return.
+        mPreviousNotificationPermission = mSite.getPermission(PermissionInfo.Type.NOTIFICATION);
+
         Intent intent = new Intent(Settings.ACTION_CHANNEL_NOTIFICATION_SETTINGS);
         intent.putExtra(Settings.EXTRA_CHANNEL_ID, channelId);
         intent.putExtra(Settings.EXTRA_APP_PACKAGE, context.getPackageName());
@@ -437,6 +444,21 @@ public class SingleWebsitePreferences extends PreferenceFragment
             if (notificationsPreference != null) {
                 setUpNotificationsPreference(notificationsPreference);
             }
+
+            // To ensure UMA receives notification revocations, we detect if the setting has changed
+            // after returning to Chrome.  This is lossy, as it will miss when users revoke a
+            // permission, but do not return immediately to Chrome (e.g. they close the permissions
+            // activity, instead of hitting the back button), but prevents us from having to check
+            // for changes each time Chrome becomes active.
+            ContentSetting newPermission = mSite.getPermission(PermissionInfo.Type.NOTIFICATION);
+            if (mPreviousNotificationPermission == ContentSetting.ALLOW &&
+                    newPermission != ContentSetting.ALLOW) {
+                WebsitePreferenceBridge.nativeReportNotificationRevokedForOrigin(
+                    mSite.getAddress().getOrigin(),
+                    newPermission.toInt(),
+                    mSite.getPermissionInfo(PermissionInfo.Type.NOTIFICATION).isIncognito());
+                mPreviousNotificationPermission = null;
+            }
         }
     }
 

chrome/android/java/src/org/chromium/chrome/browser/preferences/website/WebsitePreferenceBridge.java

@@ -249,6 +249,8 @@ public abstract class WebsitePreferenceBridge {
             String origin, String embedder, int value, boolean isIncognito);
     static native void nativeSetNotificationSettingForOrigin(
             String origin, int value, boolean isIncognito);
+    static native void nativeReportNotificationRevokedForOrigin(
+            String origin, int newSettingValue, boolean isIncognito);
     static native void nativeSetProtectedMediaIdentifierSettingForOrigin(
             String origin, String embedder, int value, boolean isIncognito);
     static native void nativeSetSensorsSettingForOrigin(

chrome/browser/android/preferences/website_preference_bridge.cc

@@ -427,6 +427,11 @@ static void JNI_WebsitePreferenceBridge_SetNotificationSettingForOrigin(
     const JavaParamRef<jstring>& origin,
     jint value,
     jboolean is_incognito) {
+  // Note: For Android O+, SetNotificationSettingForOrigin is only called when
+  // the "Clear & Reset" button in Site Settings is pressed. Otherwise, we rely
+  // on ReportNotificationRevokedForOrigin to explicitly record metrics
+  // when we detect changes initiated in Android.
+  //
   // Note: Web Notification permission behaves differently from all other
   // permission types. See https://crbug.com/416894.
   Profile* profile = GetActiveUserProfile(is_incognito);
@@ -438,16 +443,42 @@ static void JNI_WebsitePreferenceBridge_SetNotificationSettingForOrigin(
         url, CONTENT_SETTINGS_TYPE_NOTIFICATIONS);
   }
 
-  if (MaybeResetDSEPermission(CONTENT_SETTINGS_TYPE_NOTIFICATIONS, url, url,
+  if (MaybeResetDSEPermission(CONTENT_SETTINGS_TYPE_NOTIFICATIONS, url, GURL(),
                               is_incognito, setting)) {
     return;
   }
 
+  PermissionUtil::ScopedRevocationReporter scoped_revocation_reporter(
+      profile, url, GURL(), CONTENT_SETTINGS_TYPE_NOTIFICATIONS,
+      PermissionSourceUI::SITE_SETTINGS);
+
   NotificationPermissionContext::UpdatePermission(profile, url, setting);
   WebSiteSettingsUmaUtil::LogPermissionChange(
       CONTENT_SETTINGS_TYPE_NOTIFICATIONS, setting);
 }
 
+// In Android O+, Android is responsible for revoking notification settings--
+// We detect this change and explicitly report it back for UMA reporting.
+static void JNI_WebsitePreferenceBridge_ReportNotificationRevokedForOrigin(
+    JNIEnv* env,
+    const JavaParamRef<jclass>& clazz,
+    const JavaParamRef<jstring>& origin,
+    jint new_setting_value,
+    jboolean is_incognito) {
+  Profile* profile = GetActiveUserProfile(is_incognito);
+  GURL url = GURL(ConvertJavaStringToUTF8(env, origin));
+
+  ContentSetting setting = static_cast<ContentSetting>(new_setting_value);
+  DCHECK_NE(setting, CONTENT_SETTING_ALLOW);
+
+  WebSiteSettingsUmaUtil::LogPermissionChange(
+      CONTENT_SETTINGS_TYPE_NOTIFICATIONS, setting);
+
+  PermissionUmaUtil::PermissionRevoked(CONTENT_SETTINGS_TYPE_NOTIFICATIONS,
+                                       PermissionSourceUI::ANDROID_SETTINGS,
+                                       url.GetOrigin(), profile);
+}
+
 static void JNI_WebsitePreferenceBridge_GetCameraOrigins(
     JNIEnv* env,
     const JavaParamRef<jclass>& clazz,

chrome/browser/permissions/permission_uma_util.h

@@ -28,6 +28,7 @@ enum class PermissionSourceUI {
   OIB = 1,
   SITE_SETTINGS = 2,
   PAGE_ACTION = 3,
+  ANDROID_SETTINGS = 4,
 
   // Always keep this at the end.
   NUM,

tools/metrics/ukm/ukm.xml

@@ -3805,7 +3805,6 @@ be describing additional metrics about the same event.
   <metric name="Source">
     <summary>
       An enum of type PermissionSourceUI. The UI surface for this action.
-      Currently we only record when this is PROMPT.
     </summary>
   </metric>
 </event>