Commit d82ee001 authored by Marcin Wiacek's avatar Marcin Wiacek Committed by Commit Bot

@IntDef cleanup in chrome/java/externalauth

@IntDef/@StringDef annotation are preferred way for declaring
set of String/int values:

1. they need less space in APK than enum, see
https://developer.android.com/topic/performance/reduce-apk-size#remove-enums
2. they give more control over allowed values than "static final" values

Main goal of patch is writing "static final" values, enum
and some classes in one common @IntDef/@StringDef form:

1. with @IntDef/@StringDef first, @Retention second
   and related @interface third
2. with values inside @interface
3. with NUM_ENTRIES declaring number of entries if necessary
4. with comment about numbering from 0 without gaps when necessary
5. with @Retention(RetentionPolicy.SOURCE)
6. without "static final" in the @interface

Additionally there are done some other trivial cleanups.

Change-Id: I8285733bbecf695b133a3865310f9a3cf0c515d6
Reviewed-on: https://chromium-review.googlesource.com/1133983
Commit-Queue: Marcin Wiącek <marcin@mwiacek.com>
Commit-Queue: Theresa <twellington@chromium.org>
Reviewed-by: default avatarTheresa <twellington@chromium.org>
Cr-Commit-Position: refs/heads/master@{#574399}
parent bed9bc8a
......@@ -165,11 +165,8 @@ public class ExternalAuthUtils {
*/
public boolean isGooglePlayServicesMissing(final Context context) {
final int resultCode = checkGooglePlayServicesAvailable(context);
if (resultCode == ConnectionResult.SERVICE_MISSING
|| resultCode == ConnectionResult.SERVICE_INVALID) {
return true;
}
return false;
return (resultCode == ConnectionResult.SERVICE_MISSING
|| resultCode == ConnectionResult.SERVICE_INVALID);
}
/**
......@@ -187,9 +184,7 @@ public class ExternalAuthUtils {
Context context = ContextUtils.getApplicationContext();
final int resultCode = checkGooglePlayServicesAvailable(context);
recordConnectionResult(resultCode);
if (resultCode == ConnectionResult.SUCCESS) {
return true;
}
if (resultCode == ConnectionResult.SUCCESS) return true;
// resultCode is some kind of error.
Log.v(TAG, "Unable to use Google Play Services: %s", describeError(resultCode));
if (isUserRecoverableError(resultCode)) {
......
......@@ -8,6 +8,7 @@ import android.app.Activity;
import android.app.Dialog;
import android.content.Context;
import android.content.DialogInterface;
import android.support.annotation.IntDef;
import com.google.android.gms.common.GoogleApiAvailability;
......@@ -15,6 +16,8 @@ import org.chromium.base.ThreadUtils;
import org.chromium.base.metrics.CachedMetrics.ActionEvent;
import org.chromium.base.metrics.CachedMetrics.EnumeratedHistogramSample;
import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
import java.util.concurrent.atomic.AtomicBoolean;
/**
......@@ -44,15 +47,20 @@ public abstract class UserRecoverableErrorHandler {
private static final String ERROR_HANDLER_ACTION_HISTOGRAM_NAME =
"GooglePlayServices.ErrorHandlerAction";
// Never remove or reorder histogram values. It is safe to append new values to the end.
private static final int ERROR_HANDLER_ACTION_SILENT = 0;
private static final int ERROR_HANDLER_ACTION_SYSTEM_NOTIFICATION = 1;
private static final int ERROR_HANDLER_ACTION_MODAL_DIALOG = 2;
private static final int ERROR_HANDLER_ACTION_IGNORED_AS_REDUNDANT = 3;
private static final int ERROR_HANDLER_ACTION_HISTOGRAM_BOUNDARY = 4;
@IntDef({ErrorHandlerAction.SILENT, ErrorHandlerAction.SYSTEM_NOTIFICATION,
ErrorHandlerAction.MODAL_DIALOG, ErrorHandlerAction.IGNORED_AS_REDUNDANT})
@Retention(RetentionPolicy.SOURCE)
private @interface ErrorHandlerAction {
int SILENT = 0;
int SYSTEM_NOTIFICATION = 1;
int MODAL_DIALOG = 2;
int IGNORED_AS_REDUNDANT = 3;
int NUM_ENTRIES = 4;
}
private static final EnumeratedHistogramSample sErrorHandlerActionHistogramSample =
new EnumeratedHistogramSample(ERROR_HANDLER_ACTION_HISTOGRAM_NAME,
ERROR_HANDLER_ACTION_HISTOGRAM_BOUNDARY);
new EnumeratedHistogramSample(
ERROR_HANDLER_ACTION_HISTOGRAM_NAME, ErrorHandlerAction.NUM_ENTRIES);
private static final ActionEvent sModalDialogShownActionEvent =
new ActionEvent("Signin_Android_GmsUserRecoverableDialogShown");
......@@ -87,7 +95,7 @@ public abstract class UserRecoverableErrorHandler {
public static final class Silent extends UserRecoverableErrorHandler {
@Override
protected final void handle(final Context context, final int errorCode) {
sErrorHandlerActionHistogramSample.record(ERROR_HANDLER_ACTION_SILENT);
sErrorHandlerActionHistogramSample.record(ErrorHandlerAction.SILENT);
}
}
......@@ -109,12 +117,11 @@ public abstract class UserRecoverableErrorHandler {
@Override
protected void handle(final Context context, final int errorCode) {
if (!sNotificationShown.getAndSet(true)) {
sErrorHandlerActionHistogramSample
.record(ERROR_HANDLER_ACTION_IGNORED_AS_REDUNDANT);
sErrorHandlerActionHistogramSample.record(ErrorHandlerAction.IGNORED_AS_REDUNDANT);
return;
}
GoogleApiAvailability.getInstance().showErrorNotification(context, errorCode);
sErrorHandlerActionHistogramSample.record(ERROR_HANDLER_ACTION_SYSTEM_NOTIFICATION);
sErrorHandlerActionHistogramSample.record(ErrorHandlerAction.SYSTEM_NOTIFICATION);
}
}
......@@ -219,7 +226,7 @@ public abstract class UserRecoverableErrorHandler {
mDialog.show();
sModalDialogShownActionEvent.record();
}
sErrorHandlerActionHistogramSample.record(ERROR_HANDLER_ACTION_MODAL_DIALOG);
sErrorHandlerActionHistogramSample.record(ErrorHandlerAction.MODAL_DIALOG);
}
/**
......
......@@ -49,9 +49,7 @@ public class VerifiedHandler extends Handler {
public boolean sendMessageAtTime(Message msg, long uptimeMillis) {
Messenger client = msg.replyTo;
if (!mClientTrustMap.containsKey(client)) mClientTrustMap.put(client, checkCallerIsValid());
if (!mClientTrustMap.get(client)) return false;
return super.sendMessageAtTime(msg, uptimeMillis);
return (!mClientTrustMap.get(client)) ? false : super.sendMessageAtTime(msg, uptimeMillis);
}
/**
......@@ -59,11 +57,9 @@ public class VerifiedHandler extends Handler {
* set during construction.
*/
public boolean checkCallerIsValid() {
if (TextUtils.isEmpty(mCallerPackageToMatch)) {
return ExternalAuthUtils.getInstance().isCallerValid(mContext, mAuthRequirements);
} else {
return ExternalAuthUtils.getInstance().isCallerValidForPackage(
mContext, mAuthRequirements, mCallerPackageToMatch);
}
return TextUtils.isEmpty(mCallerPackageToMatch)
? ExternalAuthUtils.getInstance().isCallerValid(mContext, mAuthRequirements)
: ExternalAuthUtils.getInstance().isCallerValidForPackage(
mContext, mAuthRequirements, mCallerPackageToMatch);
}
}
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