Commit 2530db14 authored by dfalcantara's avatar dfalcantara Committed by Commit bot

Revert of Revert "Revert of Implement GetDisplayed on android M+ (patchset #3...

Revert of Revert "Revert of Implement GetDisplayed on android M+ (patchset #3 id:60001 of https://codereview.… (patchset #1 id:1 of https://codereview.chromium.org/2585823002/ )

Reason for revert:
org.chromium.chrome.browser.push_messaging.PushMessagingTest#testDefaultNotification
is still failing consistently on the Marshmallow bots:

https://test-results.appspot.com/dashboards/flakiness_dashboard.html#testType=chrome_public_test_apk&tests=org.chromium.chrome.browser.push_messaging.PushMessagingTest

Example log:
https://build.chromium.org/p/chromium.android/builders/Marshmallow%2064%20bit%20Tester/builds/7408

org.chromium.chrome.browser.push_messaging.PushMessagingTest#testDefaultNotification (run #1):
junit.framework.AssertionFailedError: expected:<1> but was:<2>
	at org.chromium.chrome.browser.notifications.NotificationTestBase.waitForNotification(NotificationTestBase.java:107)
	at org.chromium.chrome.browser.push_messaging.PushMessagingTest.testDefaultNotification(PushMessagingTest.java:246)
	at android.test.InstrumentationTestCase.runMethod(InstrumentationTestCase.java:214)
	at android.test.InstrumentationTestCase.runTest(InstrumentationTestCase.java:199)
	at android.test.ActivityInstrumentationTestCase2.runTest(ActivityInstrumentationTestCase2.java:192)
	at org.chromium.chrome.test.ChromeActivityTestCaseBase.runTest(ChromeActivityTestCaseBase.java:752)
	at org.chromium.base.test.BaseTestResult.runParameterized(BaseTestResult.java:161)
	at org.chromium.base.test.BaseTestResult.run(BaseTestResult.java:124)
	at android.test.AndroidTestRunner.runTest(AndroidTestRunner.java:191)
	at android.test.AndroidTestRunner.runTest(AndroidTestRunner.java:176)
	at android.test.InstrumentationTestRunner.onStart(InstrumentationTestRunner.java:555)
	at android.app.Instrumentation$InstrumentationThread.run(Instrumentation.java:1879)

Original issue's description:
> Revert "Revert of Implement GetDisplayed on android M+ (patchset #3 id:60001 of https://codereview.chromium.org/2536313003/ )"
>
> This reverts commit 06b647e4.
>
> It also includes an extra null check in case no active notifications are
> returned.
>
> TBR=peter,avi
>
> BUG=
>
> Committed: https://crrev.com/ef0ab6b078544507d8ae76371bc962095858554d
> Cr-Commit-Position: refs/heads/master@{#439278}

TBR=avi@chromium.org,peter@rybin.spb.ru,miguelg@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG=

Review-Url: https://codereview.chromium.org/2589333002
Cr-Commit-Position: refs/heads/master@{#439884}
parent 9d888502
......@@ -166,9 +166,6 @@ public class CustomNotificationBuilder extends NotificationBuilderBase {
// Notification.Builder.setPublicVersion was added in Android L.
builder.setPublicVersion(createPublicNotification(mContext));
}
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.M) {
builder.setExtras(mExtras);
}
Notification notification = builder.build();
notification.bigContentView = bigView;
......
......@@ -18,7 +18,6 @@ import android.graphics.PorterDuff;
import android.graphics.PorterDuffColorFilter;
import android.graphics.drawable.Icon;
import android.os.Build;
import android.os.Bundle;
import org.chromium.base.VisibleForTesting;
import org.chromium.chrome.browser.widget.RoundedIconGenerator;
......@@ -122,7 +121,6 @@ public abstract class NotificationBuilderBase {
protected long[] mVibratePattern;
protected long mTimestamp;
protected boolean mRenotify;
protected Bundle mExtras;
private Bitmap mLargeIcon;
......@@ -310,18 +308,6 @@ public abstract class NotificationBuilderBase {
return this;
}
/**
* Sets the extras bundle on supported platforms.
*/
@TargetApi(Build.VERSION_CODES.M)
public NotificationBuilderBase setExtras(Bundle extras) {
if (Build.VERSION.SDK_INT < Build.VERSION_CODES.M) {
return this;
}
mExtras = extras;
return this;
}
/**
* Gets the large icon for the notification.
*
......
......@@ -4,10 +4,7 @@
package org.chromium.chrome.browser.notifications;
import android.annotation.TargetApi;
import android.app.Notification;
import android.os.Build;
import android.service.notification.StatusBarNotification;
/**
* A proxy for the Android Notification Manager. This allows tests to be written without having to
......@@ -21,7 +18,4 @@ public interface NotificationManagerProxy {
void cancelAll();
void notify(int id, Notification notification);
void notify(String tag, int id, Notification notification);
@TargetApi(Build.VERSION_CODES.M)
StatusBarNotification[] getActiveNotifications();
}
......@@ -6,7 +6,6 @@ package org.chromium.chrome.browser.notifications;
import android.app.Notification;
import android.app.NotificationManager;
import android.service.notification.StatusBarNotification;
/**
* Default implementation of the NotificationManagerProxy, which passes through all calls to the
......@@ -43,9 +42,4 @@ public class NotificationManagerProxyImpl implements NotificationManagerProxy {
public void notify(String tag, int id, Notification notification) {
mNotificationManager.notify(tag, id, notification);
}
@Override
public StatusBarNotification[] getActiveNotifications() {
return mNotificationManager.getActiveNotifications();
}
}
......@@ -15,7 +15,6 @@ import android.graphics.Bitmap;
import android.net.Uri;
import android.os.Build;
import android.os.Bundle;
import android.service.notification.StatusBarNotification;
import android.text.Spannable;
import android.text.SpannableStringBuilder;
import android.text.TextUtils;
......@@ -44,8 +43,6 @@ import org.chromium.webapk.lib.client.WebApkValidator;
import java.net.URI;
import java.net.URISyntaxException;
import java.util.ArrayList;
import java.util.List;
import javax.annotation.Nullable;
......@@ -575,15 +572,6 @@ public class NotificationPlatformBridge {
makeDefaults(vibrationPattern.length, silent, vibrateEnabled));
notificationBuilder.setVibrate(makeVibrationPattern(vibrationPattern));
// The extras bundle is available from API 20 but it's currenlty only used to retrieve
// notifications which is only available from 23 (Marshmallow) onwards.
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.M) {
Bundle extras = new Bundle();
extras.putString(NotificationConstants.EXTRA_NOTIFICATION_ID, notificationId);
extras.putString(NotificationConstants.EXTRA_NOTIFICATION_INFO_PROFILE_ID, profileId);
notificationBuilder.setExtras(extras);
}
String platformTag = makePlatformTag(notificationId, origin, tag);
if (webApkPackage.isEmpty()) {
mNotificationManager.notify(platformTag, PLATFORM_ID, notificationBuilder.build());
......@@ -687,39 +675,6 @@ public class NotificationPlatformBridge {
}
}
/**
* Returns the notification ids on disply for a given |profileId|.
*
* @param profileId of the profile to retrieve notifications for.
*/
@CalledByNative
private String[] getNotificationsForProfile(String profileId) {
if (Build.VERSION.SDK_INT < Build.VERSION_CODES.M) {
return null;
}
if (mNotificationManager.getActiveNotifications() == null) {
return null;
}
StatusBarNotification[] displayedNotifications =
mNotificationManager.getActiveNotifications();
List<String> notifications = new ArrayList<String>();
for (StatusBarNotification notification : displayedNotifications) {
Bundle extras = notification.getNotification().extras;
String notificationId = extras.getString(NotificationConstants.EXTRA_NOTIFICATION_ID);
String notificationProfileId =
extras.getString(NotificationConstants.EXTRA_NOTIFICATION_INFO_PROFILE_ID);
if (notificationId != null && profileId.equals(notificationProfileId)) {
notifications.add(notificationId);
}
}
if (notifications.size() == 0) return null;
String[] result = new String[notifications.size()];
return notifications.toArray(result);
}
/**
* Calls NotificationPlatformBridgeAndroid::OnNotificationClicked in native code to indicate
* that the notification with the given parameters has been clicked on.
......
......@@ -60,9 +60,6 @@ public class StandardNotificationBuilder extends NotificationBuilderBase {
// Notification.Builder.setPublicVersion was added in Android L.
builder.setPublicVersion(createPublicNotification(mContext));
}
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.M) {
builder.setExtras(mExtras);
}
return builder.build();
}
}
......@@ -7,7 +7,6 @@
#include <utility>
#include <vector>
#include "base/android/build_info.h"
#include "base/android/jni_array.h"
#include "base/android/jni_string.h"
#include "base/command_line.h"
......@@ -316,30 +315,8 @@ bool NotificationPlatformBridgeAndroid::GetDisplayed(
const std::string& profile_id,
bool incognito,
std::set<std::string>* notifications) const {
DCHECK(notifications);
JNIEnv* env = AttachCurrentThread();
// Android only supports retrieving existing notifications from M+
if (base::android::BuildInfo::GetInstance()->sdk_int() <
base::android::SDK_VERSION_MARSHMALLOW) {
return false;
}
const ScopedJavaLocalRef<jstring> j_profile_id =
ConvertUTF8ToJavaString(env, profile_id);
ScopedJavaLocalRef<jobjectArray> j_notification_ids =
Java_NotificationPlatformBridge_getNotificationsForProfile(
env, java_object_, j_profile_id);
if (j_notification_ids.obj()) {
std::vector<std::string> notification_ids;
base::android::AppendJavaStringArrayToStringVector(
env, j_notification_ids.obj(), &notification_ids);
for (const auto& id : notification_ids) {
notifications->insert(id);
}
}
return true;
// TODO(miguelg): This can actually be implemented for M+
return false;
}
// static
......
......@@ -5,7 +5,6 @@
package org.chromium.chrome.test.util.browser.notifications;
import android.app.Notification;
import android.service.notification.StatusBarNotification;
import org.chromium.chrome.browser.notifications.NotificationManagerProxy;
......@@ -102,11 +101,6 @@ public class MockNotificationManagerProxy implements NotificationManagerProxy {
mMutationCount++;
}
@Override
public StatusBarNotification[] getActiveNotifications() {
return null;
}
private static String makeKey(int id, @Nullable String tag) {
String key = Integer.toString(id);
if (tag != null) key += KEY_SEPARATOR + tag;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment