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

Move existing @IntDef elements inside existing ParallelRequestStatus

@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

Change-Id: I2d2f80b5d0b84b1315016903b74f06d10ab6d4e8
Reviewed-on: https://chromium-review.googlesource.com/1146189Reviewed-by: default avatarEgor Pasko (futex_wait(&amp;secret))) <pasko@chromium.org>
Commit-Queue: Marcin Wiącek <marcin@mwiacek.com>
Cr-Commit-Position: refs/heads/master@{#577628}
parent 92d2c5ba
...@@ -75,6 +75,8 @@ import org.chromium.content_public.common.Referrer; ...@@ -75,6 +75,8 @@ import org.chromium.content_public.common.Referrer;
import java.io.BufferedReader; import java.io.BufferedReader;
import java.io.FileReader; import java.io.FileReader;
import java.io.IOException; import java.io.IOException;
import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.Arrays; import java.util.Arrays;
import java.util.HashSet; import java.util.HashSet;
...@@ -147,27 +149,31 @@ public class CustomTabsConnection { ...@@ -147,27 +149,31 @@ public class CustomTabsConnection {
static final String PARALLEL_REQUEST_URL_KEY = static final String PARALLEL_REQUEST_URL_KEY =
"android.support.customtabs.PARALLEL_REQUEST_URL"; "android.support.customtabs.PARALLEL_REQUEST_URL";
@IntDef({PARALLEL_REQUEST_NO_REQUEST, PARALLEL_REQUEST_SUCCESS, @IntDef({ParallelRequestStatus.NO_REQUEST, ParallelRequestStatus.SUCCESS,
PARALLEL_REQUEST_FAILURE_NOT_INITIALIZED, PARALLEL_REQUEST_FAILURE_NOT_AUTHORIZED, ParallelRequestStatus.FAILURE_NOT_INITIALIZED,
PARALLEL_REQUEST_FAILURE_INVALID_URL, PARALLEL_REQUEST_FAILURE_INVALID_REFERRER, ParallelRequestStatus.FAILURE_NOT_AUTHORIZED, ParallelRequestStatus.FAILURE_INVALID_URL,
PARALLEL_REQUEST_FAILURE_INVALID_REFERRER_FOR_SESSION}) ParallelRequestStatus.FAILURE_INVALID_REFERRER,
@interface ParallelRequestStatus {} ParallelRequestStatus.FAILURE_INVALID_REFERRER_FOR_SESSION})
@Retention(RetentionPolicy.SOURCE)
@VisibleForTesting @interface ParallelRequestStatus {
static final int PARALLEL_REQUEST_NO_REQUEST = 0; // Values should start from 0 and can't have gaps (they're used for indexing
@VisibleForTesting // PARALLEL_REQUEST_MESSAGES).
static final int PARALLEL_REQUEST_SUCCESS = 1; @VisibleForTesting
@VisibleForTesting int NO_REQUEST = 0;
static final int PARALLEL_REQUEST_FAILURE_NOT_INITIALIZED = 2; @VisibleForTesting
@VisibleForTesting int SUCCESS = 1;
static final int PARALLEL_REQUEST_FAILURE_NOT_AUTHORIZED = 3; @VisibleForTesting
@VisibleForTesting int FAILURE_NOT_INITIALIZED = 2;
static final int PARALLEL_REQUEST_FAILURE_INVALID_URL = 4; @VisibleForTesting
@VisibleForTesting int FAILURE_NOT_AUTHORIZED = 3;
static final int PARALLEL_REQUEST_FAILURE_INVALID_REFERRER = 5; @VisibleForTesting
@VisibleForTesting int FAILURE_INVALID_URL = 4;
static final int PARALLEL_REQUEST_FAILURE_INVALID_REFERRER_FOR_SESSION = 6; @VisibleForTesting
private static final int PARALLEL_REQUEST_STATUS_MAX = 7; int FAILURE_INVALID_REFERRER = 5;
@VisibleForTesting
int FAILURE_INVALID_REFERRER_FOR_SESSION = 6;
int NUM_ENTRIES = 7;
}
private static final String[] PARALLEL_REQUEST_MESSAGES = {"No request", "Success", private static final String[] PARALLEL_REQUEST_MESSAGES = {"No request", "Success",
"Chrome not initialized", "Not authorized", "Invalid URL", "Invalid referrer", "Chrome not initialized", "Not authorized", "Invalid URL", "Invalid referrer",
...@@ -175,7 +181,7 @@ public class CustomTabsConnection { ...@@ -175,7 +181,7 @@ public class CustomTabsConnection {
private static final EnumeratedHistogramSample sParallelRequestStatusOnStart = private static final EnumeratedHistogramSample sParallelRequestStatusOnStart =
new EnumeratedHistogramSample( new EnumeratedHistogramSample(
"CustomTabs.ParallelRequestStatusOnStart", PARALLEL_REQUEST_STATUS_MAX); "CustomTabs.ParallelRequestStatusOnStart", ParallelRequestStatus.NUM_ENTRIES);
private static final CustomTabsConnection sInstance = private static final CustomTabsConnection sInstance =
AppHooks.get().createCustomTabsConnection(); AppHooks.get().createCustomTabsConnection();
...@@ -883,25 +889,26 @@ public class CustomTabsConnection { ...@@ -883,25 +889,26 @@ public class CustomTabsConnection {
private int maybeStartParallelRequest(CustomTabsSessionToken session, Intent intent) { private int maybeStartParallelRequest(CustomTabsSessionToken session, Intent intent) {
ThreadUtils.assertOnUiThread(); ThreadUtils.assertOnUiThread();
if (!intent.hasExtra(PARALLEL_REQUEST_URL_KEY)) return PARALLEL_REQUEST_NO_REQUEST; if (!intent.hasExtra(PARALLEL_REQUEST_URL_KEY)) return ParallelRequestStatus.NO_REQUEST;
if (!ChromeBrowserInitializer.getInstance(ContextUtils.getApplicationContext()) if (!ChromeBrowserInitializer.getInstance(ContextUtils.getApplicationContext())
.hasNativeInitializationCompleted()) { .hasNativeInitializationCompleted()) {
return PARALLEL_REQUEST_FAILURE_NOT_INITIALIZED; return ParallelRequestStatus.FAILURE_NOT_INITIALIZED;
} }
if (!mClientManager.getAllowParallelRequestForSession(session)) { if (!mClientManager.getAllowParallelRequestForSession(session)) {
return PARALLEL_REQUEST_FAILURE_NOT_AUTHORIZED; return ParallelRequestStatus.FAILURE_NOT_AUTHORIZED;
} }
Uri referrer = intent.getParcelableExtra(PARALLEL_REQUEST_REFERRER_KEY); Uri referrer = intent.getParcelableExtra(PARALLEL_REQUEST_REFERRER_KEY);
Uri url = intent.getParcelableExtra(PARALLEL_REQUEST_URL_KEY); Uri url = intent.getParcelableExtra(PARALLEL_REQUEST_URL_KEY);
int policy = int policy =
intent.getIntExtra(PARALLEL_REQUEST_REFERRER_POLICY_KEY, WebReferrerPolicy.DEFAULT); intent.getIntExtra(PARALLEL_REQUEST_REFERRER_POLICY_KEY, WebReferrerPolicy.DEFAULT);
if (url == null) return PARALLEL_REQUEST_FAILURE_INVALID_URL; if (url == null) return ParallelRequestStatus.FAILURE_INVALID_URL;
if (referrer == null) return PARALLEL_REQUEST_FAILURE_INVALID_REFERRER; if (referrer == null) return ParallelRequestStatus.FAILURE_INVALID_REFERRER;
if (policy < 0 || policy > WebReferrerPolicy.LAST) policy = WebReferrerPolicy.DEFAULT; if (policy < 0 || policy > WebReferrerPolicy.LAST) policy = WebReferrerPolicy.DEFAULT;
if (url.toString().equals("") || !isValid(url)) return PARALLEL_REQUEST_FAILURE_INVALID_URL; if (url.toString().equals("") || !isValid(url))
return ParallelRequestStatus.FAILURE_INVALID_URL;
if (!canDoParallelRequest(session, referrer)) { if (!canDoParallelRequest(session, referrer)) {
return PARALLEL_REQUEST_FAILURE_INVALID_REFERRER_FOR_SESSION; return ParallelRequestStatus.FAILURE_INVALID_REFERRER_FOR_SESSION;
} }
String urlString = url.toString(); String urlString = url.toString();
...@@ -911,7 +918,7 @@ public class CustomTabsConnection { ...@@ -911,7 +918,7 @@ public class CustomTabsConnection {
if (mLogRequests) { if (mLogRequests) {
Log.w(TAG, "startParallelRequest(%s, %s, %d)", urlString, referrerString, policy); Log.w(TAG, "startParallelRequest(%s, %s, %d)", urlString, referrerString, policy);
} }
return PARALLEL_REQUEST_SUCCESS; return ParallelRequestStatus.SUCCESS;
} }
/** @return Whether {@code session} can create a parallel request for a given /** @return Whether {@code session} can create a parallel request for a given
......
...@@ -99,13 +99,13 @@ public class DetachedResourceRequestTest { ...@@ -99,13 +99,13 @@ public class DetachedResourceRequestTest {
CustomTabsSessionToken session = prepareSession(); CustomTabsSessionToken session = prepareSession();
ThreadUtils.runOnUiThreadBlocking(() -> { ThreadUtils.runOnUiThreadBlocking(() -> {
int expected = CustomTabsConnection.PARALLEL_REQUEST_NO_REQUEST; int expected = CustomTabsConnection.ParallelRequestStatus.NO_REQUEST;
HistogramDelta histogram = HistogramDelta histogram =
new HistogramDelta("CustomTabs.ParallelRequestStatusOnStart", expected); new HistogramDelta("CustomTabs.ParallelRequestStatusOnStart", expected);
Assert.assertEquals(expected, mConnection.handleParallelRequest(session, new Intent())); Assert.assertEquals(expected, mConnection.handleParallelRequest(session, new Intent()));
Assert.assertEquals(1, histogram.getDelta()); Assert.assertEquals(1, histogram.getDelta());
expected = CustomTabsConnection.PARALLEL_REQUEST_FAILURE_INVALID_URL; expected = CustomTabsConnection.ParallelRequestStatus.FAILURE_INVALID_URL;
histogram = new HistogramDelta("CustomTabs.ParallelRequestStatusOnStart", expected); histogram = new HistogramDelta("CustomTabs.ParallelRequestStatusOnStart", expected);
Intent intent = Intent intent =
prepareIntent(Uri.parse("android-app://this.is.an.android.app"), ORIGIN); prepareIntent(Uri.parse("android-app://this.is.an.android.app"), ORIGIN);
...@@ -113,20 +113,21 @@ public class DetachedResourceRequestTest { ...@@ -113,20 +113,21 @@ public class DetachedResourceRequestTest {
mConnection.handleParallelRequest(session, intent)); mConnection.handleParallelRequest(session, intent));
Assert.assertEquals(1, histogram.getDelta()); Assert.assertEquals(1, histogram.getDelta());
expected = CustomTabsConnection.PARALLEL_REQUEST_FAILURE_INVALID_URL; expected = CustomTabsConnection.ParallelRequestStatus.FAILURE_INVALID_URL;
histogram = new HistogramDelta("CustomTabs.ParallelRequestStatusOnStart", expected); histogram = new HistogramDelta("CustomTabs.ParallelRequestStatusOnStart", expected);
intent = prepareIntent(Uri.parse(""), ORIGIN); intent = prepareIntent(Uri.parse(""), ORIGIN);
Assert.assertEquals("Should not allow an empty URL", expected, Assert.assertEquals("Should not allow an empty URL", expected,
mConnection.handleParallelRequest(session, intent)); mConnection.handleParallelRequest(session, intent));
Assert.assertEquals(1, histogram.getDelta()); Assert.assertEquals(1, histogram.getDelta());
expected = CustomTabsConnection.PARALLEL_REQUEST_FAILURE_INVALID_REFERRER_FOR_SESSION; expected =
CustomTabsConnection.ParallelRequestStatus.FAILURE_INVALID_REFERRER_FOR_SESSION;
histogram = new HistogramDelta("CustomTabs.ParallelRequestStatusOnStart", expected); histogram = new HistogramDelta("CustomTabs.ParallelRequestStatusOnStart", expected);
intent = prepareIntent(Uri.parse("HTTPS://foo.bar"), Uri.parse("wrong://origin")); intent = prepareIntent(Uri.parse("HTTPS://foo.bar"), Uri.parse("wrong://origin"));
Assert.assertEquals("Should not allow an arbitrary origin", expected, Assert.assertEquals("Should not allow an arbitrary origin", expected,
mConnection.handleParallelRequest(session, intent)); mConnection.handleParallelRequest(session, intent));
expected = CustomTabsConnection.PARALLEL_REQUEST_SUCCESS; expected = CustomTabsConnection.ParallelRequestStatus.SUCCESS;
histogram = new HistogramDelta("CustomTabs.ParallelRequestStatusOnStart", expected); histogram = new HistogramDelta("CustomTabs.ParallelRequestStatusOnStart", expected);
intent = prepareIntent(Uri.parse("HTTPS://foo.bar"), ORIGIN); intent = prepareIntent(Uri.parse("HTTPS://foo.bar"), ORIGIN);
Assert.assertEquals(expected, mConnection.handleParallelRequest(session, intent)); Assert.assertEquals(expected, mConnection.handleParallelRequest(session, intent));
...@@ -149,7 +150,7 @@ public class DetachedResourceRequestTest { ...@@ -149,7 +150,7 @@ public class DetachedResourceRequestTest {
Uri url = Uri.parse(mServer.getURL("/echotitle")); Uri url = Uri.parse(mServer.getURL("/echotitle"));
ThreadUtils.runOnUiThread(() -> { ThreadUtils.runOnUiThread(() -> {
Assert.assertEquals(CustomTabsConnection.PARALLEL_REQUEST_SUCCESS, Assert.assertEquals(CustomTabsConnection.ParallelRequestStatus.SUCCESS,
mConnection.handleParallelRequest(session, prepareIntent(url, ORIGIN))); mConnection.handleParallelRequest(session, prepareIntent(url, ORIGIN)));
}); });
cb.waitForCallback(0, 1); cb.waitForCallback(0, 1);
...@@ -163,7 +164,7 @@ public class DetachedResourceRequestTest { ...@@ -163,7 +164,7 @@ public class DetachedResourceRequestTest {
mServer = EmbeddedTestServer.createAndStartServer(mContext); mServer = EmbeddedTestServer.createAndStartServer(mContext);
final Uri url = Uri.parse(mServer.getURL("/set-cookie?acookie")); final Uri url = Uri.parse(mServer.getURL("/set-cookie?acookie"));
ThreadUtils.runOnUiThreadBlocking(() -> { ThreadUtils.runOnUiThreadBlocking(() -> {
Assert.assertEquals(CustomTabsConnection.PARALLEL_REQUEST_SUCCESS, Assert.assertEquals(CustomTabsConnection.ParallelRequestStatus.SUCCESS,
mConnection.handleParallelRequest(session, prepareIntent(url, ORIGIN))); mConnection.handleParallelRequest(session, prepareIntent(url, ORIGIN)));
}); });
...@@ -256,7 +257,7 @@ public class DetachedResourceRequestTest { ...@@ -256,7 +257,7 @@ public class DetachedResourceRequestTest {
}); });
final Uri url = Uri.parse(mServer.getURL("/set-cookie?acookie")); final Uri url = Uri.parse(mServer.getURL("/set-cookie?acookie"));
ThreadUtils.runOnUiThreadBlocking(() -> { ThreadUtils.runOnUiThreadBlocking(() -> {
Assert.assertEquals(CustomTabsConnection.PARALLEL_REQUEST_SUCCESS, Assert.assertEquals(CustomTabsConnection.ParallelRequestStatus.SUCCESS,
mConnection.handleParallelRequest(session, prepareIntent(url, ORIGIN))); mConnection.handleParallelRequest(session, prepareIntent(url, ORIGIN)));
}); });
...@@ -286,7 +287,7 @@ public class DetachedResourceRequestTest { ...@@ -286,7 +287,7 @@ public class DetachedResourceRequestTest {
CustomTabsSessionToken session = prepareSession(url); CustomTabsSessionToken session = prepareSession(url);
ThreadUtils.runOnUiThreadBlocking(() -> { ThreadUtils.runOnUiThreadBlocking(() -> {
Assert.assertEquals(CustomTabsConnection.PARALLEL_REQUEST_SUCCESS, Assert.assertEquals(CustomTabsConnection.ParallelRequestStatus.SUCCESS,
mConnection.handleParallelRequest(session, prepareIntent(url, origin))); mConnection.handleParallelRequest(session, prepareIntent(url, origin)));
}); });
...@@ -343,7 +344,7 @@ public class DetachedResourceRequestTest { ...@@ -343,7 +344,7 @@ public class DetachedResourceRequestTest {
Uri url = Uri.parse(mServer.getURL(relativeUrl)); Uri url = Uri.parse(mServer.getURL(relativeUrl));
ThreadUtils.runOnUiThreadBlocking(() -> { ThreadUtils.runOnUiThreadBlocking(() -> {
Assert.assertEquals(CustomTabsConnection.PARALLEL_REQUEST_SUCCESS, Assert.assertEquals(CustomTabsConnection.ParallelRequestStatus.SUCCESS,
mConnection.handleParallelRequest(session, prepareIntent(url, ORIGIN))); mConnection.handleParallelRequest(session, prepareIntent(url, ORIGIN)));
}); });
readFromSocketCallback.waitForCallback(0); readFromSocketCallback.waitForCallback(0);
......
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