📎 Split further logic from Verifier.

With this CL, the split between Verifier and VerifierDelegate is more
strict. The Verifier keeps the current verification state and triggers
verification when necessary and the VerifierDelegate deals with
everything else (specifically whether we should perform full
verification or just check the cache, and what scope the verification
is relevant for).

Bug: 1018672
Change-Id: I92608eb4a7dfb8aa526679d0bb678847c63f4f97
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1893852
Commit-Queue: Peter Conn <peconn@chromium.org>
Reviewed-by: Pavel Shmakov <pshmakov@chromium.org>
Reviewed-by: Peter Kotwicz <pkotwicz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#712170}

chrome/android/chrome_junit_test_java_sources.gni

@@ -33,6 +33,7 @@ chrome_junit_test_java_sources = [
   "junit/src/org/chromium/chrome/browser/browserservices/trustedwebactivityui/controller/TestVerifierDelegate.java",
   "junit/src/org/chromium/chrome/browser/browserservices/trustedwebactivityui/controller/TrustedWebActivityDisclosureControllerTest.java",
   "junit/src/org/chromium/chrome/browser/browserservices/trustedwebactivityui/controller/TrustedWebActivityOpenTimeRecorderTest.java",
+  "junit/src/org/chromium/chrome/browser/browserservices/trustedwebactivityui/controller/TwaVerifierDelegateTest.java",
   "junit/src/org/chromium/chrome/browser/browserservices/trustedwebactivityui/controller/VerifierTest.java",
   "junit/src/org/chromium/chrome/browser/compositor/CompositorSurfaceManagerImplTest.java",
   "junit/src/org/chromium/chrome/browser/compositor/animation/CompositorAnimationHandlerTest.java",

chrome/android/java/src/org/chromium/chrome/browser/browserservices/OriginVerifier.java

@@ -167,9 +167,8 @@ public class OriginVerifier {
      */
     public static boolean wasPreviouslyVerified(String packageName, Origin origin,
             @Relation int relation) {
-        return shouldOverrideVerification(packageName, origin, relation)
-                || VerificationResultStore.isRelationshipSaved(new Relationship(packageName,
-                getCertificateSHA256FingerprintForPackage(packageName), origin, relation));
+        return wasPreviouslyVerified(packageName,
+                getCertificateSHA256FingerprintForPackage(packageName), origin, relation);
     }
 
 
@@ -186,7 +185,8 @@ public class OriginVerifier {
      */
     private static boolean wasPreviouslyVerified(String packageName, String signatureFingerprint,
             Origin origin, @Relation int relation) {
-        return VerificationResultStore.isRelationshipSaved(
+        return shouldOverrideVerification(packageName, origin, relation)
+                || VerificationResultStore.isRelationshipSaved(
                 new Relationship(packageName, signatureFingerprint, origin, relation));
     }
 

chrome/android/java/src/org/chromium/chrome/browser/browserservices/trustedwebactivityui/TrustedWebActivityCoordinator.java

@@ -4,6 +4,9 @@
 
 package org.chromium.chrome.browser.browserservices.trustedwebactivityui;
 
+import org.chromium.chrome.browser.ChromeFeatureList;
+import org.chromium.chrome.browser.browserservices.BrowserServicesIntentDataProvider;
+import org.chromium.chrome.browser.browserservices.Origin;
 import org.chromium.chrome.browser.browserservices.TrustedWebActivityUmaRecorder;
 import org.chromium.chrome.browser.browserservices.trustedwebactivityui.controller.ClientPackageNameProvider;
 import org.chromium.chrome.browser.browserservices.trustedwebactivityui.controller.TrustedWebActivityBrowserControlsVisibilityManager;
@@ -12,15 +15,18 @@ import org.chromium.chrome.browser.browserservices.trustedwebactivityui.controll
 import org.chromium.chrome.browser.browserservices.trustedwebactivityui.controller.TwaRegistrar;
 import org.chromium.chrome.browser.browserservices.trustedwebactivityui.controller.Verifier;
 import org.chromium.chrome.browser.browserservices.trustedwebactivityui.controller.Verifier.VerificationStatus;
+import org.chromium.chrome.browser.browserservices.trustedwebactivityui.controller.VerifierDelegate;
 import org.chromium.chrome.browser.browserservices.trustedwebactivityui.splashscreen.TwaSplashController;
 import org.chromium.chrome.browser.browserservices.trustedwebactivityui.view.TrustedWebActivityDisclosureView;
 import org.chromium.chrome.browser.customtabs.CustomTabIntentDataProvider;
 import org.chromium.chrome.browser.customtabs.CustomTabStatusBarColorProvider;
+import org.chromium.chrome.browser.customtabs.CustomTabsConnection;
 import org.chromium.chrome.browser.customtabs.content.CustomTabActivityNavigationController;
 import org.chromium.chrome.browser.customtabs.features.ImmersiveModeController;
 import org.chromium.chrome.browser.dependency_injection.ActivityScope;
 import org.chromium.chrome.browser.lifecycle.ActivityLifecycleDispatcher;
 import org.chromium.chrome.browser.lifecycle.InflationObserver;
+import org.chromium.chrome.browser.lifecycle.NativeInitObserver;
 
 import javax.inject.Inject;
 
@@ -48,6 +54,7 @@ public class TrustedWebActivityCoordinator implements InflationObserver {
             TrustedWebActivityDisclosureView disclosureView,
             TrustedWebActivityOpenTimeRecorder openTimeRecorder,
             Verifier verifier,
+            VerifierDelegate verifierDelegate,
             CustomTabActivityNavigationController navigationController,
             Lazy<TwaSplashController> splashController,
             CustomTabIntentDataProvider intentDataProvider,
@@ -57,7 +64,8 @@ public class TrustedWebActivityCoordinator implements InflationObserver {
             TrustedWebActivityBrowserControlsVisibilityManager browserControlsVisibilityManager,
             Lazy<ImmersiveModeController> immersiveModeController,
             TwaRegistrar twaRegistrar,
-            ClientPackageNameProvider clientPackageNameProvider) {
+            ClientPackageNameProvider clientPackageNameProvider,
+            CustomTabsConnection customTabsConnection) {
         // We don't need to do anything with most of the classes above, we just need to resolve them
         // so they start working.
         mVerifier = verifier;
@@ -67,11 +75,14 @@ public class TrustedWebActivityCoordinator implements InflationObserver {
         mTwaRegistrar = twaRegistrar;
         mClientPackageNameProvider = clientPackageNameProvider;
 
-        navigationController.setLandingPageOnCloseCriterion(verifier::isPageOnVerifiedOrigin);
+        navigationController.setLandingPageOnCloseCriterion(
+                verifierDelegate::wasPreviouslyVerified);
         initSplashScreen(splashController, intentDataProvider, umaRecorder);
 
         verifier.addVerificationObserver(this::onVerificationUpdate);
         lifecycleDispatcher.register(this);
+        lifecycleDispatcher.register(
+                new PostMessageDisabler(customTabsConnection, intentDataProvider));
     }
 
     @Override
@@ -110,7 +121,8 @@ public class TrustedWebActivityCoordinator implements InflationObserver {
         // want to register the clients once the state reaches SUCCESS, however we are happy to
         // show the TWA UI while the state is null or pending.
         if (state != null && state.status == VerificationStatus.SUCCESS) {
-            mTwaRegistrar.registerClient(mClientPackageNameProvider.get(), state.origin);
+            mTwaRegistrar.registerClient(mClientPackageNameProvider.get(),
+                    Origin.create(state.scope));
         }
 
         boolean inTwaMode = state == null || state.status != VerificationStatus.FAILURE;
@@ -128,4 +140,25 @@ public class TrustedWebActivityCoordinator implements InflationObserver {
     private void updateImmersiveMode(boolean inTwaMode) {
         // TODO(pshmakov): implement this once we can depend on tip-of-tree of androidx-browser.
     }
+
+    // This doesn't belong here, but doesn't deserve a separate class. Do extract it if more
+    // PostMessage-related code appears.
+    private static class PostMessageDisabler implements NativeInitObserver {
+        private final CustomTabsConnection mCustomTabsConnection;
+        private final BrowserServicesIntentDataProvider mIntentDataProvider;
+
+        PostMessageDisabler(CustomTabsConnection connection,
+                BrowserServicesIntentDataProvider intentDataProvider) {
+            mCustomTabsConnection = connection;
+            mIntentDataProvider = intentDataProvider;
+        }
+
+        @Override
+        public void onFinishNativeInitialization() {
+            if (!ChromeFeatureList.isEnabled(ChromeFeatureList.TRUSTED_WEB_ACTIVITY_POST_MESSAGE)) {
+                mCustomTabsConnection.resetPostMessageHandlerForSession(
+                        mIntentDataProvider.getSession(), null);
+            }
+        }
+    }
 }

chrome/android/java/src/org/chromium/chrome/browser/browserservices/trustedwebactivityui/controller/TwaVerifierDelegate.java

@@ -5,18 +5,19 @@
 package org.chromium.chrome.browser.browserservices.trustedwebactivityui.controller;
 
 import org.chromium.base.Promise;
-import org.chromium.chrome.browser.ChromeFeatureList;
 import org.chromium.chrome.browser.browserservices.Origin;
 import org.chromium.chrome.browser.browserservices.OriginVerifier;
 import org.chromium.chrome.browser.customtabs.CustomTabIntentDataProvider;
-import org.chromium.chrome.browser.customtabs.CustomTabsConnection;
 import org.chromium.chrome.browser.customtabs.content.CustomTabActivityTabProvider;
 import org.chromium.chrome.browser.dependency_injection.ActivityScope;
 import org.chromium.chrome.browser.lifecycle.ActivityLifecycleDispatcher;
 import org.chromium.chrome.browser.lifecycle.Destroyable;
-import org.chromium.chrome.browser.lifecycle.NativeInitObserver;
 import org.chromium.content_public.browser.WebContents;
 
+import java.util.HashSet;
+import java.util.List;
+import java.util.Set;
+
 import javax.inject.Inject;
 
 import androidx.browser.customtabs.CustomTabsService;
@@ -25,23 +26,23 @@ import androidx.browser.customtabs.CustomTabsService;
  * Provides Trusted Web Activity specific behaviour for the {@link Verifier}.
  */
 @ActivityScope
-public class TwaVerifierDelegate implements VerifierDelegate, Destroyable, NativeInitObserver {
+public class TwaVerifierDelegate implements VerifierDelegate, Destroyable {
     /** The Digital Asset Link relationship used for Trusted Web Activities. */
     private static final int RELATIONSHIP = CustomTabsService.RELATION_HANDLE_ALL_URLS;
 
-    private final CustomTabsConnection mCustomTabsConnection;
     private final CustomTabIntentDataProvider mIntentDataProvider;
     private final OriginVerifier mOriginVerifier;
 
+    // These origins need to be verified via OriginVerifier#start, bypassing cache.
+    private Set<Origin> mOriginsPendingVerification;
+
     @Inject
     public TwaVerifierDelegate(
             ActivityLifecycleDispatcher lifecycleDispatcher,
             CustomTabIntentDataProvider intentDataProvider,
-            CustomTabsConnection customTabsConnection,
             OriginVerifier.Factory originVerifierFactory,
             CustomTabActivityTabProvider tabProvider,
             ClientPackageNameProvider clientPackageNameProvider) {
-        mCustomTabsConnection = customTabsConnection;
         mIntentDataProvider = intentDataProvider;
 
         // TODO(peconn): See if we can get rid of the dependency on Web Contents.
@@ -54,31 +55,61 @@ public class TwaVerifierDelegate implements VerifierDelegate, Destroyable, Nativ
     }
 
     @Override
-    public boolean wasPreviouslyVerified(Origin origin) {
-        return mOriginVerifier.wasPreviouslyVerified(origin);
+    public void destroy() {
+        // Verification may finish after activity is destroyed.
+        mOriginVerifier.removeListener();
     }
 
     @Override
-    public Promise<Boolean> verify(Origin origin) {
+    public Promise<Boolean> verify(String url) {
+        Origin origin = Origin.create(url);
+        if (origin == null) return Promise.fulfilled(false);
+
+        collectTrustedOriginsIfNeeded();
         Promise<Boolean> promise = new Promise<>();
-        mOriginVerifier.start(
-                (packageName, unused, verified, online) -> promise.fulfill(verified), origin);
+        if (mOriginsPendingVerification.contains(origin)) {
+
+            mOriginVerifier.start((packageName, unused, verified, online) -> {
+                mOriginsPendingVerification.remove(origin);
+                promise.fulfill(verified);
+            }, origin);
+
+        } else {
+            promise.fulfill(mOriginVerifier.wasPreviouslyVerified(origin));
+        }
+
         return promise;
     }
 
     @Override
-    public void destroy() {
-        // Verification may finish after activity is destroyed.
-        mOriginVerifier.removeListener();
+    public String getVerifiedScope(String url) {
+        Origin origin = Origin.create(url);
+        if (origin == null) return null;
+        return origin.toString();
     }
 
     @Override
-    public void onFinishNativeInitialization() {
-        // This doesn't belong here, but doesn't deserve a separate class. Do extract it if more
-        // PostMessage-related code appears.
-        if (!ChromeFeatureList.isEnabled(ChromeFeatureList.TRUSTED_WEB_ACTIVITY_POST_MESSAGE)) {
-            mCustomTabsConnection.resetPostMessageHandlerForSession(
-                    mIntentDataProvider.getSession(), null);
+    public boolean wasPreviouslyVerified(String url) {
+        return mOriginVerifier.wasPreviouslyVerified(Origin.createOrThrow(url));
+    }
+
+    private void collectTrustedOriginsIfNeeded() {
+        if (mOriginsPendingVerification != null) return;
+
+        mOriginsPendingVerification = new HashSet<>();
+
+        Origin initialOrigin = Origin.create(mIntentDataProvider.getUrlToLoad());
+        if (initialOrigin != null) mOriginsPendingVerification.add(initialOrigin);
+
+        List<String> additionalOrigins =
+                mIntentDataProvider.getTrustedWebActivityAdditionalOrigins();
+        if (additionalOrigins != null) {
+            for (String originAsString : additionalOrigins) {
+                Origin origin = Origin.create(originAsString);
+                if (origin == null) continue;
+
+                mOriginsPendingVerification.add(origin);
+            }
         }
     }
 }

chrome/android/java/src/org/chromium/chrome/browser/browserservices/trustedwebactivityui/controller/Verifier.java

@@ -4,11 +4,9 @@
 
 package org.chromium.chrome.browser.browserservices.trustedwebactivityui.controller;
 
-import org.chromium.base.Callback;
 import org.chromium.base.ObserverList;
 import org.chromium.base.Promise;
 import org.chromium.chrome.browser.browserservices.BrowserServicesIntentDataProvider;
-import org.chromium.chrome.browser.browserservices.Origin;
 import org.chromium.chrome.browser.browserservices.trustedwebactivityui.TrustedWebActivityModel;
 import org.chromium.chrome.browser.customtabs.CustomTabIntentDataProvider;
 import org.chromium.chrome.browser.customtabs.content.CustomTabActivityTabProvider;
@@ -23,9 +21,6 @@ import org.chromium.content_public.browser.NavigationHandle;
 
 import java.lang.annotation.Retention;
 import java.lang.annotation.RetentionPolicy;
-import java.util.HashSet;
-import java.util.List;
-import java.util.Set;
 
 import javax.inject.Inject;
 
@@ -36,19 +31,13 @@ import androidx.annotation.Nullable;
 /**
  * Checks whether the currently seen web page belongs to a verified origin and updates the
  * {@link TrustedWebActivityModel} accordingly.
- *
- * TODO(peconn): Make this class work with both Origins and Scopes (for WebAPK unificiation).
  */
 @ActivityScope
 public class Verifier implements NativeInitObserver {
     private final CustomTabActivityTabProvider mTabProvider;
-    private final TabObserverRegistrar mTabObserverRegistrar;
     private final BrowserServicesIntentDataProvider mIntentDataProvider;
     private final VerifierDelegate mDelegate;
 
-    // These origins need to be verified via OriginVerifier#start, bypassing cache.
-    private final Set<Origin> mOriginsToVerify = new HashSet<>();
-
     @Nullable private VerificationState mState;
 
     private final ObserverList<Runnable> mObservers = new ObserverList<>();
@@ -63,23 +52,22 @@ public class Verifier implements NativeInitObserver {
 
     /** Represents the verification state of currently viewed web page. */
     public static class VerificationState {
-        public final Origin origin;
+        public final String scope;
         @VerificationStatus
         public final int status;
 
-        public VerificationState(Origin origin, @VerificationStatus int status) {
-            this.origin = origin;
+        public VerificationState(String scope, @VerificationStatus int status) {
+            this.scope = scope;
             this.status = status;
         }
     }
 
-    /** A {@link TabObserver} that checks whether we are on a verified Origin on page navigation. */
+    /** A {@link TabObserver} that checks whether we are on a verified page on navigation. */
     private final TabObserver mVerifyOnPageLoadObserver = new EmptyTabObserver() {
         @Override
         public void onDidFinishNavigation(Tab tab, NavigationHandle navigation) {
             if (!navigation.hasCommitted() || !navigation.isInMainFrame()) return;
-
-            verifyVisitedOrigin(Origin.createOrThrow(navigation.getUrl()));
+            verify(navigation.getUrl());
         }
     };
 
@@ -90,7 +78,7 @@ public class Verifier implements NativeInitObserver {
                     // When a link with target="_blank" is followed and the user navigates back, we
                     // don't get the onDidFinishNavigation event (because the original page wasn't
                     // navigated away from, it was only ever hidden). https://crbug.com/942088
-                    verifyVisitedOrigin(Origin.createOrThrow(tab.getUrl()));
+                    verify(tab.getUrl());
                 }
             };
 
@@ -104,7 +92,6 @@ public class Verifier implements NativeInitObserver {
         // TODO(peconn): Change the CustomTabIntentDataProvider to a BrowserServices... once
         // https://chromium-review.googlesource.com/c/chromium/src/+/1877600 has landed.
         mTabProvider = tabProvider;
-        mTabObserverRegistrar =  tabObserverRegistrar;
         mIntentDataProvider = intentDataProvider;
         mDelegate = delegate;
 
@@ -114,9 +101,8 @@ public class Verifier implements NativeInitObserver {
     }
 
     /**
-     * @return the {@link VerificationState} of the origin we are currently in.
-     * Since resolving the origin requires native, returns null before native is loaded.
-     * TODO(peconn): Is there any reason to distinguish between null and PENDING?
+     * @return the {@link VerificationState} of the page we are currently on.
+     * Since verification may require native, may return null before native is loaded.
      */
     @Nullable
     public VerificationState getState() {
@@ -133,69 +119,22 @@ public class Verifier implements NativeInitObserver {
 
     @Override
     public void onFinishNativeInitialization() {
-        // This value comes from an externally sent Intent, it may be invalid.
-        Origin initialOrigin = Origin.create(mIntentDataProvider.getUrlToLoad());
-
-        if (initialOrigin == null) {
-            mTabObserverRegistrar.unregisterTabObserver(mVerifyOnPageLoadObserver);
-            updateState(initialOrigin, VerificationStatus.FAILURE);
-            return;
-        }
-
-        collectTrustedOrigins(initialOrigin);
-        verifyVisitedOrigin(initialOrigin);
-    }
-
-    private void collectTrustedOrigins(Origin initialOrigin) {
-        mOriginsToVerify.add(initialOrigin);
-        List<String> additionalOrigins =
-                mIntentDataProvider.getTrustedWebActivityAdditionalOrigins();
-        if (additionalOrigins != null) {
-            for (String originAsString : additionalOrigins) {
-                // This value comes from an externally sent Intent, it may be invalid.
-                Origin origin = Origin.create(originAsString);
-                if (origin != null) mOriginsToVerify.add(origin);
-            }
-        }
-    }
-
-    /**
-     * Returns whether the given |url| is on an Origin that the package has been previously
-     * verified for.
-     */
-    public boolean isPageOnVerifiedOrigin(String url) {
-        Origin origin = Origin.create(url);
-        if (origin == null) return false;
-        return mDelegate.wasPreviouslyVerified(origin);
+        verify(mIntentDataProvider.getUrlToLoad());
     }
 
     /**
-     * Verifies an arbitrary url.
-     * Returns a {@link Promise<Boolean>} with boolean telling whether verification succeeded.
+     * Perform verification for the given page.
      */
-    public Promise<Boolean> verifyOrigin(String url) {
-        Origin origin = Origin.create(url);
-        if (origin == null) return Promise.fulfilled(false);
-
-        if (mDelegate.wasPreviouslyVerified(origin)) {
-            return Promise.fulfilled(true);
-        }
-        return mDelegate.verify(origin);
-    }
+    private void verify(String url) {
+        Promise<Boolean> result = mDelegate.verify(url);
+        String scope = mDelegate.getVerifiedScope(url);
+        if (scope == null) return;
 
-    /**
-     * Perform verification for the origin the user is currently on.
-     */
-    private void verifyVisitedOrigin(Origin origin) {
-        if (mOriginsToVerify.contains(origin)) {
-            // Do verification bypassing the cache.
-            updateState(origin, VerificationStatus.PENDING);
-            mDelegate.verify(origin).then(
-                    (Callback<Boolean>) verified -> onVerificationResult(origin, verified));
+        if (result.isFulfilled()) {
+            updateState(scope, statusFromBoolean(result.getResult()));
         } else {
-            // Look into cache only
-            boolean verified = mDelegate.wasPreviouslyVerified(origin);
-            updateState(origin, verified ? VerificationStatus.SUCCESS : VerificationStatus.FAILURE);
+            updateState(scope, VerificationStatus.PENDING);
+            result.then(verified -> { onVerificationResult(scope, verified); });
         }
     }
 
@@ -204,19 +143,24 @@ public class Verifier implements NativeInitObserver {
      * client called |validateRelationship| before launching the TWA and we found that verification
      * in the cache.
      */
-    private void onVerificationResult(Origin origin, boolean verified) {
-        mOriginsToVerify.remove(origin);
+    private void onVerificationResult(String scope, boolean verified) {
         Tab tab = mTabProvider.getTab();
-        boolean stillOnSameOrigin = tab != null && origin.equals(Origin.create(tab.getUrl()));
-        if (stillOnSameOrigin) {
-            updateState(origin, verified ? VerificationStatus.SUCCESS : VerificationStatus.FAILURE);
+
+        boolean resultStillApplies = tab != null
+                && scope.equals(mDelegate.getVerifiedScope(tab.getUrl()));
+        if (resultStillApplies) {
+            updateState(scope, verified ? VerificationStatus.SUCCESS : VerificationStatus.FAILURE);
         }
     }
 
-    private void updateState(Origin origin, @VerificationStatus int status) {
-        mState = new VerificationState(origin, status);
+    private void updateState(String scope, @VerificationStatus int status) {
+        mState = new VerificationState(scope, status);
         for (Runnable observer : mObservers) {
             observer.run();
         }
     }
+
+    private static @VerificationStatus int statusFromBoolean(boolean success) {
+        return success ? VerificationStatus.SUCCESS : VerificationStatus.FAILURE;
+    }
 }

chrome/android/java/src/org/chromium/chrome/browser/browserservices/trustedwebactivityui/controller/VerifierDelegate.java

@@ -5,17 +5,55 @@
 package org.chromium.chrome.browser.browserservices.trustedwebactivityui.controller;
 
 import org.chromium.base.Promise;
-import org.chromium.chrome.browser.browserservices.Origin;
+
+import androidx.annotation.Nullable;
 
 /**
  * A Delegate for the {@link Verifier} that provides implementation specific to
  * Trusted Web Activities, WebAPKs or A2HS as appropriate.
  */
 public interface VerifierDelegate {
-    // TODO(peconn): Make distinction between verify and wasPreviouslyVerified more clear.
-    /** Asynchronously checks whether the given Origin is verified. */
-    Promise<Boolean> verify(Origin origin);
+    /**
+     * Checks whether the given URL is verified.
+     *
+     * The returned Promise may be immediately fulfilled (eg if we know that the given url is one
+     * we shouldn't even attempt to verify or if we have a cached result). It is worth explicitly
+     * checking for this to deal with the result synchronously and not incur the delay of
+     * {@link Promise#then}.
+     */
+    Promise<Boolean> verify(String url);
+
+    /**
+     * A synchronous version of verify that returns true iff verification has previously been
+     * completed successfully for the given url.
+     */
+    boolean wasPreviouslyVerified(String url);
 
-    /** Synchronously checks whether verification was successful for the given Origin. */
-    boolean wasPreviouslyVerified(Origin origin);
+    /**
+     * Returns the widest scope for which verification is relevant. This can be used to determine
+     * whether two different urls are the same for the purposes of verification. Returns
+     * {@link null} if the given url cannot be parsed.
+     *
+     * The purpose of this method is to determine whether two different pages can share verification
+     * state. Eg, if we've already verified a TWA for https://www.example.com/webapp/page1.html we
+     * don't need to perform verification again for https://www.example.com/webapp/folder/page2.html
+     * (but we do for https://developers.google.com/web/updates).
+     *
+     * eg, for a TWA where verification is on a per origin basis, this method would map to origins:
+     * https://www.example.com/webapp/page1.html        -> https://www.example.com
+     * https://www.example.com/webapp/folder/page2.html -> https://www.example.com
+     * https://developers.google.com/web/updates        -> https://developers.google.com
+     *
+     * eg, say we have a WebAPK with the verified scope being https://www.example.com/webapp/, then
+     * this method would map:
+     *
+     * https://www.example.com/webapp/page1.html         -> https://www.example.com/webapp/
+     * https://www.example.com/webapp/folder/page2.html  -> https://www.example.com/webapp/
+     * https://www.example.com/somewhere_else/page3.html -> https://www.example.com/somewhere_else/
+     *
+     * The last result can really be anything other than https://www.example.com/webapp/ - just
+     * something to signify we aren't on the verified scope.
+     */
+    @Nullable
+    String getVerifiedScope(String url);
 }

chrome/android/java/src/org/chromium/chrome/browser/browserservices/trustedwebactivityui/sharing/TwaSharingController.java

@@ -11,7 +11,7 @@ import android.util.Pair;
 import org.chromium.base.Promise;
 import org.chromium.chrome.browser.browserservices.TrustedWebActivityUmaRecorder;
 import org.chromium.chrome.browser.browserservices.TrustedWebActivityUmaRecorder.ShareRequestMethod;
-import org.chromium.chrome.browser.browserservices.trustedwebactivityui.controller.Verifier;
+import org.chromium.chrome.browser.browserservices.trustedwebactivityui.controller.TwaVerifierDelegate;
 import org.chromium.chrome.browser.customtabs.CustomTabIntentDataProvider;
 import org.chromium.chrome.browser.customtabs.content.CustomTabActivityNavigationController;
 import org.chromium.chrome.browser.customtabs.content.CustomTabActivityTabProvider;
@@ -36,19 +36,19 @@ public class TwaSharingController {
     private final CustomTabActivityTabProvider mTabProvider;
     private final CustomTabActivityNavigationController mNavigationController;
     private final WebApkPostShareTargetNavigator mPostNavigator;
-    private final Verifier mVerifier;
+    private final TwaVerifierDelegate mVerifierDelegate;
     private final TrustedWebActivityUmaRecorder mUmaRecorder;
 
     @Inject
     public TwaSharingController(CustomTabActivityTabProvider tabProvider,
             CustomTabActivityNavigationController navigationController,
             WebApkPostShareTargetNavigator postNavigator,
-            Verifier verifier,
+            TwaVerifierDelegate verifierDelegate,
             TrustedWebActivityUmaRecorder umaRecorder) {
         mTabProvider = tabProvider;
         mNavigationController = navigationController;
         mPostNavigator = postNavigator;
-        mVerifier = verifier;
+        mVerifierDelegate = verifierDelegate;
         mUmaRecorder = umaRecorder;
     }
 
@@ -65,7 +65,7 @@ public class TwaSharingController {
             return Promise.fulfilled(false);
         }
 
-        return mVerifier.verifyOrigin(shareTarget.action).then(
+        return mVerifierDelegate.verify(shareTarget.action).then(
                 (Promise.Function<Boolean, Boolean>) (verified) -> {
             if (!verified) {
                 return false;

chrome/android/junit/src/org/chromium/chrome/browser/browserservices/trustedwebactivityui/controller/TestVerifierDelegate.java

@@ -19,18 +19,6 @@ class TestVerifierDelegate implements VerifierDelegate {
     private final Set<Origin> mPreviouslyVerifiedOrigins = new HashSet<>();
     private final Map<Origin, Promise<Boolean>> mPendingVerifications = new HashMap<>();
 
-    @Override
-    public Promise<Boolean> verify(Origin origin) {
-        Promise<Boolean> promise = new Promise<>();
-        mPendingVerifications.put(origin, promise);
-        return promise;
-    }
-
-    @Override
-    public boolean wasPreviouslyVerified(Origin origin) {
-        return mPreviouslyVerifiedOrigins.contains(origin);
-    }
-
     public void passVerification(Origin origin) {
         completeVerification(origin, true);
     }
@@ -52,7 +40,20 @@ class TestVerifierDelegate implements VerifierDelegate {
         return mPendingVerifications.containsKey(origin);
     }
 
-    public boolean hasAnyPendingVerifications() {
-        return mPendingVerifications.size() != 0;
+    @Override
+    public Promise<Boolean> verify(String url) {
+        Promise<Boolean> promise = new Promise<>();
+        mPendingVerifications.put(Origin.createOrThrow(url), promise);
+        return promise;
+    }
+
+    @Override
+    public boolean wasPreviouslyVerified(String url) {
+        throw new UnsupportedOperationException();
+    }
+
+    @Override
+    public String getVerifiedScope(String url) {
+        return Origin.createOrThrow(url).toString();
     }
 }

chrome/android/junit/src/org/chromium/chrome/browser/browserservices/trustedwebactivityui/controller/TrustedWebActivityDisclosureControllerTest.java

@@ -26,7 +26,6 @@ import org.robolectric.annotation.Config;
 
 import org.chromium.base.test.BaseRobolectricTestRunner;
 import org.chromium.base.test.util.Feature;
-import org.chromium.chrome.browser.browserservices.Origin;
 import org.chromium.chrome.browser.browserservices.TrustedWebActivityUmaRecorder;
 import org.chromium.chrome.browser.browserservices.trustedwebactivityui.TrustedWebActivityModel;
 import org.chromium.chrome.browser.browserservices.trustedwebactivityui.controller.Verifier.VerificationState;
@@ -41,7 +40,7 @@ import org.chromium.chrome.browser.preferences.ChromePreferenceManager;
 @Config(manifest = Config.NONE)
 public class TrustedWebActivityDisclosureControllerTest {
     private static final String CLIENT_PACKAGE = "com.example.twaclient";
-    private static final Origin ORIGIN = Origin.create("com.example.twa");
+    private static final String SCOPE = "https://www.example.com";
 
     @Mock public ChromePreferenceManager mPreferences;
     @Mock public ActivityLifecycleDispatcher mLifecycleDispatcher;
@@ -106,11 +105,11 @@ public class TrustedWebActivityDisclosureControllerTest {
     }
 
     private void enterVerifiedOrigin() {
-        setVerificationState(new VerificationState(ORIGIN, VerificationStatus.SUCCESS));
+        setVerificationState(new VerificationState(SCOPE, VerificationStatus.SUCCESS));
     }
 
     private void exitVerifiedOrigin() {
-        setVerificationState(new VerificationState(ORIGIN, VerificationStatus.FAILURE));
+        setVerificationState(new VerificationState(SCOPE, VerificationStatus.FAILURE));
     }
 
     private void setVerificationState(VerificationState state) {

chrome/android/junit/src/org/chromium/chrome/browser/browserservices/trustedwebactivityui/controller/TrustedWebActivityOpenTimeRecorderTest.java

@@ -8,7 +8,6 @@ import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.anyLong;
 import static org.mockito.Mockito.clearInvocations;
 import static org.mockito.Mockito.doNothing;
-import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
@@ -25,7 +24,6 @@ import org.robolectric.annotation.Config;
 
 import org.chromium.base.test.BaseRobolectricTestRunner;
 import org.chromium.chrome.browser.ActivityTabProvider;
-import org.chromium.chrome.browser.browserservices.Origin;
 import org.chromium.chrome.browser.browserservices.TrustedWebActivityUmaRecorder;
 import org.chromium.chrome.browser.browserservices.trustedwebactivityui.controller.Verifier.VerificationState;
 import org.chromium.chrome.browser.browserservices.trustedwebactivityui.controller.Verifier.VerificationStatus;
@@ -203,7 +201,7 @@ public class TrustedWebActivityOpenTimeRecorderTest {
     }
 
     private void setVerificationStatus(@VerificationStatus int status) {
-        VerificationState newState = new VerificationState(mock(Origin.class), status);
+        VerificationState newState = new VerificationState("www.example.com", status);
         when(mVerifier.getState()).thenReturn(newState);
         for (Runnable observer : mVerificationObserverCaptor.getAllValues()) {
             observer.run();

chrome/android/junit/src/org/chromium/chrome/browser/browserservices/trustedwebactivityui/controller/TwaVerifierDelegateTest.java

+// Copyright 2019 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+package org.chromium.chrome.browser.browserservices.trustedwebactivityui.controller;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.anyInt;
+import static org.mockito.ArgumentMatchers.anyString;
+import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.reset;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.TestRule;
+import org.junit.runner.RunWith;
+import org.mockito.ArgumentCaptor;
+import org.mockito.Mock;
+import org.mockito.MockitoAnnotations;
+import org.robolectric.annotation.Config;
+
+import org.chromium.base.Promise;
+import org.chromium.base.test.BaseRobolectricTestRunner;
+import org.chromium.chrome.browser.ChromeFeatureList;
+import org.chromium.chrome.browser.browserservices.Origin;
+import org.chromium.chrome.browser.browserservices.OriginVerifier;
+import org.chromium.chrome.browser.browserservices.OriginVerifier.OriginVerificationListener;
+import org.chromium.chrome.browser.customtabs.CustomTabIntentDataProvider;
+import org.chromium.chrome.browser.customtabs.CustomTabsConnection;
+import org.chromium.chrome.browser.customtabs.content.CustomTabActivityTabProvider;
+import org.chromium.chrome.browser.lifecycle.ActivityLifecycleDispatcher;
+import org.chromium.chrome.test.util.browser.Features;
+import org.chromium.chrome.test.util.browser.Features.DisableFeatures;
+
+import java.util.Collections;
+
+/**
+ * Tests for {@link TwaVerifierDelegate}.
+ */
+@RunWith(BaseRobolectricTestRunner.class)
+@Config(manifest = Config.NONE)
+@DisableFeatures(ChromeFeatureList.TRUSTED_WEB_ACTIVITY_POST_MESSAGE)
+public class TwaVerifierDelegateTest {
+    private static final String INITIAL_URL = "https://www.initialurl.com/page.html";
+    private static final String ADDITIONAL_ORIGIN = "https://www.otherverifiedorigin.com";
+    private static final String OTHER_URL = "https://www.notverifiedurl.com/page2.html";
+
+    @Rule public TestRule mFeaturesProcessor = new Features.JUnitProcessor();
+
+    @Mock ActivityLifecycleDispatcher mLifecycleDispatcher;
+    @Mock CustomTabIntentDataProvider mIntentDataProvider;
+    @Mock CustomTabsConnection mCustomTabsConnection;
+    @Mock OriginVerifier.Factory mOriginVerifierFactory;
+    @Mock OriginVerifier mOriginVerifier;
+    @Mock CustomTabActivityTabProvider mActivityTabProvider;
+    @Mock ClientPackageNameProvider mClientPackageNameProvider;
+
+    private TwaVerifierDelegate mDelegate;
+
+    @Before
+    public void setUp() {
+        MockitoAnnotations.initMocks(this);
+
+        when(mIntentDataProvider.getUrlToLoad()).thenReturn(INITIAL_URL);
+        when(mIntentDataProvider.getTrustedWebActivityAdditionalOrigins()).thenReturn(
+                Collections.singletonList(ADDITIONAL_ORIGIN));
+
+        when(mOriginVerifierFactory.create(anyString(), anyInt(), any()))
+                .thenReturn(mOriginVerifier);
+
+        when(mClientPackageNameProvider.get()).thenReturn("some.package.name");
+
+        mDelegate = new TwaVerifierDelegate(mLifecycleDispatcher, mIntentDataProvider,
+                mOriginVerifierFactory, mActivityTabProvider, mClientPackageNameProvider);
+    }
+
+    @Test
+    public void verifiedScopeIsOrigin() {
+        assertEquals("https://www.example.com",
+                mDelegate.getVerifiedScope("https://www.example.com"));
+        assertEquals("https://www.example.com",
+                mDelegate.getVerifiedScope("https://www.example.com/page1.html"));
+        assertEquals("https://www.example.com",
+                mDelegate.getVerifiedScope("https://www.example.com/dir/page2.html"));
+    }
+
+    @Test
+    public void isPageInVerificationCache() {
+        Origin trusted = Origin.create("https://www.trusted.com");
+        Origin untrusted = Origin.create("https://www.untrusted.com");
+        when(mOriginVerifier.wasPreviouslyVerified(eq(trusted))).thenReturn(true);
+        when(mOriginVerifier.wasPreviouslyVerified(eq(untrusted))).thenReturn(false);
+
+        assertTrue(mDelegate.wasPreviouslyVerified(trusted.toString()));
+        assertFalse(mDelegate.wasPreviouslyVerified(untrusted.toString()));
+    }
+
+    @Test
+    public void verify_failsInvalidUrl() {
+        Promise<Boolean> result = mDelegate.verify("not-an-origin");
+        assertFalse(result.getResult());
+    }
+
+    /** Tests that the first call to verify for a trusted origin performs full verification. */
+    @Test
+    public void verify_firstCall() {
+        verifyFullCheck(INITIAL_URL);
+    }
+
+    /** Tests that subsequent calls to verify for a given origin just check the cache. */
+    @Test
+    public void verify_subsequentCalls() {
+        verifyFullCheck(INITIAL_URL);
+        verifyCacheCheck(INITIAL_URL);
+    }
+
+    /** Tests that things work as they should navigating to another verified origin and back. */
+    @Test
+    public void verify_acrossOriginSubsequentCalls() {
+        verifyFullCheck(INITIAL_URL);
+        verifyFullCheck(ADDITIONAL_ORIGIN);
+        verifyCacheCheck(INITIAL_URL);
+        verifyCacheCheck(ADDITIONAL_ORIGIN);
+    }
+
+    private void verifyFullCheck(String url) {
+        Promise<Boolean> promise = mDelegate.verify(url);
+
+        ArgumentCaptor<OriginVerificationListener> callback =
+                ArgumentCaptor.forClass(OriginVerificationListener.class);
+        verify(mOriginVerifier).start(callback.capture(), eq(Origin.create(url)));
+
+        callback.getValue().onOriginVerified(null, null, true, true);
+
+        assertTrue(promise.getResult());
+
+        // So we don't interfere with the rest of the test.
+        reset(mOriginVerifier);
+    }
+
+    private void verifyCacheCheck(String url) {
+        Origin origin = Origin.create(url);
+
+        when(mOriginVerifier.wasPreviouslyVerified(eq(origin))).thenReturn(true);
+
+        Promise<Boolean> promise = mDelegate.verify(url);
+
+        verify(mOriginVerifier, never()).start(any(), any());
+        verify(mOriginVerifier).wasPreviouslyVerified(eq(origin));
+
+        assertTrue(promise.getResult());
+
+        // So we don't interfere with the rest of the test.
+        reset(mOriginVerifier);
+    }
+
+    @Test
+    public void verify_notTrustedOrigin() {
+        Promise<Boolean> promise = mDelegate.verify(OTHER_URL);
+
+        verify(mOriginVerifier, never()).start(any(), any());
+        verify(mOriginVerifier).wasPreviouslyVerified(eq(Origin.create(OTHER_URL)));
+
+        assertFalse(promise.getResult());
+    }
+}

chrome/android/junit/src/org/chromium/chrome/browser/browserservices/trustedwebactivityui/controller/VerifierTest.java

@@ -5,7 +5,6 @@
 package org.chromium.chrome.browser.browserservices.trustedwebactivityui.controller;
 
 import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
 import static org.mockito.Mockito.doNothing;
 import static org.mockito.Mockito.when;
@@ -46,13 +45,10 @@ import java.util.Collections;
 @EnableFeatures(ChromeFeatureList.TRUSTED_WEB_ACTIVITY)
 @DisableFeatures(ChromeFeatureList.TRUSTED_WEB_ACTIVITY_POST_MESSAGE)
 public class VerifierTest {
-
     private static final Origin TRUSTED_ORIGIN = Origin.create("https://www.origin1.com/");
     private static final Origin OTHER_TRUSTED_ORIGIN = Origin.create("https://www.origin2.com/");
     private static final String TRUSTED_ORIGIN_PAGE1 = TRUSTED_ORIGIN + "/page1";
-    private static final String TRUSTED_ORIGIN_PAGE2 = TRUSTED_ORIGIN + "/page2";
     private static final String OTHER_TRUSTED_ORIGIN_PAGE1 = OTHER_TRUSTED_ORIGIN + "/page1";
-    private static final String OTHER_TRUSTED_ORIGIN_PAGE2 = OTHER_TRUSTED_ORIGIN + "/page2";
     private static final String UNTRUSTED_PAGE = "https://www.origin3.com/page1";
 
     public static final String PACKAGE_NAME = "package.name";
@@ -116,16 +112,6 @@ public class VerifierTest {
         assertStatus(VerificationStatus.FAILURE);
     }
 
-    @Test
-    public void usesCache_whenNavigatingWithinPreviouslyVerifiedOrigin() {
-        setInitialUrl(TRUSTED_ORIGIN_PAGE1);
-        mVerifier.onFinishNativeInitialization();
-        mVerifierDelegate.passVerification(Origin.create(TRUSTED_ORIGIN_PAGE1));
-
-        navigateToUrl(TRUSTED_ORIGIN_PAGE2);
-        verifyUsesCache();
-    }
-
     @Test
     public void verifies_WhenNavigatingToOtherTrustedOrigin() {
         setInitialUrl(TRUSTED_ORIGIN_PAGE1);
@@ -136,41 +122,6 @@ public class VerifierTest {
         verifyStartsVerification(OTHER_TRUSTED_ORIGIN_PAGE1);
     }
 
-    @Test
-    public void usesCache_WhenReturningBackToFirstVerifiedOrigin() {
-        setInitialUrl(TRUSTED_ORIGIN_PAGE1);
-        mVerifier.onFinishNativeInitialization();
-        mVerifierDelegate.passVerification(Origin.create(TRUSTED_ORIGIN_PAGE1));
-        navigateToUrl(OTHER_TRUSTED_ORIGIN_PAGE1);
-        mVerifierDelegate.passVerification(Origin.create(OTHER_TRUSTED_ORIGIN_PAGE1));
-
-        navigateToUrl(TRUSTED_ORIGIN_PAGE2);
-        verifyUsesCache();
-    }
-
-    @Test
-    public void usesCache_WhenReturningBackToSecondVerifiedOrigin() {
-        setInitialUrl(TRUSTED_ORIGIN_PAGE1);
-        mVerifier.onFinishNativeInitialization();
-        mVerifierDelegate.passVerification(Origin.create(TRUSTED_ORIGIN_PAGE1));
-        navigateToUrl(OTHER_TRUSTED_ORIGIN_PAGE1);
-        mVerifierDelegate.passVerification(Origin.create(OTHER_TRUSTED_ORIGIN_PAGE1));
-        navigateToUrl(TRUSTED_ORIGIN_PAGE1);
-
-        navigateToUrl(OTHER_TRUSTED_ORIGIN_PAGE2);
-        verifyUsesCache();
-    }
-
-    @Test
-    public void usesCache_WhenNavigatesToUntrustedOrigin() {
-        setInitialUrl(TRUSTED_ORIGIN_PAGE1);
-        mVerifier.onFinishNativeInitialization();
-        mVerifierDelegate.passVerification(Origin.create(TRUSTED_ORIGIN_PAGE1));
-
-        navigateToUrl(UNTRUSTED_PAGE);
-        verifyUsesCache();
-    }
-
     @Test
     public void doesntUpdateState_IfVerificationFinishedAfterLeavingOrigin() {
         setInitialUrl(TRUSTED_ORIGIN_PAGE1);
@@ -196,10 +147,6 @@ public class VerifierTest {
         assertEquals(status, mVerifier.getState().status);
     }
 
-    private void verifyUsesCache() {
-        assertFalse(mVerifierDelegate.hasAnyPendingVerifications());
-    }
-
     private void verifyStartsVerification(String url) {
         assertTrue(mVerifierDelegate.hasPendingVerification(Origin.create(url)));
     }