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

Migrate WebappScopePolicy to @IntDef

@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: Ibfce629a890785c6b367d38ab45b05ecac893934
Reviewed-on: https://chromium-review.googlesource.com/1183488Reviewed-by: default avatarTed Choc <tedchoc@chromium.org>
Reviewed-by: default avatarMounir Lamouri <mlamouri@chromium.org>
Commit-Queue: Marcin Wiącek <marcin@mwiacek.com>
Cr-Commit-Position: refs/heads/master@{#585549}
parent 9537e32b
......@@ -260,8 +260,8 @@ public class ExternalNavigationDelegateImpl implements ExternalNavigationDelegat
Context context = getAvailableContext();
if (context instanceof WebappActivity) {
WebappActivity webappActivity = (WebappActivity) context;
return webappActivity.scopePolicy().applyPolicyForNavigationToUrl(
webappActivity.getWebappInfo(), url);
return WebappScopePolicy.applyPolicyForNavigationToUrl(
webappActivity.scopePolicy(), webappActivity.getWebappInfo(), url);
}
return WebappScopePolicy.NavigationDirective.NORMAL_BEHAVIOR;
}
......
......@@ -723,7 +723,7 @@ public class WebappActivity extends SingleTabActivity {
updateToolbarCloseButtonVisibility();
if (!scopePolicy().isUrlInScope(mWebappInfo, url)) {
if (!WebappScopePolicy.isUrlInScope(scopePolicy(), mWebappInfo, url)) {
// Briefly show the toolbar for off-scope navigations.
getFullscreenManager()
.getBrowserVisibilityDelegate()
......@@ -785,8 +785,8 @@ public class WebappActivity extends SingleTabActivity {
};
}
public WebappScopePolicy scopePolicy() {
return isVerified() ? WebappScopePolicy.STRICT : WebappScopePolicy.LEGACY;
public @WebappScopePolicy.Type int scopePolicy() {
return isVerified() ? WebappScopePolicy.Type.STRICT : WebappScopePolicy.Type.LEGACY;
}
/**
......@@ -850,8 +850,8 @@ public class WebappActivity extends SingleTabActivity {
final int lastIndex = nc.getLastCommittedEntryIndex();
int index = lastIndex;
while (index > 0
&& !scopePolicy().isUrlInScope(
getWebappInfo(), nc.getEntryAtIndex(index).getUrl())) {
&& !WebappScopePolicy.isUrlInScope(
scopePolicy(), getWebappInfo(), nc.getEntryAtIndex(index).getUrl())) {
index--;
}
......
......@@ -47,8 +47,8 @@ class WebappBrowserControlsDelegate extends TabStateBrowserControlsVisibilityDel
* @param twaVerificationFailed Whether a verification attempt for TWA to client has failed.
* @return Whether the browser controls should be shown for {@code url}.
*/
static boolean shouldShowBrowserControls(WebappScopePolicy scopePolicy, WebappInfo info,
String url, int securityLevel, boolean twaVerificationFailed) {
static boolean shouldShowBrowserControls(@WebappScopePolicy.Type int scopePolicy,
WebappInfo info, String url, int securityLevel, boolean twaVerificationFailed) {
// Do not show browser controls when URL is not ready yet.
if (TextUtils.isEmpty(url)) return false;
......@@ -82,8 +82,8 @@ class WebappBrowserControlsDelegate extends TabStateBrowserControlsVisibilityDel
* {@code url}.
*/
private static boolean shouldShowBrowserControlsForUrl(
WebappScopePolicy scopePolicy, WebappInfo webappInfo, String url) {
return !scopePolicy.isUrlInScope(webappInfo, url);
@WebappScopePolicy.Type int scopePolicy, WebappInfo webappInfo, String url) {
return !WebappScopePolicy.isUrlInScope(scopePolicy, webappInfo, url);
}
private static boolean shouldShowBrowserControlsForSecurityLevel(int securityLevel) {
......
......@@ -13,23 +13,19 @@ import java.lang.annotation.RetentionPolicy;
/**
* Defines which URLs are inside a web app scope as well as what to do when user navigates to them.
*/
public enum WebappScopePolicy {
LEGACY {
@Override
public boolean isUrlInScope(WebappInfo info, String url) {
return UrlUtilities.sameDomainOrHost(info.uri().toString(), url, true);
}
},
STRICT {
@Override
public boolean isUrlInScope(WebappInfo info, String url) {
return UrlUtilities.isUrlWithinScope(url, info.scopeUri().toString());
}
};
public class WebappScopePolicy {
@IntDef({Type.LEGACY, Type.STRICT})
@Retention(RetentionPolicy.SOURCE)
public @interface Type {
// Values should be numerated from 0 and can't have gaps.
int LEGACY = 0;
int STRICT = 1;
int NUM_ENTRIES = 2;
}
@IntDef({NavigationDirective.NORMAL_BEHAVIOR,
NavigationDirective.IGNORE_EXTERNAL_INTENT_REQUESTS})
@Retention(RetentionPolicy.SOURCE)
public @interface NavigationDirective {
// No special handling.
int NORMAL_BEHAVIOR = 0;
......@@ -41,11 +37,22 @@ public enum WebappScopePolicy {
* @return {@code true} if given {@code url} is in scope of a web app as defined by its
* {@code WebappInfo}, {@code false} otherwise.
*/
abstract boolean isUrlInScope(WebappInfo info, String url);
public static boolean isUrlInScope(@Type int type, WebappInfo info, String url) {
switch (type) {
case Type.LEGACY:
return UrlUtilities.sameDomainOrHost(info.uri().toString(), url, true);
case Type.STRICT:
return UrlUtilities.isUrlWithinScope(url, info.scopeUri().toString());
default:
assert false;
return false;
}
}
/** Applies the scope policy for navigation to {@link url}. */
public @NavigationDirective int applyPolicyForNavigationToUrl(WebappInfo info, String url) {
if (isUrlInScope(info, url)) return NavigationDirective.IGNORE_EXTERNAL_INTENT_REQUESTS;
return NavigationDirective.NORMAL_BEHAVIOR;
public static @NavigationDirective int applyPolicyForNavigationToUrl(
@Type int type, WebappInfo info, String url) {
return isUrlInScope(type, info, url) ? NavigationDirective.IGNORE_EXTERNAL_INTENT_REQUESTS
: NavigationDirective.NORMAL_BEHAVIOR;
}
}
......@@ -1285,7 +1285,7 @@ public class ExternalNavigationHandlerTest {
final String twaScope = "https://my_twa.org";
final String twaPackageName = "org.my_twa";
mDelegate.add(new IntentActivity(twaScope, twaPackageName)
.withWebappScopePolicy(WebappScopePolicy.STRICT));
.withWebappScopePolicy(WebappScopePolicy.Type.STRICT));
mDelegate.setReferrerWebappPackageName(twaPackageName);
checkUrl(twaScope + "/new.html").expecting(OverrideUrlLoadingResult.NO_OVERRIDE, IGNORE);
......@@ -1301,7 +1301,7 @@ public class ExternalNavigationHandlerTest {
final String twaScope = "https://my_twa.org";
final String twaPackageName = "org.my_twa";
mDelegate.add(new IntentActivity(twaScope, twaPackageName)
.withWebappScopePolicy(WebappScopePolicy.STRICT));
.withWebappScopePolicy(WebappScopePolicy.Type.STRICT));
mDelegate.setReferrerWebappPackageName(twaPackageName);
checkUrl(SEARCH_RESULT_URL_FOR_TOM_HANKS)
......@@ -1319,7 +1319,7 @@ public class ExternalNavigationHandlerTest {
final String twaScope = "https://my_twa.org";
final String twaPackageName = "org.my_twa";
mDelegate.add(new IntentActivity(twaScope, twaPackageName)
.withWebappScopePolicy(WebappScopePolicy.STRICT));
.withWebappScopePolicy(WebappScopePolicy.Type.STRICT));
mDelegate.setReferrerWebappPackageName(twaPackageName);
checkUrl(SEARCH_RESULT_URL_FOR_TOM_HANKS)
......@@ -1337,7 +1337,7 @@ public class ExternalNavigationHandlerTest {
final String twaScope = "https://my_twa.org";
final String twaPackageName = "org.my_twa";
mDelegate.add(new IntentActivity(twaScope, twaPackageName)
.withWebappScopePolicy(WebappScopePolicy.LEGACY));
.withWebappScopePolicy(WebappScopePolicy.Type.LEGACY));
mDelegate.setReferrerWebappPackageName(twaPackageName);
checkUrl(SEARCH_RESULT_URL_FOR_TOM_HANKS)
......@@ -1468,21 +1468,21 @@ public class ExternalNavigationHandlerTest {
private String mUrlPrefix;
private String mPackageName;
private boolean mIsWebApk;
private WebappScopePolicy mWebappScopePolicy;
private @WebappScopePolicy.Type int mWebappScopePolicy;
public IntentActivity(String urlPrefix, String packageName) {
mUrlPrefix = urlPrefix;
mPackageName = packageName;
mWebappScopePolicy = WebappScopePolicy.LEGACY;
mWebappScopePolicy = WebappScopePolicy.Type.LEGACY;
}
public IntentActivity withIsWebApk(boolean isWebApk) {
mIsWebApk = isWebApk;
mWebappScopePolicy = WebappScopePolicy.STRICT;
mWebappScopePolicy = WebappScopePolicy.Type.STRICT;
return this;
}
public IntentActivity withWebappScopePolicy(WebappScopePolicy policy) {
public IntentActivity withWebappScopePolicy(@WebappScopePolicy.Type int policy) {
mWebappScopePolicy = policy;
return this;
}
......@@ -1499,7 +1499,7 @@ public class ExternalNavigationHandlerTest {
return mIsWebApk;
}
public WebappScopePolicy webappScopePolicy() {
public @WebappScopePolicy.Type int webappScopePolicy() {
return mWebappScopePolicy;
}
......@@ -1556,8 +1556,8 @@ public class ExternalNavigationHandlerTest {
for (IntentActivity intentActivity : mIntentActivities) {
if (intentActivity.packageName().equals(mReferrerWebappPackageName)) {
WebappInfo info = newWebappInfoFromScope(intentActivity.urlPrefix());
return intentActivity.webappScopePolicy().applyPolicyForNavigationToUrl(
info, url);
return WebappScopePolicy.applyPolicyForNavigationToUrl(
intentActivity.webappScopePolicy(), info, url);
}
}
return WebappScopePolicy.NavigationDirective.NORMAL_BEHAVIOR;
......
......@@ -45,7 +45,8 @@ public class WebappVisibilityTest {
@Override
public void run() {
testCanAutoHideBrowserControls();
for (WebappScopePolicy scopePolicy : WebappScopePolicy.values()) {
for (@WebappScopePolicy.Type int scopePolicy = WebappScopePolicy.Type.LEGACY;
scopePolicy < WebappScopePolicy.Type.NUM_ENTRIES; scopePolicy++) {
for (@WebDisplayMode int displayMode : new int[] {WebDisplayMode.STANDALONE,
WebDisplayMode.FULLSCREEN, WebDisplayMode.MINIMAL_UI}) {
testShouldShowBrowserControls(scopePolicy, displayMode);
......@@ -65,7 +66,7 @@ public class WebappVisibilityTest {
}
private static void testShouldShowBrowserControls(
WebappScopePolicy scopePolicy, @WebDisplayMode int displayMode) {
@WebappScopePolicy.Type int scopePolicy, @WebDisplayMode int displayMode) {
// Show browser controls for out-of-domain URLs.
Assert.assertTrue(shouldShowBrowserControls(WEBAPP_URL, "http://notoriginalwebsite.com",
ConnectionSecurityLevel.NONE, scopePolicy, displayMode));
......@@ -85,12 +86,12 @@ public class WebappVisibilityTest {
// For WebAPKs but not Webapps show browser controls for subdomains and private
// registries that are secure.
Assert.assertEquals(
scopePolicy == WebappScopePolicy.STRICT || displayMode == WebDisplayMode.MINIMAL_UI,
Assert.assertEquals(scopePolicy == WebappScopePolicy.Type.STRICT
|| displayMode == WebDisplayMode.MINIMAL_UI,
shouldShowBrowserControls(WEBAPP_URL, "http://sub.originalwebsite.com",
ConnectionSecurityLevel.NONE, scopePolicy, displayMode));
Assert.assertEquals(
scopePolicy == WebappScopePolicy.STRICT || displayMode == WebDisplayMode.MINIMAL_UI,
Assert.assertEquals(scopePolicy == WebappScopePolicy.Type.STRICT
|| displayMode == WebDisplayMode.MINIMAL_UI,
shouldShowBrowserControls(WEBAPP_URL, "http://thing.originalwebsite.com",
ConnectionSecurityLevel.NONE, scopePolicy, displayMode));
......@@ -114,7 +115,8 @@ public class WebappVisibilityTest {
}
private static boolean shouldShowBrowserControls(String webappStartUrlOrScopeUrl, String url,
int securityLevel, WebappScopePolicy scopePolicy, @WebDisplayMode int displayMode) {
int securityLevel, @WebappScopePolicy.Type int scopePolicy,
@WebDisplayMode int displayMode) {
return WebappBrowserControlsDelegate.shouldShowBrowserControls(scopePolicy,
createWebappInfo(webappStartUrlOrScopeUrl, scopePolicy, displayMode), url,
securityLevel, false);
......@@ -125,8 +127,8 @@ public class WebappVisibilityTest {
}
private static WebappInfo createWebappInfo(String webappStartUrlOrScopeUrl,
WebappScopePolicy scopePolicy, @WebDisplayMode int displayMode) {
return scopePolicy == WebappScopePolicy.LEGACY
@WebappScopePolicy.Type int scopePolicy, @WebDisplayMode int displayMode) {
return scopePolicy == WebappScopePolicy.Type.LEGACY
? WebappInfo.create("", webappStartUrlOrScopeUrl, null, null, null, null,
displayMode, 0, 0, 0, 0, null, false /* isIconGenerated */,
false /* forceNavigation */)
......
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