📎 Move TwaRegistrar out of Verifier and into Coordinator.

One fewer TWA specific classes in the Verifier.

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

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

@@ -8,6 +8,7 @@ import org.chromium.chrome.browser.browserservices.TrustedWebActivityUmaRecorder
 import org.chromium.chrome.browser.browserservices.trustedwebactivityui.controller.TrustedWebActivityBrowserControlsVisibilityManager;
 import org.chromium.chrome.browser.browserservices.trustedwebactivityui.controller.TrustedWebActivityDisclosureController;
 import org.chromium.chrome.browser.browserservices.trustedwebactivityui.controller.TrustedWebActivityOpenTimeRecorder;
+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.splashscreen.TwaSplashController;
@@ -35,6 +36,7 @@ public class TrustedWebActivityCoordinator implements InflationObserver {
     private TrustedWebActivityBrowserControlsVisibilityManager mBrowserControlsVisibilityManager;
     private final CustomTabStatusBarColorProvider mStatusBarColorProvider;
     private final Lazy<ImmersiveModeController> mImmersiveModeController;
+    private final TwaRegistrar mTwaRegistrar;
 
     private boolean mInTwaMode = true;
 
@@ -51,13 +53,15 @@ public class TrustedWebActivityCoordinator implements InflationObserver {
             CustomTabStatusBarColorProvider statusBarColorProvider,
             ActivityLifecycleDispatcher lifecycleDispatcher,
             TrustedWebActivityBrowserControlsVisibilityManager browserControlsVisibilityManager,
-            Lazy<ImmersiveModeController> immersiveModeController) {
+            Lazy<ImmersiveModeController> immersiveModeController,
+            TwaRegistrar twaRegistrar) {
         // 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;
         mBrowserControlsVisibilityManager = browserControlsVisibilityManager;
         mStatusBarColorProvider = statusBarColorProvider;
         mImmersiveModeController = immersiveModeController;
+        mTwaRegistrar = twaRegistrar;
 
         navigationController.setLandingPageOnCloseCriterion(verifier::isPageOnVerifiedOrigin);
         initSplashScreen(splashController, intentDataProvider, umaRecorder);
@@ -97,6 +101,14 @@ public class TrustedWebActivityCoordinator implements InflationObserver {
 
     private void onVerificationUpdate() {
         Verifier.VerificationState state = mVerifier.getState();
+
+        // The state will start off as null and progress to PENDING then SUCCESS/FAILURE. We only
+        // 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(mVerifier.getClientPackageName(), state.origin);
+        }
+
         boolean inTwaMode = state == null || state.status != VerificationStatus.FAILURE;
         if (inTwaMode == mInTwaMode) return;
         mInTwaMode = inTwaMode;

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

@@ -60,7 +60,7 @@ public class TwaRegistrar {
      * for PostMessage). Only at step 4 do we know that Chrome should associate the browsing data
      * for that origin with that app.
      */
-    void registerClient(String packageName, Origin origin) {
+    public void registerClient(String packageName, Origin origin) {
         if (mRegisteredOrigins.contains(origin)) return;
 
         // Register that we should wipe data for this origin when the client app is uninstalled.

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

@@ -45,8 +45,6 @@ public class Verifier implements NativeInitObserver {
     private final TabObserverRegistrar mTabObserverRegistrar;
     private final BrowserServicesIntentDataProvider mIntentDataProvider;
     private final VerifierDelegate mDelegate;
-    // TODO(peconn): Move this up into the coordinator.
-    private final TwaRegistrar mTwaRegistrar;
 
     // These origins need to be verified via OriginVerifier#start, bypassing cache.
     private final Set<Origin> mOriginsToVerify = new HashSet<>();
@@ -101,15 +99,13 @@ public class Verifier implements NativeInitObserver {
             TabObserverRegistrar tabObserverRegistrar,
             CustomTabActivityTabProvider tabProvider,
             CustomTabIntentDataProvider intentDataProvider,
-            VerifierDelegate delegate,
-            TwaRegistrar twaRegistrar) {
+            VerifierDelegate delegate) {
         // 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;
-        mTwaRegistrar = twaRegistrar;
 
         tabObserverRegistrar.registerTabObserver(mVerifyOnPageLoadObserver);
         tabProvider.addObserver(mVerifyOnTabSwitchObserver);
@@ -145,6 +141,11 @@ public class Verifier implements NativeInitObserver {
     @Override
     public void onFinishNativeInitialization() {
         Origin initialOrigin = new Origin(mIntentDataProvider.getUrlToLoad());
+        if (!initialOrigin.isValid()) {
+            mTabObserverRegistrar.unregisterTabObserver(mVerifyOnPageLoadObserver);
+            updateState(initialOrigin, VerificationStatus.FAILURE);
+            return;
+        }
 
         collectTrustedOrigins(initialOrigin);
         verifyVisitedOrigin(initialOrigin);
@@ -212,10 +213,6 @@ public class Verifier implements NativeInitObserver {
     }
 
     private void updateState(Origin origin, @VerificationStatus int status) {
-        if (status == VerificationStatus.SUCCESS) {
-            mTwaRegistrar.registerClient(mDelegate.getClientPackageName(), origin);
-        }
-
         mState = new VerificationState(origin, status);
         for (Runnable observer : mObservers) {
             observer.run();

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

@@ -65,7 +65,6 @@ public class VerifierTest {
     @Mock CustomTabActivityTabProvider mTabProvider;
     @Mock CustomTabIntentDataProvider mIntentDataProvider;
     @Mock Tab mTab;
-    @Mock TwaRegistrar mTwaRegistrar;
     @Captor ArgumentCaptor<TabObserver> mTabObserverCaptor;
 
     TestVerifierDelegate mVerifierDelegate = new TestVerifierDelegate(PACKAGE_NAME);
@@ -80,7 +79,7 @@ public class VerifierTest {
         when(mIntentDataProvider.getTrustedWebActivityAdditionalOrigins())
                 .thenReturn(Collections.singletonList("https://www.origin2.com/"));
         mVerifier = new Verifier(mLifecycleDispatcher, mTabObserverRegistrar,
-                mTabProvider, mIntentDataProvider, mVerifierDelegate, mTwaRegistrar);
+                mTabProvider, mIntentDataProvider, mVerifierDelegate);
         // TODO(peconn): Add check on permission updated being updated.
     }