Stop showing notifications if they're ignored

chrome/browser/android/ntp/content_suggestions_notification_helper.cc

Index: chrome/browser/android/ntp/content_suggestions_notification_helper.cc
diff --git a/chrome/browser/android/ntp/content_suggestions_notification_helper.cc b/chrome/browser/android/ntp/content_suggestions_notification_helper.cc
index db5abdd3b659d21147ce6a302283ebbb834b695f..c26fff3fbfe332c925d636ed03c0bb666976db0c 100644
--- a/chrome/browser/android/ntp/content_suggestions_notification_helper.cc
+++ b/chrome/browser/android/ntp/content_suggestions_notification_helper.cc
@@ -8,20 +8,24 @@
 
 #include "base/android/jni_android.h"
 #include "base/android/jni_string.h"
+#include "base/metrics/histogram_macros.h"
 #include "base/strings/utf_string_conversions.h"
+#include "chrome/browser/ntp_snippets/ntp_snippets_features.h"
+#include "chrome/browser/ntp_snippets/ntp_snippets_metrics.h"
+#include "chrome/browser/ntp_snippets/ntp_snippets_prefs.h"
+#include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/profiles/profile_manager.h"
+#include "components/prefs/pref_service.h"
+#include "components/variations/variations_associated_data.h"
 #include "jni/ContentSuggestionsNotificationHelper_jni.h"
 #include "ui/gfx/android/java_bitmap.h"
 #include "ui/gfx/image/image.h"
 #include "ui/gfx/image/image_skia.h"
 
+using base::android::JavaParamRef;
+
 namespace ntp_snippets {
 
-void ContentSuggestionsNotificationHelper::OpenURL(const GURL& url) {
-  JNIEnv* env = base::android::AttachCurrentThread();
-  Java_ContentSuggestionsNotificationHelper_openUrl(
-      env, base::android::ConvertUTF8ToJavaString(env, url.spec()));
-}
-
 void ContentSuggestionsNotificationHelper::SendNotification(
     const GURL& url,
     const base::string16& title,
@@ -50,4 +54,63 @@ void ContentSuggestionsNotificationHelper::HideAllNotifications() {
   Java_ContentSuggestionsNotificationHelper_hideAllNotifications(env);
 }
 
+bool ContentSuggestionsNotificationHelper::IsDisabledForProfile(
+    Profile* profile) {
+  auto prefs = profile->GetPrefs();

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

Use PrefService* in the other places as well?

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

Done.

+  int current =
+      prefs->GetInteger(kContentSuggestionsConsecutiveIgnoredPrefName);
+  int limit = variations::GetVariationParamByFeatureAsInt(
+      kContentSuggestionsNotificationsFeature,
+      kContentSuggestionsNotificationsIgnoredLimitParam,
+      kContentSuggestionsNotificationsIgnoredDefaultLimit);
+  return current >= limit;
+}
+
+// static
+bool ContentSuggestionsNotificationHelper::Register(JNIEnv* env) {
+  return RegisterNativesImpl(env);
+}
+
 }  // namespace ntp_snippets
+
+static bool IsDisabledForProfile(Profile* profile) {

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

Is this just a convenience method to get rid of
"ntp_snippets::ContentSuggestionsNotificationHelper::"? It should probably go
into an anonymous namespace instead of being marked static (the static is
necessary below so that the methods can be declared in the generated header
file, but not here), but then could you simply import it into the anonymous
namespace?

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

"using ContentSuggestionsNotificationHelper::IsDisabledForProfile" doesn't work
because it's a class member. Since the static functions are now in the
namespace, I moved the implementation to the anonymous namespace. The class
member is now just exposing that function to other files.

+  return ntp_snippets::ContentSuggestionsNotificationHelper::
+      IsDisabledForProfile(profile);
+}
+
+static void NotificationTapped(JNIEnv* env, const JavaParamRef<jclass>&) {

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

Add a commented out variable name for the class?

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

Done.

+  Profile* profile = ProfileManager::GetLastUsedProfile()->GetOriginalProfile();
+  PrefService* prefs = profile->GetPrefs();
+  prefs->SetInteger(kContentSuggestionsConsecutiveIgnoredPrefName, 0);
+  RecordContentSuggestionsNotificationAction(CONTENT_SUGGESTIONS_TAP);
+}
+
+static void NotificationDismissed(JNIEnv* env, const JavaParamRef<jclass>&) {
+  Profile* profile = ProfileManager::GetLastUsedProfile()->GetOriginalProfile();
+  auto prefs = profile->GetPrefs();
+  const bool was_disabled = IsDisabledForProfile(profile);
+  int ignored =
+      prefs->GetInteger(kContentSuggestionsConsecutiveIgnoredPrefName);
+  prefs->SetInteger(kContentSuggestionsConsecutiveIgnoredPrefName, ignored + 1);
+  RecordContentSuggestionsNotificationAction(
+      CONTENT_SUGGESTIONS_DISMISSAL);
+  const bool is_disabled = IsDisabledForProfile(profile);
+  if (!was_disabled && is_disabled) {
+    RecordContentSuggestionsNotificationOptOut(CONTENT_SUGGESTIONS_IMPLICIT);
+  }
+}
+
+static void NotificationExpired(JNIEnv* env, const JavaParamRef<jclass>&) {

[Alexei Svitkine (slow), 2017/01/11 16:45:59]

You can use @JNINamespace("ntp_snippets") on the Java class and then put these
in the same namespace as the rest of the file.

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

That's great, thanks.

+  Profile* profile = ProfileManager::GetLastUsedProfile()->GetOriginalProfile();
+  auto prefs = profile->GetPrefs();
+  const bool was_disabled = IsDisabledForProfile(profile);
+  int ignored =
+      prefs->GetInteger(kContentSuggestionsConsecutiveIgnoredPrefName);
+  prefs->SetInteger(kContentSuggestionsConsecutiveIgnoredPrefName, ignored + 1);
+  RecordContentSuggestionsNotificationAction(
+      CONTENT_SUGGESTIONS_HIDE_DEADLINE);
+  const bool is_disabled = IsDisabledForProfile(profile);
+  if (!was_disabled && is_disabled) {
+    RecordContentSuggestionsNotificationOptOut(CONTENT_SUGGESTIONS_IMPLICIT);
+  }
+}