[CCT] Allow referrer as an embedded app id

This CL utilizes the referrer string as an embedded app id if its
package name is not available. This lets the branding controller work
for the host apps not passing the right package name, therefore helps
avoid the reported bug displaying the branding label repeatedly.

Bug: b:294924984
Change-Id: Ie57fa4d6ae9ee145cd6e5c2a06d00d82ed02ef8b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4803084
Reviewed-by: Wenyu Fu <wenyufu@chromium.org>
Code-Coverage: findit-for-me@appspot.gserviceaccount.com <findit-for-me@appspot.gserviceaccount.com>
Reviewed-by: Sinan Sahin <sinansahin@google.com>
Commit-Queue: Jinsuk Kim <jinsukkim@chromium.org>
Reviewed-by: Kevin Grosu <kgrosu@google.com>
Cr-Commit-Position: refs/heads/main@{#1188374}

chrome/android/java/src/org/chromium/chrome/browser/customtabs/BaseCustomTabRootUiCoordinator.java

@@ -200,13 +200,13 @@ public BaseCustomTabRootUiCoordinator(@NonNull AppCompatActivity activity,
         if (intentDataProvider.get().getActivityType() == ActivityType.CUSTOM_TAB
                 && !intentDataProvider.get().isOpenedByChrome()
                 && !intentDataProvider.get().isIncognito()) {
-            String packageName = mIntentDataProvider.get().getClientPackageName();
-            if (TextUtils.isEmpty(packageName)) {
-                packageName = CustomTabIntentDataProvider.getReferrerPackageName(activity);
+            String appId = mIntentDataProvider.get().getClientPackageName();
+            if (TextUtils.isEmpty(appId)) {
+                appId = CustomTabIntentDataProvider.getAppIdFromReferrer(activity);
             }
-            String appName = activity.getResources().getString(R.string.app_name);
+            String browserName = activity.getResources().getString(R.string.app_name);
             mBrandingController = new BrandingController(
-                    activity, packageName, appName, new ChromePureJavaExceptionReporter());
+                    activity, appId, browserName, new ChromePureJavaExceptionReporter());
         }
         mTabController = tabController;
         mPageInsightsToken = TokenHolder.INVALID_TOKEN;

chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabIntentDataProvider.java

@@ -503,19 +503,24 @@ private static int getActivitySideSheetRoundedCornersPositionFromIntent(Intent i
     }
 
     /**
-     * Get the package name from {@link #getReferrerUriString(Activity)}. If the referrer format
-     * is invalid, return an empty string.
+     * Extracts the name that identifies the embedding app from the referrer.
+     * @return Host name as an id if the referrer is of a well-formed URI with app intent scheme.
+     *    If not, just the whole referrer string.
      * TODO(https://crbug.com/1350252): Move this to IntentHandler.
-     * */
-    static String getReferrerPackageName(Activity activity) {
+     */
+    static String getAppIdFromReferrer(Activity activity) {
         String referrer =
                 CustomTabActivityLifecycleUmaTracker.getReferrerUriString(activity).toLowerCase(
                         Locale.US);
         if (TextUtils.isEmpty(referrer)) return "";
 
         Uri uri = Uri.parse(referrer);
-        return TextUtils.equals(UrlConstants.APP_INTENT_SCHEME, uri.getScheme()) ? uri.getHost()
-                                                                                 : "";
+        boolean isUrl = TextUtils.equals(UrlConstants.APP_INTENT_SCHEME, uri.getScheme());
+        if (isUrl) {
+            String host = uri.getHost();
+            if (!TextUtils.isEmpty(host)) return host;
+        }
+        return referrer;
     }
 
     /**

chrome/android/junit/src/org/chromium/chrome/browser/customtabs/CustomTabIntentDataProviderTest.java

@@ -33,7 +33,6 @@
 import android.graphics.Bitmap;
 import android.net.Uri;
 import android.os.Bundle;
-import android.text.TextUtils;
 import android.view.ContextThemeWrapper;
 
 import androidx.browser.customtabs.CustomTabColorSchemeParams;
@@ -458,18 +457,18 @@ public void sideSheetPosition() {
     }
 
     @Test
-    public void testGetReferrerPackageName() {
+    public void testGetAppIdFromReferrer() {
         assertEquals("extra.activity.referrer",
-                CustomTabIntentDataProvider.getReferrerPackageName(
+                CustomTabIntentDataProvider.getAppIdFromReferrer(
                         buildMockActivity("android-app://extra.activity.referrer")));
         assertEquals("co.abc.xyz",
-                CustomTabIntentDataProvider.getReferrerPackageName(
+                CustomTabIntentDataProvider.getAppIdFromReferrer(
                         buildMockActivity("android-app://co.abc.xyz")));
 
-        assertReferrerInvalid("");
-        assertReferrerInvalid("invalid");
-        assertReferrerInvalid("android-app://");
-        assertReferrerInvalid(Uri.parse("https://www.one.com").toString());
+        assertNonPackageUriReferrer("");
+        assertNonPackageUriReferrer("invalid");
+        assertNonPackageUriReferrer("android-app://"); // empty host name is invalid.
+        assertNonPackageUriReferrer(Uri.parse("https://www.one.com").toString());
     }
 
     @Test
@@ -785,10 +784,9 @@ protected Uri getLaunchingUrl() {
         return Uri.parse("https://www.example.com/");
     }
 
-    private void assertReferrerInvalid(String referrerStr) {
-        assertTrue("Referrer should be invalid for the input: " + referrerStr,
-                TextUtils.isEmpty(CustomTabIntentDataProvider.getReferrerPackageName(
-                        buildMockActivity(referrerStr))));
+    private void assertNonPackageUriReferrer(String referrerStr) {
+        assertEquals(referrerStr,
+                CustomTabIntentDataProvider.getAppIdFromReferrer(buildMockActivity(referrerStr)));
     }
 
     private Activity buildMockActivity(String referrer) {

chrome/browser/android/customtabs/branding/BUILD.gn

@@ -40,6 +40,7 @@ robolectric_library("junit") {
   sources = [
     "java/src/org/chromium/chrome/browser/customtabs/features/branding/BrandingCheckerUnitTest.java",
     "java/src/org/chromium/chrome/browser/customtabs/features/branding/BrandingControllerUnitTest.java",
+    "java/src/org/chromium/chrome/browser/customtabs/features/branding/SharedPreferencesBrandingTimeStorageUnitTest.java",
   ]
 
   deps = [

chrome/browser/android/customtabs/branding/java/src/org/chromium/chrome/browser/customtabs/features/branding/BrandingChecker.java

@@ -5,10 +5,13 @@
 package org.chromium.chrome.browser.customtabs.features.branding;
 
 import android.os.SystemClock;
+import android.text.TextUtils;
 
+import androidx.annotation.IntDef;
 import androidx.annotation.MainThread;
 import androidx.annotation.NonNull;
 import androidx.annotation.Nullable;
+import androidx.annotation.VisibleForTesting;
 import androidx.annotation.WorkerThread;
 
 import org.chromium.base.Callback;
@@ -17,10 +20,24 @@
 import org.chromium.base.task.AsyncTask;
 
 /**
- * Class that maintain the data for the client app package name -> last time branding is shown.
+ * Class that maintain the data for the client app id -> last time branding is shown.
  */
 class BrandingChecker extends AsyncTask<Integer> {
     public static final int BRANDING_TIME_NOT_FOUND = -1;
+
+    // These values are persisted to logs. Entries should not be renumbered and numeric values
+    // should never be reused.
+    @IntDef({BrandingAppIdType.INVALID, BrandingAppIdType.PACKAGE_NAME, BrandingAppIdType.REFERRER,
+            BrandingAppIdType.NUM_ENTRIES})
+    @interface BrandingAppIdType {
+        int INVALID = 0;
+        int PACKAGE_NAME = 1;
+        int REFERRER = 2;
+
+        // Must be the last one.
+        int NUM_ENTRIES = 3;
+    }
+
     /**
      * Interface BrandingChecked used to fetch branding information.
      * If the storage involves any worker thread operation (e.g. Disk I/O), the storage impl has
@@ -31,45 +48,44 @@ public interface BrandingLaunchTimeStorage {
          * Return the last time branding was shown for given embedded app. If not found, return
          * {@link BrandingChecker#BRANDING_TIME_NOT_FOUND}.
          *
-         * @param packageName Package name of CCT embedded app.
+         * @param appId ID of CCT embedded app.
          * @return Timestamp when CCT branding was last shown.
          * */
         @WorkerThread
-        long get(String packageName);
+        long get(String appId);
 
         /**
          * Record the timestamp when CCT branding was last shown.
          *
-         * @param packageName Package name of CCT embedded app.
+         * @param appId ID of CCT embedded app.
          * @param brandingLaunchTime Timestamp when CCT branding was last shown.
          * */
         @MainThread
-        void put(String packageName, long brandingLaunchTime);
+        void put(String appId, long brandingLaunchTime);
     }
 
-    private final String mPackageName;
+    private final String mAppId;
     private final long mBrandingCadence;
     @BrandingDecision
     private final Callback<Integer> mBrandingCheckCallback;
     @BrandingDecision
     private final int mDefaultBrandingDecision;
 
-    private @Nullable Boolean mIsPackageValid;
     private BrandingLaunchTimeStorage mStorage;
 
     /**
      * Create a BrandingChecker used to fetch BrandingDecision.
-     * @param packageName Package name of Embedded app.
+     * @param appId ID of Embedded app.
      * @param storage Storage option that used to retrieve branding information.
      * @param brandingCheckCallback Callback that will executed when branding check is complete.
      * @param brandingCadence The minimum time required to show another branding, to avoid overflow
      *                        clients with branding info.
      * @param defaultBrandingDecision Default branding decision when task is canceled.
      */
-    BrandingChecker(String packageName, BrandingLaunchTimeStorage storage,
+    BrandingChecker(String appId, BrandingLaunchTimeStorage storage,
             @NonNull @BrandingDecision Callback<Integer> brandingCheckCallback,
             long brandingCadence, @BrandingDecision int defaultBrandingDecision) {
-        mPackageName = packageName;
+        mAppId = appId;
         mStorage = storage;
         mBrandingCheckCallback = brandingCheckCallback;
         mBrandingCadence = brandingCadence;
@@ -82,16 +98,19 @@ public interface BrandingLaunchTimeStorage {
         @BrandingDecision
         Integer brandingDecision = null;
         long startTime = SystemClock.elapsedRealtime();
-        mIsPackageValid = PackageUtils.isPackageInstalled(mPackageName);
-        if (mIsPackageValid) {
-            long timeLastBranding = mStorage.get(mPackageName);
+        if (!TextUtils.isEmpty(mAppId)) {
+            long timeLastBranding = mStorage.get(mAppId);
             brandingDecision = makeBrandingDecisionFromLaunchTime(startTime, timeLastBranding);
         }
-
+        @BrandingAppIdType
+        int appIdType = getAppIdType(mAppId);
+        boolean isPackageValid = appIdType == BrandingAppIdType.PACKAGE_NAME;
         RecordHistogram.recordTimesHistogram("CustomTabs.Branding.BrandingCheckDuration",
                 SystemClock.elapsedRealtime() - startTime);
+        RecordHistogram.recordEnumeratedHistogram(
+                "CustomTabs.Branding.AppIdType", appIdType, BrandingAppIdType.NUM_ENTRIES);
         RecordHistogram.recordBooleanHistogram(
-                "CustomTabs.Branding.IsPackageNameValid", mIsPackageValid);
+                "CustomTabs.Branding.IsPackageNameValid", isPackageValid);
 
         return brandingDecision;
     }
@@ -108,6 +127,13 @@ protected void onCancelled() {
         onTaskFinished(null);
     }
 
+    @VisibleForTesting
+    static @BrandingAppIdType int getAppIdType(String appId) {
+        if (TextUtils.isEmpty(appId)) return BrandingAppIdType.INVALID;
+        if (PackageUtils.isPackageInstalled(appId)) return BrandingAppIdType.PACKAGE_NAME;
+        return BrandingAppIdType.REFERRER;
+    }
+
     private @BrandingDecision int makeBrandingDecisionFromLaunchTime(
             long startTime, long lastBrandingShowTime) {
         if (lastBrandingShowTime == BRANDING_TIME_NOT_FOUND) {
@@ -126,11 +152,9 @@ private void onTaskFinished(@BrandingDecision Integer brandingDecision) {
         }
         mBrandingCheckCallback.onResult(brandingDecision);
 
-        // Do not record branding time for invalid package name, or branding is not shown.
-        // TODO(https://crbug.com/1350658): Add short term storage option for invalid packages.
-        if (brandingDecision != BrandingDecision.NONE && mIsPackageValid != null
-                && mIsPackageValid) {
-            mStorage.put(mPackageName, taskFinishedTime);
+        // Do not record branding time for invalid app id, or branding is not shown.
+        if (brandingDecision != BrandingDecision.NONE && !TextUtils.isEmpty(mAppId)) {
+            mStorage.put(mAppId, taskFinishedTime);
         }
 
         RecordHistogram.recordEnumeratedHistogram("CustomTabs.Branding.BrandingDecision",

chrome/browser/android/customtabs/branding/java/src/org/chromium/chrome/browser/customtabs/features/branding/BrandingCheckerUnitTest.java

@@ -35,6 +35,7 @@
 import org.chromium.base.task.test.ShadowPostTask.TestImpl;
 import org.chromium.base.test.BaseRobolectricTestRunner;
 import org.chromium.base.test.util.CallbackHelper;
+import org.chromium.chrome.browser.customtabs.features.branding.BrandingChecker.BrandingAppIdType;
 import org.chromium.chrome.browser.customtabs.features.branding.BrandingChecker.BrandingLaunchTimeStorage;
 
 import java.util.HashMap;
@@ -49,7 +50,7 @@ public class BrandingCheckerUnitTest {
     private static final String PACKAGE_1 = "com.example.myapplication";
     private static final String PACKAGE_2 = "org.foo.bar";
     private static final String NEW_APPLICATION = "com.example.new.application";
-    private static final String INVALID_PACKAGE = "invalid.package";
+    private static final String INVALID_ID = "";
     private static final long TEST_BRANDING_CADENCE = 10;
     private static final long PACKAGE_1_BRANDING_SHOWN_SINCE_START = 2;
     private static final long PACKAGE_2_BRANDING_SHOWN_SINCE_START = 5;
@@ -152,19 +153,31 @@ public void testCancel() {
     @Test
     public void testInvalidPackage() {
         CallbackDelegate callbackDelegate = new CallbackDelegate();
-        BrandingChecker checker = createBrandingChecker(INVALID_PACKAGE, callbackDelegate);
+        BrandingChecker checker = createBrandingChecker(INVALID_ID, callbackDelegate);
         checker.executeOnExecutor(AsyncTask.THREAD_POOL_EXECUTOR);
 
         mainLooper().idle();
         assertEquals("Package is invalid, BrandingDecision should be the test default. ",
                 BrandingDecision.TOAST, callbackDelegate.getBrandingDecision());
-        assertEquals("Branding time should not record for invalid package.", -1,
-                mStorage.get(INVALID_PACKAGE));
+        assertEquals(
+                "Branding time should not record for invalid id.", -1, mStorage.get(INVALID_ID));
 
         assertHistogramRecorded(/*decision*/ BrandingDecision.TOAST, /*isPackageValid*/ false,
                 /*isTaskCanceled*/ false);
     }
 
+    @Test
+    public void testBrandingAppIdType() {
+        assertEquals(BrandingAppIdType.PACKAGE_NAME, BrandingChecker.getAppIdType(PACKAGE_1));
+        assertEquals(BrandingAppIdType.PACKAGE_NAME, BrandingChecker.getAppIdType(PACKAGE_2));
+
+        assertEquals(BrandingAppIdType.INVALID, BrandingChecker.getAppIdType(""));
+
+        // Package not installed. Regarded as referrer string.
+        assertEquals(BrandingAppIdType.REFERRER, BrandingChecker.getAppIdType("com.not.installed"));
+        assertEquals(BrandingAppIdType.REFERRER, BrandingChecker.getAppIdType("2//com.seedly"));
+    }
+
     @Test
     public void testLongWaitingOnStorage() {
         CallbackDelegate callbackDelegate1 = new CallbackDelegate();

chrome/browser/android/customtabs/branding/java/src/org/chromium/chrome/browser/customtabs/features/branding/BrandingController.java

@@ -81,7 +81,7 @@ public class BrandingController {
             new OneshotSupplierImpl<>();
     private final BrandingChecker mBrandingChecker;
     private final Context mContext;
-    private final String mAppName;
+    private final String mBrowserName;
     private final boolean mEnableIconAnimation;
     @Nullable
     private final PureJavaExceptionReporter mExceptionReporter;
@@ -94,14 +94,14 @@ public class BrandingController {
     /**
      * Branding controller responsible for showing branding.
      * @param context Context used to fetch package information for embedded app.
-     * @param packageName The package name for the embedded app.
-     * @param appName The appName shown on the branding toast.
+     * @param appId The ID for the embedded app.
+     * @param browserName The browser name shown on the branding toast.
      * @param exceptionReporter Optional reporter that reports wrong state quietly.
      */
-    public BrandingController(Context context, String packageName, String appName,
+    public BrandingController(Context context, String appId, String browserName,
             @Nullable PureJavaExceptionReporter exceptionReporter) {
         mContext = context;
-        mAppName = appName;
+        mBrowserName = browserName;
         mExceptionReporter = exceptionReporter;
         mEnableIconAnimation = ANIMATE_TOOLBAR_ICON_TRANSITION.getValue();
         mBrandingDecision.onAvailable(
@@ -110,7 +110,7 @@ public BrandingController(Context context, String packageName, String appName,
                 ChromeFeatureList.sCctBrandTransparencyMemoryImprovement.isEnabled();
 
         // TODO(https://crbug.com/1350661): Start branding checker during CCT warm up.
-        mBrandingChecker = new BrandingChecker(packageName,
+        mBrandingChecker = new BrandingChecker(appId,
                 SharedPreferencesBrandingTimeStorage.getInstance(), mBrandingDecision::set,
                 BRANDING_CADENCE_MS.getValue(), BrandingDecision.TOAST);
         mBrandingChecker.executeWithTaskTraits(TaskTraits.USER_VISIBLE_MAY_BLOCK);
@@ -192,7 +192,7 @@ private void showToastBranding(long durationMs) {
         }
 
         String toastText = mContext.getResources().getString(
-                R.string.twa_running_in_chrome_template, mAppName);
+                R.string.twa_running_in_chrome_template, mBrowserName);
         TextView runInChromeTextView = (TextView) LayoutInflater.from(mContext).inflate(
                 R.layout.custom_tabs_toast_branding_layout, null, false);
         runInChromeTextView.setText(toastText);