Handle the notification settings intent.

This intent comes from two places. Since Android L there is a settings cog on two UI surfaces: (1) On the rear side (after long press) of notifications. (2) In the "App notifications" screen in Android Settings. Touching the cog on a flipped notification takes the user to the site specific preferences screen which displays (among others) the notification preference for the site origin. Touching he cog on the App notifications screen takes the user to Chrome's notifications preferences list for all sites. BUG=436594,461885 [email protected],[email protected] Review URL: https://codereview.chromium.org/942103003 Cr-Commit-Position: refs/heads/master@{#318097} (cherry picked from commit efff60a) Review URL: https://codereview.chromium.org/967593002 Cr-Commit-Position: refs/branch-heads/2311@{#50} Cr-Branched-From: 09b7de5-refs/heads/master@{#317474}
crosswalk-project · Feb 27, 2015 · fc9ff20 · fc9ff20
1 parent b435753
commit fc9ff20
Show file tree

Hide file tree

Showing 9 changed files with 144 additions and 76 deletions.
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationConstants.java b/chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationConstants.java
@@ -15,6 +15,7 @@ public class NotificationConstants {
             "org.chromium.chrome.browser.notifications.CLOSE_NOTIFICATION";
 
     public static final String EXTRA_NOTIFICATION_ID = "notification_id";
+    public static final String EXTRA_NOTIFICATION_TAG = "notification_tag";
 
     // TODO(peter): Remove these extras once Notifications are powered by a database on the
     // native side, that contains all the additional information.
@@ -30,4 +31,10 @@ public class NotificationConstants {
      * Unique identifier for the "Signed in to Chrome" notification.
      */
     public static final int NOTIFICATION_ID_SIGNED_IN = 2;
+
+    /**
+     * Separator used to separate the notification origin from additional data such as the
+     * developer specified tag.
+     */
+    public static final String NOTIFICATION_TAG_SEPARATOR = ";";
 }
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationUIManager.java b/chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationUIManager.java
@@ -12,14 +12,18 @@
 import android.content.res.Resources;
 import android.graphics.Bitmap;
 import android.graphics.Color;
+import android.os.Bundle;
 import android.support.v4.app.NotificationCompat;
+import android.text.TextUtils;
 import android.util.Log;
 
 import org.chromium.base.CalledByNative;
 import org.chromium.chrome.R;
 import org.chromium.chrome.browser.preferences.Preferences;
 import org.chromium.chrome.browser.preferences.PreferencesLauncher;
 import org.chromium.chrome.browser.preferences.website.SingleWebsitePreferences;
+import org.chromium.chrome.browser.preferences.website.WebsitePreferences;
+import org.chromium.chrome.browser.preferences.website.WebsiteSettingsCategoryFilter;
 import org.chromium.chrome.browser.widget.RoundedIconGenerator;
 
 /**
@@ -121,6 +125,59 @@ public static boolean dispatchNotificationEvent(Intent intent) {
         return false;
     }
 
+    /**
+     * Launches the notifications preferences screen. If the received intent indicates it came
+     * from the gear button on a flipped notification, this launches the site specific preferences
+     * screen.
+     *
+     * @param context The context that received the intent.
+     * @param incomingIntent The received intent.
+     */
+    public static void launchNotificationPreferences(Context context, Intent incomingIntent) {
+        // Use the application context because it lives longer. When using he given context, it
+        // may be stopped before the preferences intent is handled.
+        Context applicationContext = context.getApplicationContext();
+
+        // If the user touched the settings cog on a flipped notification there will be a
+        // notification tag extra. From the tag we can read the origin of the notification, and use
+        // that to open the settings screen for that specific origin.
+        String notificationTag =
+                incomingIntent.getStringExtra(NotificationConstants.EXTRA_NOTIFICATION_TAG);
+        boolean launchSingleWebsitePreferences = notificationTag != null;
+
+        String fragmentName = launchSingleWebsitePreferences
+                ? SingleWebsitePreferences.class.getName()
+                : WebsitePreferences.class.getName();
+        Intent preferencesIntent =
+                PreferencesLauncher.createIntentForSettingsPage(applicationContext, fragmentName);
+
+        Bundle fragmentArguments;
+        if (launchSingleWebsitePreferences) {
+            // All preferences for a specific origin.
+            String[] tagParts =
+                    notificationTag.split(NotificationConstants.NOTIFICATION_TAG_SEPARATOR);
+            assert tagParts.length >= 2;
+            String origin = tagParts[0];
+            fragmentArguments = SingleWebsitePreferences.createFragmentArgsForSite(origin);
+        } else {
+            // Notification preferences for all origins.
+            fragmentArguments = new Bundle();
+            fragmentArguments.putString(WebsitePreferences.EXTRA_CATEGORY,
+                    WebsiteSettingsCategoryFilter.FILTER_PUSH_NOTIFICATIONS);
+            fragmentArguments.putString(WebsitePreferences.EXTRA_TITLE,
+                    applicationContext.getResources().getString(
+                            R.string.push_notifications_permission_title));
+        }
+        preferencesIntent.putExtra(Preferences.EXTRA_SHOW_FRAGMENT_ARGUMENTS, fragmentArguments);
+
+        // When coming from the gear on a flipped notification, we need to ensure that no existing
+        // preference tasks are being re-used in order for it to appear on top.
+        if (launchSingleWebsitePreferences)
+            preferencesIntent.addFlags(Intent.FLAG_ACTIVITY_CLEAR_TASK);
+
+        applicationContext.startActivity(preferencesIntent);
+    }
+
     private PendingIntent getPendingIntent(String action, String notificationId, int platformId,
                                            byte[] notificationData) {
         Intent intent = new Intent(action);
@@ -133,9 +190,19 @@ private PendingIntent getPendingIntent(String action, String notificationId, int
                 PendingIntent.FLAG_UPDATE_CURRENT);
     }
 
+    private static String makePlatformTag(String tag, int platformId, String origin) {
+        // The given tag may contain the separator character, so add it last to make reading the
+        // preceding origin token reliable. If no tag was specified (it is the default empty
+        // string), make the platform tag unique by appending the platform id.
+        String platformTag = origin + NotificationConstants.NOTIFICATION_TAG_SEPARATOR + tag;
+        if (TextUtils.isEmpty(tag)) platformTag += platformId;
+        return platformTag;
+    }
+
     /**
      * Displays a notification with the given |notificationId|, |title|, |body| and |icon|.
      *
+     * @param tag A string identifier for this notification.
      * @param notificationId Unique id provided by the Chrome Notification system.
      * @param title Title to be displayed in the notification.
      * @param body Message to be displayed in the notification. Will be trimmed to one line of
@@ -147,8 +214,8 @@ private PendingIntent getPendingIntent(String action, String notificationId, int
      * @return The id using which the notification can be identified.
      */
     @CalledByNative
-    private int displayNotification(String notificationId, String title, String body, Bitmap icon,
-                                    String origin, byte[] notificationData) {
+    private int displayNotification(String tag, String notificationId, String title, String body,
+            Bitmap icon, String origin, byte[] notificationData) {
         if (icon == null || icon.getWidth() == 0) {
             icon = getIconGenerator().generateIconForUrl(origin, true);
         }
@@ -181,7 +248,8 @@ private int displayNotification(String notificationId, String title, String body
                            pendingSettingsIntent)
                 .setSubText(origin);
 
-        mNotificationManager.notify(mLastNotificationId, notificationBuilder.build());
+        String platformTag = makePlatformTag(tag, mLastNotificationId, origin);
+        mNotificationManager.notify(platformTag, mLastNotificationId, notificationBuilder.build());
 
         return mLastNotificationId++;
     }
@@ -213,11 +281,12 @@ private RoundedIconGenerator getIconGenerator() {
     }
 
     /**
-     * Closes the notification identified by |platformId|.
+     * Closes the notification identified by |tag|, |platformId|, and |origin|.
      */
     @CalledByNative
-    private void closeNotification(int platformId) {
-        mNotificationManager.cancel(platformId);
+    private void closeNotification(String tag, int platformId, String origin) {
+        String platformTag = makePlatformTag(tag, platformId, origin);
+        mNotificationManager.cancel(platformTag, platformId);
     }
 
     private boolean onNotificationClicked(String notificationId, int platformId,

diff --git a/chrome/android/java/src/org/chromium/chrome/browser/preferences/Preferences.java b/chrome/android/java/src/org/chromium/chrome/browser/preferences/Preferences.java
@@ -6,7 +6,6 @@
 
 import android.annotation.SuppressLint;
 import android.app.Fragment;
-import android.app.Notification;
 import android.content.Intent;
 import android.content.pm.ActivityInfo;
 import android.content.pm.PackageManager.NameNotFoundException;
@@ -28,8 +27,6 @@
 import org.chromium.base.annotations.SuppressFBWarnings;
 import org.chromium.base.library_loader.ProcessInitException;
 import org.chromium.chrome.R;
-import org.chromium.chrome.browser.preferences.website.WebsitePreferences;
-import org.chromium.chrome.browser.preferences.website.WebsiteSettingsCategoryFilter;
 
 /**
  * The Chrome settings activity.
@@ -139,23 +136,6 @@ protected void onCreate(Bundle savedInstanceState) {
         Bundle initialArguments = getIntent().getBundleExtra(EXTRA_SHOW_FRAGMENT_ARGUMENTS);
         boolean displayHomeAsUp = getIntent().getBooleanExtra(EXTRA_DISPLAY_HOME_AS_UP, true);
 
-        // The notification settings cog on the flipped side of Notifications and in the Android
-        // Settings "App Notifications" view will open us with a specific category rather than
-        // the default intent extras which are commonly used to open specific views.
-        if (getIntent().hasCategory(Notification.INTENT_CATEGORY_NOTIFICATION_PREFERENCES)) {
-            Bundle arguments = new Bundle();
-            arguments.putString(WebsitePreferences.EXTRA_CATEGORY,
-                    WebsiteSettingsCategoryFilter.FILTER_PUSH_NOTIFICATIONS);
-            arguments.putString(WebsitePreferences.EXTRA_TITLE,
-                    getResources().getString(R.string.push_notifications_permission_title));
-
-            // TODO(peter): Open the appropriate page for the origin in the Website Settings.
-
-            initialFragment = WebsitePreferences.class.getName();
-            initialArguments = arguments;
-            displayHomeAsUp = false;
-        }
-
         if (displayHomeAsUp) getSupportActionBar().setDisplayHomeAsUpEnabled(true);
 
         // This must be called before the fragment transaction below.

diff --git a/...id/java/src/org/chromium/chrome/browser/preferences/website/SingleWebsitePreferences.java b/...id/java/src/org/chromium/chrome/browser/preferences/website/SingleWebsitePreferences.java
@@ -39,7 +39,7 @@ public class SingleWebsitePreferences extends PreferenceFragment
     // Website object. If EXTRA_ADDRESS is present, the fragment will find all
     // permissions for that website address and display those.
     public static final String EXTRA_SITE = "org.chromium.chrome.preferences.site";
-    public static final String EXTRA_ADDRESS = "org.chromium.chrome.preferences.address";
+    public static final String EXTRA_ORIGIN = "org.chromium.chrome.preferences.origin";
 
     // Preference keys, see single_website_preferences.xml
     // Headings:
@@ -103,22 +103,21 @@ public static Bundle createFragmentArgsForSite(String url) {
         // TODO(mvanouwerkerk): Define a pure getOrigin method in UrlUtilities that is the
         // equivalent of the call below, because this is perfectly fine for non-display purposes.
         String origin = UrlUtilities.getOriginForDisplay(URI.create(url), true /*  schowScheme */);
-        fragmentArgs.putSerializable(
-                SingleWebsitePreferences.EXTRA_ADDRESS, WebsiteAddress.create(origin));
+        fragmentArgs.putString(SingleWebsitePreferences.EXTRA_ORIGIN, origin);
         return fragmentArgs;
     }
 
     @Override
     public void onActivityCreated(Bundle savedInstanceState) {
         getActivity().setTitle(R.string.prefs_content_settings);
         Object extraSite = getArguments().getSerializable(EXTRA_SITE);
-        Object extraAddress = getArguments().getSerializable(EXTRA_ADDRESS);
+        Object extraOrigin = getArguments().getSerializable(EXTRA_ORIGIN);
 
-        if (extraSite != null && extraAddress == null) {
+        if (extraSite != null && extraOrigin == null) {
             mSite = (Website) extraSite;
             displaySitePermissions();
-        } else if (extraAddress != null && extraSite == null) {
-            mSiteAddress = (WebsiteAddress) extraAddress;
+        } else if (extraOrigin != null && extraSite == null) {
+            mSiteAddress = WebsiteAddress.create((String) extraOrigin);
             WebsitePermissionsFetcher fetcher =
                     new WebsitePermissionsFetcher(new SingleWebsitePermissionsPopulator());
             fetcher.fetchAllPreferences();

diff --git a/chrome/android/javatests/src/org/chromium/chrome/browser/preferences/PreferencesTest.java b/chrome/android/javatests/src/org/chromium/chrome/browser/preferences/PreferencesTest.java
@@ -5,9 +5,7 @@
 package org.chromium.chrome.browser.preferences;
 
 import android.app.Activity;
-import android.app.Fragment;
 import android.app.Instrumentation;
-import android.app.Notification;
 import android.content.Context;
 import android.content.Intent;
 import android.preference.Preference;
@@ -18,7 +16,6 @@
 import org.chromium.base.ThreadUtils;
 import org.chromium.base.test.util.Feature;
 import org.chromium.chrome.browser.accessibility.FontSizePrefs;
-import org.chromium.chrome.browser.preferences.website.WebsitePreferences;
 import org.chromium.chrome.browser.search_engines.TemplateUrlService;
 import org.chromium.chrome.browser.search_engines.TemplateUrlService.LoadListener;
 import org.chromium.chrome.shell.ChromeShellTestBase;
@@ -125,25 +122,8 @@ public void run() {
         });
     }
 
-    /**
-     * Starts the preference activity as if it's been opened from the App Notification settings
-     * screen to verify that it opens the Notifications Site Settings screen.
-     */
-    @SmallTest
-    @Feature({"Preferences"})
-    public void testNotificationSettingsCategoryIntent() throws Exception {
-        Instrumentation instrumentation = getInstrumentation();
-
-        Intent intent = PreferencesLauncher.createIntentForSettingsPage(
-                instrumentation.getTargetContext(), null);
-        intent.addCategory(Notification.INTENT_CATEGORY_NOTIFICATION_PREFERENCES);
-
-        Activity activity = instrumentation.startActivitySync(intent);
-        assertTrue(activity instanceof Preferences);
-
-        Fragment fragment = ((Preferences) activity).getFragmentForTest();
-        assertTrue(fragment instanceof WebsitePreferences);
-    }
+    // TODO(mvanouwerkerk): Write new preference intent tests for notification settings.
+    // https://crbug.com/461885
 
     /**
      * Tests setting FontScaleFactor and ForceEnableZoom in AccessibilityPreferences and ensures

diff --git a/chrome/android/shell/java/AndroidManifest.xml.jinja2 b/chrome/android/shell/java/AndroidManifest.xml.jinja2
@@ -49,6 +49,7 @@
               <intent-filter>
                   <action android:name="android.intent.action.MAIN" />
                   <category android:name="android.intent.category.LAUNCHER" />
+                  <category android:name="android.intent.category.NOTIFICATION_PREFERENCES" />
               </intent-filter>
         </activity>
         <activity android:name="org.chromium.sync.test.util.MockGrantCredentialsPermissionActivity"
@@ -63,11 +64,6 @@
                 android:configChanges="orientation|keyboardHidden|keyboard|screenSize"
                 android:label="@string/preferences"
                 android:exported="false">
-            <intent-filter>
-                <action android:name="android.intent.action.MAIN" />
-                <category android:name="android.intent.category.DEFAULT" />
-                <category android:name="android.intent.category.NOTIFICATION_PREFERENCES" />
-            </intent-filter>
         </activity>
 
         <!-- The following service entries exist in order to allow us to

diff --git a/chrome/android/shell/java/src/org/chromium/chrome/shell/ChromeShellActivity.java b/chrome/android/shell/java/src/org/chromium/chrome/shell/ChromeShellActivity.java
@@ -6,6 +6,7 @@
 
 import android.app.Activity;
 import android.app.FragmentManager;
+import android.app.Notification;
 import android.content.Intent;
 import android.os.Bundle;
 import android.provider.Browser;
@@ -39,6 +40,7 @@
 import org.chromium.chrome.browser.identity.UuidBasedUniqueIdentificationGenerator;
 import org.chromium.chrome.browser.nfc.BeamController;
 import org.chromium.chrome.browser.nfc.BeamProvider;
+import org.chromium.chrome.browser.notifications.NotificationUIManager;
 import org.chromium.chrome.browser.preferences.PreferencesLauncher;
 import org.chromium.chrome.browser.printing.PrintingControllerFactory;
 import org.chromium.chrome.browser.printing.TabPrinter;
@@ -209,6 +211,12 @@ public String getTabUrlForBeam() {
                 return tab.getUrl();
             }
         });
+
+        // The notification settings cog on the flipped side of Notifications and in the Android
+        // Settings "App Notifications" view will open us with a specific category.
+        if (getIntent().hasCategory(Notification.INTENT_CATEGORY_NOTIFICATION_PREFERENCES)) {
+            NotificationUIManager.launchNotificationPreferences(this, getIntent());
+        }
     }
 
     @Override