Stop showing notifications if they're ignored

chrome/android/java/src/org/chromium/chrome/browser/ntp/ContentSuggestionsNotificationHelper.java

Index: chrome/android/java/src/org/chromium/chrome/browser/ntp/ContentSuggestionsNotificationHelper.java
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/ntp/ContentSuggestionsNotificationHelper.java b/chrome/android/java/src/org/chromium/chrome/browser/ntp/ContentSuggestionsNotificationHelper.java
index 337a3a997a905b558711614896dae30c57f4c9ae..62aaf1fe9c1380844f84ab9323b2fa73d70904d4 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/ntp/ContentSuggestionsNotificationHelper.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/ntp/ContentSuggestionsNotificationHelper.java
@@ -37,6 +37,25 @@ public class ContentSuggestionsNotificationHelper {
     private ContentSuggestionsNotificationHelper() {} // Prevent instantiation
 
     /**
+     * Opens the content suggestion when notification is tapped.
+     */
+    public static final class OpenUrlReceiver extends BroadcastReceiver {
+        public void onReceive(Context context, Intent intent) {
+            openUrl(intent.getData());
+            nativeNotificationTapped();

[Bernhard Bauer, 2017/01/11 15:21:49]

Do you actually know that the native library has been loaded at this point? What
if Chrome had been killed in the mean time?

[sfiera, 2017/01/11 15:41:51]

Ah, no, I don't (checked; it may not be). I really don't know about this sort of
thing. How do I start it up?

[Bernhard Bauer, 2017/01/11 15:58:24]

That's the thing. Loading the native library is quite expensive (around 300ms I
think?), and broadcasts are received on the UI thread, so you don't really want
to block it while you're initializing the native side. I think you'll just have
to make do with staying on the Android side of things. You can use the Android
SharedPreferences class to store your values, and then call into Java from
native to read it.

[sfiera, 2017/01/12 11:26:23]

In the OpenUrlReceiver, we're going to be spinning up the native library anyway,
to actually open the URL, if there's a way to not block. In the other two, I
guess it would be good to not load the native library.

I can store preferences on the Java side, but can I also update histograms? If
not, is there a pattern for updating histograms later, when the native library
starts up? I assume we could get bad timestamps on our data points, but probably
not more than a day, given our fetching schedule.

[Bernhard Bauer, 2017/01/12 12:13:52]

What do you mean by not block?
Yup. (The difference in openUrl() is that it starts a new activity, which can do
all sorts of things, as it is user-visible and in the foreground. The receiver
itself OTOH runs hidden in the background, so doing long-running and/or
intensive work in there is discouraged).
There is CachedMetrics for recording metrics before native startup, but that
only caches in memory, so you'd have to also serialize the queued up metrics in
Java.

Do we actually use any metrics with timestamps? AFAIK, histograms just get
accumulated without recording a timestamp (UserMetricsAction use timestamps, but
we don't seem to record any of these).

[sfiera, 2017/01/12 12:50:24]

Here, it's not safe yet to call the native method, because openUrl() is
asynchronous. If we could schedule something to update the metrics later,
without blocking inside this function, it would be fine—basically, CachedMetrics
would work perfectly here, I think.
What about notifications from websites? They seem to be hooked up to call into
native code when dismissed, but don't display anything.
So, for the dismiss/timeout cases, CachedMetrics is helpful as an example, but
isn't usable in our own implementation, is it? What should I be looking at for
the serialization?

I feel like I'm out of my element here, with my primitive mental model of Chrome
as either having started up or not :)
It's not explicitly recorded, but something lets UMA graph histograms over time,
so there's some sort of implicit timestamp (from collection time, maybe). If we
wanted to look at the timeline of a metric, we might want to keep in mind that
it could be off by a day, but off by a day is probably fine.

+        }
+    }
+
+    /**
+     * Records dismissal when notification is swiped away.
+     */
+    public static final class DeleteReceiver extends BroadcastReceiver {
+        public void onReceive(Context context, Intent intent) {
+            nativeNotificationDismissed();
+        }
+    }
+
+    /**
      * Removes the notification after a timeout period.
      */
     public static final class TimeoutReceiver extends BroadcastReceiver {
@@ -44,19 +63,19 @@ public class ContentSuggestionsNotificationHelper {
             int id = intent.getIntExtra(NOTIFICATION_ID_EXTRA, -1);
             if (id < 0) return;
             hideNotification(id);
+            nativeNotificationExpired();
         }
     }
 
-    @CalledByNative
-    private static void openUrl(String url) {
+    private static void openUrl(Uri uri) {
         Context context = ContextUtils.getApplicationContext();
-        Intent intent = new Intent();
-        intent.setAction(Intent.ACTION_VIEW);
-        intent.setData(Uri.parse(url));
-        intent.setClass(context, ChromeLauncherActivity.class);
-        intent.addFlags(Intent.FLAG_ACTIVITY_NEW_TASK);
-        intent.putExtra(Browser.EXTRA_APPLICATION_ID, context.getPackageName());
-        intent.putExtra(ShortcutHelper.REUSE_URL_MATCHING_TAB_ELSE_NEW_TAB, true);
+        Intent intent = new Intent()
+                                .setAction(Intent.ACTION_VIEW)
+                                .setData(uri)
+                                .setClass(context, ChromeLauncherActivity.class)
+                                .addFlags(Intent.FLAG_ACTIVITY_NEW_TASK)
+                                .putExtra(Browser.EXTRA_APPLICATION_ID, context.getPackageName())
+                                .putExtra(ShortcutHelper.REUSE_URL_MATCHING_TAB_ELSE_NEW_TAB, true);
         IntentHandler.addTrustedIntentExtras(intent);
         context.startActivity(intent);
     }
@@ -81,16 +100,13 @@ public class ContentSuggestionsNotificationHelper {
             }
         }
 
-        Intent intent = new Intent()
-                                .setAction(Intent.ACTION_VIEW)
-                                .setData(Uri.parse(url))
-                                .setClass(context, ChromeLauncherActivity.class)
-                                .putExtra(ShortcutHelper.REUSE_URL_MATCHING_TAB_ELSE_NEW_TAB, true)
-                                .putExtra(Browser.EXTRA_APPLICATION_ID, context.getPackageName());
+        Intent contentIntent = new Intent(context, OpenUrlReceiver.class).setData(Uri.parse(url));
+        Intent deleteIntent = new Intent(context, DeleteReceiver.class).setData(Uri.parse(url));
         NotificationCompat.Builder builder =
                 new NotificationCompat.Builder(context)
                         .setAutoCancel(true)
-                        .setContentIntent(PendingIntent.getActivity(context, 0, intent, 0))
+                        .setContentIntent(PendingIntent.getBroadcast(context, 0, contentIntent, 0))
+                        .setDeleteIntent(PendingIntent.getBroadcast(context, 0, deleteIntent, 0))
                         .setContentTitle(title)
                         .setContentText(text)
                         .setGroup(NOTIFICATION_TAG)
@@ -134,4 +150,8 @@ public class ContentSuggestionsNotificationHelper {
             manager.cancel(NOTIFICATION_TAG, 0);
         }
     }
+
+    private static native void nativeNotificationTapped();
+    private static native void nativeNotificationDismissed();
+    private static native void nativeNotificationExpired();
 }