Commit eaa83600 authored by johnme's avatar johnme Committed by Commit bot

Push API: Remove deprecated lastAppId storage.

This was just a hacky workaround whilst GCM didn't send us subtypes for
incoming messages. Now that it does, we don't need it any more (and it
might cause false positives during testing).

BUG=350383

Review URL: https://codereview.chromium.org/907003003

Cr-Commit-Position: refs/heads/master@{#315358}
parent 2d3f4b2a
...@@ -5,10 +5,8 @@ ...@@ -5,10 +5,8 @@
package org.chromium.components.gcm_driver; package org.chromium.components.gcm_driver;
import android.content.Context; import android.content.Context;
import android.content.SharedPreferences;
import android.os.AsyncTask; import android.os.AsyncTask;
import android.os.Bundle; import android.os.Bundle;
import android.preference.PreferenceManager;
import android.util.Log; import android.util.Log;
import org.chromium.base.CalledByNative; import org.chromium.base.CalledByNative;
...@@ -24,7 +22,7 @@ import java.util.List; ...@@ -24,7 +22,7 @@ import java.util.List;
/** /**
* This class is the Java counterpart to the C++ GCMDriverAndroid class. * This class is the Java counterpart to the C++ GCMDriverAndroid class.
* It uses Android's Java GCM APIs to implements GCM registration etc, and * It uses Android's Java GCM APIs to implement GCM registration etc, and
* sends back GCM messages over JNI. * sends back GCM messages over JNI.
* *
* Threading model: all calls to/from C++ happen on the UI thread. * Threading model: all calls to/from C++ happen on the UI thread.
...@@ -33,8 +31,6 @@ import java.util.List; ...@@ -33,8 +31,6 @@ import java.util.List;
public class GCMDriver { public class GCMDriver {
private static final String TAG = "GCMDriver"; private static final String TAG = "GCMDriver";
private static final String LAST_GCM_APP_ID_KEY = "last_gcm_app_id";
// The instance of GCMDriver currently owned by a C++ GCMDriverAndroid, if any. // The instance of GCMDriver currently owned by a C++ GCMDriverAndroid, if any.
private static GCMDriver sInstance = null; private static GCMDriver sInstance = null;
...@@ -76,7 +72,6 @@ public class GCMDriver { ...@@ -76,7 +72,6 @@ public class GCMDriver {
@CalledByNative @CalledByNative
private void register(final String appId, final String[] senderIds) { private void register(final String appId, final String[] senderIds) {
setLastAppId(appId);
new AsyncTask<Void, Void, String>() { new AsyncTask<Void, Void, String>() {
@Override @Override
protected String doInBackground(Void... voids) { protected String doInBackground(Void... voids) {
...@@ -138,17 +133,15 @@ public class GCMDriver { ...@@ -138,17 +133,15 @@ public class GCMDriver {
List<String> dataKeysAndValues = new ArrayList<String>(); List<String> dataKeysAndValues = new ArrayList<String>();
for (String key : extras.keySet()) { for (String key : extras.keySet()) {
// TODO(johnme): Check there aren't other keys that we need to exclude. // TODO(johnme): Check there aren't other keys that we need to exclude.
if (key.equals(bundleSubtype) || key.equals(bundleSenderId) || if (key.equals(bundleSubtype) || key.equals(bundleSenderId)
key.equals(bundleCollapseKey) || key.startsWith(bundleGcmplex)) || key.equals(bundleCollapseKey) || key.startsWith(bundleGcmplex))
continue; continue;
dataKeysAndValues.add(key); dataKeysAndValues.add(key);
dataKeysAndValues.add(extras.getString(key)); dataKeysAndValues.add(extras.getString(key));
} }
String guessedAppId = GCMListener.UNKNOWN_APP_ID.equals(appId) ? getLastAppId()
: appId;
sInstance.nativeOnMessageReceived(sInstance.mNativeGCMDriverAndroid, sInstance.nativeOnMessageReceived(sInstance.mNativeGCMDriverAndroid,
guessedAppId, senderId, collapseKey, appId, senderId, collapseKey,
dataKeysAndValues.toArray(new String[dataKeysAndValues.size()])); dataKeysAndValues.toArray(new String[dataKeysAndValues.size()]));
} }
}); });
...@@ -159,9 +152,7 @@ public class GCMDriver { ...@@ -159,9 +152,7 @@ public class GCMDriver {
ThreadUtils.assertOnUiThread(); ThreadUtils.assertOnUiThread();
launchNativeThen(context, new Runnable() { launchNativeThen(context, new Runnable() {
@Override public void run() { @Override public void run() {
String guessedAppId = GCMListener.UNKNOWN_APP_ID.equals(appId) ? getLastAppId() sInstance.nativeOnMessagesDeleted(sInstance.mNativeGCMDriverAndroid, appId);
: appId;
sInstance.nativeOnMessagesDeleted(sInstance.mNativeGCMDriverAndroid, guessedAppId);
} }
}); });
} }
...@@ -174,21 +165,6 @@ public class GCMDriver { ...@@ -174,21 +165,6 @@ public class GCMDriver {
String senderId, String collapseKey, String[] dataKeysAndValues); String senderId, String collapseKey, String[] dataKeysAndValues);
private native void nativeOnMessagesDeleted(long nativeGCMDriverAndroid, String appId); private native void nativeOnMessagesDeleted(long nativeGCMDriverAndroid, String appId);
// TODO(johnme): This and setLastAppId are just temporary (crbug.com/350383).
private static String getLastAppId() {
SharedPreferences settings = PreferenceManager.getDefaultSharedPreferences(
sInstance.mContext);
return settings.getString(LAST_GCM_APP_ID_KEY, "push#unknown_app_id#0");
}
private static void setLastAppId(String appId) {
SharedPreferences settings = PreferenceManager.getDefaultSharedPreferences(
sInstance.mContext);
SharedPreferences.Editor editor = settings.edit();
editor.putString(LAST_GCM_APP_ID_KEY, appId);
editor.commit();
}
private static void launchNativeThen(Context context, Runnable task) { private static void launchNativeThen(Context context, Runnable task) {
if (sInstance != null) { if (sInstance != null) {
task.run(); task.run();
......
...@@ -6,6 +6,7 @@ package org.chromium.components.gcm_driver; ...@@ -6,6 +6,7 @@ package org.chromium.components.gcm_driver;
import android.content.Intent; import android.content.Intent;
import android.os.Bundle; import android.os.Bundle;
import android.util.Log;
import com.google.ipc.invalidation.external.client.contrib.MultiplexingGcmListener; import com.google.ipc.invalidation.external.client.contrib.MultiplexingGcmListener;
...@@ -30,10 +31,6 @@ public class GCMListener extends MultiplexingGcmListener.AbstractListener { ...@@ -30,10 +31,6 @@ public class GCMListener extends MultiplexingGcmListener.AbstractListener {
private static final String TAG = "GCMListener"; private static final String TAG = "GCMListener";
// Used as a fallback until GCM always gives us the subtype.
// TODO(johnme): Remove this once it does.
static final String UNKNOWN_APP_ID = "push#https://www.gcmlistenerfake.com#0";
public GCMListener() { public GCMListener() {
super(TAG); super(TAG);
} }
...@@ -53,15 +50,11 @@ public class GCMListener extends MultiplexingGcmListener.AbstractListener { ...@@ -53,15 +50,11 @@ public class GCMListener extends MultiplexingGcmListener.AbstractListener {
final String bundleSubtype = "subtype"; final String bundleSubtype = "subtype";
final String bundleDataForPushApi = "data"; final String bundleDataForPushApi = "data";
Bundle extras = intent.getExtras(); Bundle extras = intent.getExtras();
if (!extras.containsKey(bundleSubtype) && !extras.containsKey(bundleDataForPushApi)) { if (!extras.containsKey(bundleSubtype)) {
// TODO(johnme): Once GCM always gives us the subtype, we'll be able to early-out if Log.w(TAG, "Received push message with no subtype");
// there is no subtype extra. For now we have to also allow messages without subtype
// if they have the data key which suggests they are for the Push API, but this is
// technically a layering violation as this code is for other consumers of GCM too.
return; return;
} }
final String appId = extras.containsKey(bundleSubtype) ? extras.getString(bundleSubtype) final String appId = extras.getString(bundleSubtype);
: UNKNOWN_APP_ID;
ThreadUtils.runOnUiThread(new Runnable() { ThreadUtils.runOnUiThread(new Runnable() {
@Override public void run() { @Override public void run() {
GCMDriver.onMessageReceived(getApplicationContext(), appId, GCMDriver.onMessageReceived(getApplicationContext(), appId,
...@@ -72,10 +65,15 @@ public class GCMListener extends MultiplexingGcmListener.AbstractListener { ...@@ -72,10 +65,15 @@ public class GCMListener extends MultiplexingGcmListener.AbstractListener {
@Override @Override
protected void onDeletedMessages(int total) { protected void onDeletedMessages(int total) {
ThreadUtils.runOnUiThread(new Runnable() { // TODO(johnme): Refactor/replace MultiplexingGcmListener so it passes us the extras and
@Override public void run() { // hence the subtype (aka appId).
GCMDriver.onMessagesDeleted(getApplicationContext(), UNKNOWN_APP_ID); Log.w(TAG, "Push messages were deleted, but we can't tell the Service Worker, as we"
} + " don't have access to the intent extras so we can't get the appId");
}); return;
//ThreadUtils.runOnUiThread(new Runnable() {
// @Override public void run() {
// GCMDriver.onMessagesDeleted(getApplicationContext(), appId);
// }
//});
} }
} }
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