Enable leaving notification embargo.

There is no notification channel when the notifiction permission for
the origin is merely embargoed, so it is not possible to intent to
notification channel settings.

This CL updates the SingleWebsitePreferences page on Android, so that
when the automatically blocked setting is changed for the first time,
a synthetic CONTENT_SETTING_BLOCKED is directly written just-in-time.
This will trigger lifting embargo, and create an initially disabled
notification channel, and finally we intent into channel settings.

Bug: 996201
Change-Id: Ice838b1ca971c9d8117f1c56ecc107bf41417550
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1970473Reviewed-by: Natalie Chouinard <chouinard@chromium.org>
Reviewed-by: Finnur Thorarinsson <finnur@chromium.org>
Commit-Queue: Andy Paicu <andypaicu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#729811}

chrome/android/java/src/org/chromium/chrome/browser/settings/website/SingleWebsiteSettings.java

@@ -31,6 +31,7 @@ import org.chromium.chrome.R;
 import org.chromium.chrome.browser.browserservices.Origin;
 import org.chromium.chrome.browser.browserservices.permissiondelegation.TrustedWebActivityPermissionManager;
 import org.chromium.chrome.browser.notifications.channels.SiteChannelsManager;
+import org.chromium.chrome.browser.profiles.Profile;
 import org.chromium.chrome.browser.settings.ChromeImageViewPreference;
 import org.chromium.chrome.browser.settings.ManagedPreferenceDelegate;
 import org.chromium.chrome.browser.settings.ManagedPreferencesUtils;
@@ -473,15 +474,7 @@ public class SingleWebsiteSettings extends PreferenceFragmentCompat
             newPreference.setDefaultValue(value);
 
             newPreference.setOnPreferenceClickListener(unused -> {
-                // There is no guarantee that a channel has been initialized yet for sites
-                // that were granted permission before the channel-initialization-on-grant
-                // code was in place. However, getChannelIdForOrigin will fall back to the
-                // generic Sites channel if no specific channel has been created for the given
-                // origin, so it is safe to open the channel settings for whatever channel ID
-                // it returns.
-                String channelId = SiteChannelsManager.getInstance().getChannelIdForOrigin(
-                        mSite.getAddress().getOrigin());
-                launchOsChannelSettings(preference.getContext(), channelId);
+                launchOsChannelSettingsFromPreference(preference);
                 return true;
             });
         } else {
@@ -492,6 +485,29 @@ public class SingleWebsiteSettings extends PreferenceFragmentCompat
         }
     }
 
+    // This is implemented as a public utility function to better facilitate testing.
+    public void launchOsChannelSettingsFromPreference(Preference preference) {
+        final boolean blockedByEmbargo =
+                (WebsitePreferenceBridgeJni.get().isNotificationEmbargoedForOrigin(
+                        Profile.getLastUsedProfile(), mSite.getAddress().getOrigin()));
+
+        // There is no notification channel if the origin is merely embargoed. Create it
+        // just-in-time if the user tries to change to setting.
+        if (blockedByEmbargo) {
+            mSite.setPermission(PermissionInfo.Type.NOTIFICATION, ContentSettingValues.BLOCK);
+        }
+
+        // There is no guarantee that a channel has been initialized yet for sites
+        // that were granted permission before the channel-initialization-on-grant
+        // code was in place. However, getChannelIdForOrigin will fall back to the
+        // generic Sites channel if no specific channel has been created for the given
+        // origin, so it is safe to open the channel settings for whatever channel ID
+        // it returns.
+        String channelId = SiteChannelsManager.getInstance().getChannelIdForOrigin(
+                mSite.getAddress().getOrigin());
+        launchOsChannelSettings(preference.getContext(), channelId);
+    }
+
     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);

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

@@ -562,6 +562,7 @@ public class WebsitePreferenceBridge {
         int getMidiSettingForOrigin(String origin, String embedder, boolean isIncognito);
         int getNfcSettingForOrigin(String origin, String embedder, boolean isIncognito);
         int getNotificationSettingForOrigin(String origin, boolean isIncognito);
+        boolean isNotificationEmbargoedForOrigin(Profile profile, String origin);
         int getProtectedMediaIdentifierSettingForOrigin(
                 String origin, String embedder, boolean isIncognito);
         int getSensorsSettingForOrigin(String origin, String embedder, boolean isIncognito);

chrome/android/javatests/src/org/chromium/chrome/browser/settings/website/SiteSettingsTest.java

@@ -6,6 +6,7 @@ package org.chromium.chrome.browser.settings.website;
 
 import static org.chromium.chrome.browser.settings.website.WebsitePreferenceBridge.SITE_WILDCARD;
 
+import android.content.Intent;
 import android.os.Build;
 import android.support.test.InstrumentationRegistry;
 import android.support.test.filters.SmallTest;
@@ -28,7 +29,10 @@ import org.chromium.base.test.util.RetryOnFailure;
 import org.chromium.chrome.browser.ChromeActivity;
 import org.chromium.chrome.browser.ChromeFeatureList;
 import org.chromium.chrome.browser.ChromeSwitches;
+import org.chromium.chrome.browser.browserservices.Origin;
 import org.chromium.chrome.browser.infobar.InfoBarContainer;
+import org.chromium.chrome.browser.notifications.channels.ChannelDefinitions;
+import org.chromium.chrome.browser.notifications.channels.SiteChannelsManager;
 import org.chromium.chrome.browser.preferences.Pref;
 import org.chromium.chrome.browser.preferences.PrefServiceBridge;
 import org.chromium.chrome.browser.settings.ChromeBaseCheckBoxPreference;
@@ -36,6 +40,7 @@ import org.chromium.chrome.browser.settings.ChromeSwitchPreference;
 import org.chromium.chrome.browser.settings.LocationSettings;
 import org.chromium.chrome.browser.settings.NfcSystemLevelSetting;
 import org.chromium.chrome.browser.settings.SettingsActivity;
+import org.chromium.chrome.browser.settings.SettingsLauncher;
 import org.chromium.chrome.test.ChromeActivityTestRule;
 import org.chromium.chrome.test.ChromeJUnit4ClassRunner;
 import org.chromium.chrome.test.util.InfoBarTestAnimationListener;
@@ -45,6 +50,7 @@ import org.chromium.components.content_settings.ContentSettingsType;
 import org.chromium.content_public.browser.test.util.TestThreadUtils;
 import org.chromium.content_public.common.ContentSwitches;
 import org.chromium.net.test.EmbeddedTestServer;
+import org.chromium.net.test.ServerCertificate;
 
 import java.util.ArrayList;
 import java.util.Arrays;
@@ -56,10 +62,8 @@ import java.util.concurrent.Callable;
  */
 @RunWith(ChromeJUnit4ClassRunner.class)
 @RetryOnFailure
-@CommandLineFlags.Add({
-        ChromeSwitches.DISABLE_FIRST_RUN_EXPERIENCE,
-        ContentSwitches.HOST_RESOLVER_RULES + "=MAP * 127.0.0.1",
-})
+@CommandLineFlags.Add({ChromeSwitches.DISABLE_FIRST_RUN_EXPERIENCE,
+        ContentSwitches.HOST_RESOLVER_RULES + "=MAP * 127.0.0.1", "ignore-certificate-errors"})
 public class SiteSettingsTest {
     @Rule
     public ChromeActivityTestRule<ChromeActivity> mActivityTestRule =
@@ -70,7 +74,8 @@ public class SiteSettingsTest {
     @Before
     public void setUp() throws Exception {
         mActivityTestRule.startMainActivityOnBlankPage();
-        mTestServer = EmbeddedTestServer.createAndStartServer(InstrumentationRegistry.getContext());
+        mTestServer = EmbeddedTestServer.createAndStartHTTPSServer(
+                InstrumentationRegistry.getContext(), ServerCertificate.CERT_OK);
     }
 
     @After
@@ -879,4 +884,45 @@ public class SiteSettingsTest {
         return TestThreadUtils.runOnUiThreadBlockingNoException(
                 () -> mActivityTestRule.getActivity().getTabModelSelector().getTotalTabCount());
     }
+
+    @Test
+    @SmallTest
+    @Feature({"Preferences"})
+    public void testEmbargoedNotificationSiteSettings() throws Exception {
+        if (Build.VERSION.SDK_INT < Build.VERSION_CODES.O) return;
+        final String url = mTestServer.getURLWithHostName(
+                "example.com", "/chrome/test/data/notifications/notification_tester.html");
+
+        // Ignore notification request 4 times to enter embargo. 5th one ensures that notifications
+        // are blocked by actually causing a deny-by-embargo.
+        for (int i = 0; i < 5; i++) {
+            mActivityTestRule.loadUrl(url);
+            mActivityTestRule.runJavaScriptCodeInCurrentTab("requestPermissionAndRespond()");
+        }
+
+        Intent intent = SettingsLauncher.createIntentForSettingsPage(
+                InstrumentationRegistry.getTargetContext(), SingleWebsiteSettings.class.getName(),
+                SingleWebsiteSettings.createFragmentArgsForSite(url));
+        final SettingsActivity settingsActivity =
+                (SettingsActivity) InstrumentationRegistry.getInstrumentation().startActivitySync(
+                        intent);
+
+        TestThreadUtils.runOnUiThreadBlocking(() -> {
+            final SingleWebsiteSettings websitePreferences =
+                    (SingleWebsiteSettings) settingsActivity.getMainFragment();
+
+            final Preference notificationPreference =
+                    websitePreferences.findPreference("push_notifications_list");
+
+            websitePreferences.launchOsChannelSettingsFromPreference(notificationPreference);
+
+            // Ensure that a proper separate channel has indeed been created to allow the user to
+            // alter the setting.
+            Assert.assertNotEquals(ChannelDefinitions.ChannelId.SITES,
+                    SiteChannelsManager.getInstance().getChannelIdForOrigin(
+                            Origin.createOrThrow(url).toString()));
+        });
+
+        settingsActivity.finish();
+    }
 }

chrome/browser/android/preferences/website_preference_bridge.cc

@@ -477,15 +477,32 @@ static jint JNI_WebsitePreferenceBridge_GetNotificationSettingForOrigin(
       env, ContentSettingsType::NOTIFICATIONS, origin, origin, is_incognito);
 }
 
+static jboolean JNI_WebsitePreferenceBridge_IsNotificationEmbargoedForOrigin(
+    JNIEnv* env,
+    const JavaParamRef<jobject>& jprofile,
+    const JavaParamRef<jstring>& origin) {
+  GURL origin_url(ConvertJavaStringToUTF8(env, origin));
+  PermissionResult status =
+      PermissionManager::Get(ProfileAndroid::FromProfileAndroid(jprofile))
+          ->GetPermissionStatus(ContentSettingsType::NOTIFICATIONS, origin_url,
+                                origin_url);
+  return status.content_setting == ContentSetting::CONTENT_SETTING_BLOCK &&
+         (status.source == PermissionStatusSource::MULTIPLE_IGNORES ||
+          status.source == PermissionStatusSource::MULTIPLE_DISMISSALS);
+}
+
 static void JNI_WebsitePreferenceBridge_SetNotificationSettingForOrigin(
     JNIEnv* env,
     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: For Android O+, SetNotificationSettingForOrigin is only called when:
+  //  1) the "Clear & Reset" button in Site Settings is pressed,
+  //  2) the notification permission is blocked by embargo, so no notification
+  //     channel exists yet, and in this state the user changes the setting to
+  //     allow or "real" block in SingleWebsitePreferences.
+  // 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.
@@ -493,10 +510,8 @@ static void JNI_WebsitePreferenceBridge_SetNotificationSettingForOrigin(
   GURL url = GURL(ConvertJavaStringToUTF8(env, origin));
   ContentSetting setting = static_cast<ContentSetting>(value);
 
-  if (setting != CONTENT_SETTING_BLOCK) {
-    PermissionDecisionAutoBlocker::GetForProfile(profile)->RemoveEmbargoByUrl(
-        url, ContentSettingsType::NOTIFICATIONS);
-  }
+  PermissionDecisionAutoBlocker::GetForProfile(profile)->RemoveEmbargoByUrl(
+      url, ContentSettingsType::NOTIFICATIONS);
 
   if (MaybeResetDSEPermission(ContentSettingsType::NOTIFICATIONS, url, GURL(),
                               is_incognito, setting)) {