📎 Move client package name out of Verifier and its Delegate.

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

chrome/android/chrome_java_sources.gni

@@ -167,6 +167,7 @@ chrome_java_sources = [
   "java/src/org/chromium/chrome/browser/browserservices/trustedwebactivityui/TwaFinishHandler.java",
   "java/src/org/chromium/chrome/browser/browserservices/trustedwebactivityui/TwaIntentHandlingStrategy.java",
   "java/src/org/chromium/chrome/browser/browserservices/trustedwebactivityui/controller/ClientAppDataRecorder.java",
+  "java/src/org/chromium/chrome/browser/browserservices/trustedwebactivityui/controller/ClientPackageNameProvider.java",
   "java/src/org/chromium/chrome/browser/browserservices/trustedwebactivityui/controller/TrustedWebActivityBrowserControlsVisibilityManager.java",
   "java/src/org/chromium/chrome/browser/browserservices/trustedwebactivityui/controller/TrustedWebActivityDisclosureController.java",
   "java/src/org/chromium/chrome/browser/browserservices/trustedwebactivityui/controller/TrustedWebActivityOpenTimeRecorder.java",

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

@@ -5,6 +5,7 @@
 package org.chromium.chrome.browser.browserservices.trustedwebactivityui;
 
 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;
 import org.chromium.chrome.browser.browserservices.trustedwebactivityui.controller.TrustedWebActivityDisclosureController;
 import org.chromium.chrome.browser.browserservices.trustedwebactivityui.controller.TrustedWebActivityOpenTimeRecorder;
@@ -37,6 +38,7 @@ public class TrustedWebActivityCoordinator implements InflationObserver {
     private final CustomTabStatusBarColorProvider mStatusBarColorProvider;
     private final Lazy<ImmersiveModeController> mImmersiveModeController;
     private final TwaRegistrar mTwaRegistrar;
+    private final ClientPackageNameProvider mClientPackageNameProvider;
 
     private boolean mInTwaMode = true;
 
@@ -54,7 +56,8 @@ public class TrustedWebActivityCoordinator implements InflationObserver {
             ActivityLifecycleDispatcher lifecycleDispatcher,
             TrustedWebActivityBrowserControlsVisibilityManager browserControlsVisibilityManager,
             Lazy<ImmersiveModeController> immersiveModeController,
-            TwaRegistrar twaRegistrar) {
+            TwaRegistrar twaRegistrar,
+            ClientPackageNameProvider clientPackageNameProvider) {
         // 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;
@@ -62,6 +65,7 @@ public class TrustedWebActivityCoordinator implements InflationObserver {
         mStatusBarColorProvider = statusBarColorProvider;
         mImmersiveModeController = immersiveModeController;
         mTwaRegistrar = twaRegistrar;
+        mClientPackageNameProvider = clientPackageNameProvider;
 
         navigationController.setLandingPageOnCloseCriterion(verifier::isPageOnVerifiedOrigin);
         initSplashScreen(splashController, intentDataProvider, umaRecorder);
@@ -106,7 +110,7 @@ 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(mVerifier.getClientPackageName(), state.origin);
+            mTwaRegistrar.registerClient(mClientPackageNameProvider.get(), state.origin);
         }
 
         boolean inTwaMode = state == null || state.status != VerificationStatus.FAILURE;

chrome/android/java/src/org/chromium/chrome/browser/browserservices/trustedwebactivityui/controller/ClientPackageNameProvider.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 android.os.Bundle;
+
+import org.chromium.chrome.browser.ChromeActivity;
+import org.chromium.chrome.browser.customtabs.CustomTabIntentDataProvider;
+import org.chromium.chrome.browser.customtabs.CustomTabsConnection;
+import org.chromium.chrome.browser.dependency_injection.ActivityScope;
+import org.chromium.chrome.browser.lifecycle.ActivityLifecycleDispatcher;
+import org.chromium.chrome.browser.lifecycle.SaveInstanceStateObserver;
+
+import javax.inject.Inject;
+
+/**
+ * Provides the client package name for TWAs - this can come from either the Custom Tabs Connection
+ * or one previously stored in the Activity's save instance state.
+ */
+@ActivityScope
+public class ClientPackageNameProvider implements SaveInstanceStateObserver {
+    /** Key for storing in Activity instance state. */
+    private static final String KEY_CLIENT_PACKAGE = "twaClientPackageName";
+
+    private final String mClientPackageName;
+
+    @Inject
+    public ClientPackageNameProvider(
+            ChromeActivity activity,
+            ActivityLifecycleDispatcher lifecycleDispatcher,
+            CustomTabIntentDataProvider intentDataProvider,
+            CustomTabsConnection customTabsConnection) {
+        Bundle savedInstanceState = activity.getSavedInstanceState();
+        if (savedInstanceState != null) {
+            mClientPackageName = savedInstanceState.getString(KEY_CLIENT_PACKAGE);
+        } else {
+            mClientPackageName = customTabsConnection.getClientPackageNameForSession(
+                    intentDataProvider.getSession());
+        }
+        assert mClientPackageName != null;
+
+        lifecycleDispatcher.register(this);
+    }
+
+    @Override
+    public void onSaveInstanceState(Bundle outState) {
+        // TODO(pshmakov): address this problem in a more general way, http://crbug.com/952221
+        outState.putString(KEY_CLIENT_PACKAGE, mClientPackageName);
+    }
+
+    public String get() {
+        return mClientPackageName;
+    }
+}

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

@@ -30,6 +30,7 @@ public class TrustedWebActivityDisclosureController implements NativeInitObserve
     private final TrustedWebActivityModel mModel;
     private final Verifier mVerifier;
     private final TrustedWebActivityUmaRecorder mRecorder;
+    private final ClientPackageNameProvider mClientPackageNameProvider;
 
     @Inject
     TrustedWebActivityDisclosureController(
@@ -37,11 +38,13 @@ public class TrustedWebActivityDisclosureController implements NativeInitObserve
             TrustedWebActivityModel model,
             ActivityLifecycleDispatcher lifecycleDispatcher,
             Verifier verifier,
-            TrustedWebActivityUmaRecorder recorder) {
+            TrustedWebActivityUmaRecorder recorder,
+            ClientPackageNameProvider clientPackageNameProvider) {
         mVerifier = verifier;
         mPreferenceManager = preferenceManager;
         mModel = model;
         mRecorder = recorder;
+        mClientPackageNameProvider = clientPackageNameProvider;
         model.set(DISCLOSURE_EVENTS_CALLBACK, this);
         verifier.addVerificationObserver(this::onVerificationStatusChanged);
         lifecycleDispatcher.register(this);
@@ -58,7 +61,7 @@ public class TrustedWebActivityDisclosureController implements NativeInitObserve
     @Override
     public void onDisclosureAccepted() {
         mRecorder.recordDisclosureAccepted();
-        mPreferenceManager.setUserAcceptedTwaDisclosureForPackage(mVerifier.getClientPackageName());
+        mPreferenceManager.setUserAcceptedTwaDisclosureForPackage(mClientPackageNameProvider.get());
         mModel.set(DISCLOSURE_STATE, DISCLOSURE_STATE_DISMISSED_BY_USER);
     }
 
@@ -80,7 +83,7 @@ public class TrustedWebActivityDisclosureController implements NativeInitObserve
     /** Has a disclosure been dismissed for this client package before? */
     private boolean wasDismissed() {
         return mPreferenceManager.hasUserAcceptedTwaDisclosureForPackage(
-                mVerifier.getClientPackageName());
+                mClientPackageNameProvider.get());
     }
 
     @Override

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

@@ -4,20 +4,17 @@
 
 package org.chromium.chrome.browser.browserservices.trustedwebactivityui.controller;
 
-import android.os.Bundle;
-
 import org.chromium.base.Promise;
-import org.chromium.chrome.browser.ChromeActivity;
 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.chrome.browser.lifecycle.SaveInstanceStateObserver;
 import org.chromium.content_public.browser.WebContents;
 
 import javax.inject.Inject;
@@ -27,53 +24,35 @@ import androidx.browser.customtabs.CustomTabsService;
 /**
  * Provides Trusted Web Activity specific behaviour for the {@link Verifier}.
  */
-public class TwaVerifierDelegate implements VerifierDelegate, Destroyable, NativeInitObserver,
-        SaveInstanceStateObserver {
+@ActivityScope
+public class TwaVerifierDelegate implements VerifierDelegate, Destroyable, NativeInitObserver {
     /** The Digital Asset Link relationship used for Trusted Web Activities. */
     private static final int RELATIONSHIP = CustomTabsService.RELATION_HANDLE_ALL_URLS;
 
-    /** Used in activity instance state */
-    private static final String KEY_CLIENT_PACKAGE = "twaClientPackageName";
-
     private final CustomTabsConnection mCustomTabsConnection;
     private final CustomTabIntentDataProvider mIntentDataProvider;
     private final OriginVerifier mOriginVerifier;
-    private final String mClientPackageName;
 
     @Inject
     public TwaVerifierDelegate(
-            ChromeActivity activity,
             ActivityLifecycleDispatcher lifecycleDispatcher,
             CustomTabIntentDataProvider intentDataProvider,
             CustomTabsConnection customTabsConnection,
             OriginVerifier.Factory originVerifierFactory,
-            CustomTabActivityTabProvider tabProvider) {
+            CustomTabActivityTabProvider tabProvider,
+            ClientPackageNameProvider clientPackageNameProvider) {
         mCustomTabsConnection = customTabsConnection;
         mIntentDataProvider = intentDataProvider;
 
-        Bundle savedInstanceState = activity.getSavedInstanceState();
-        if (savedInstanceState != null) {
-            mClientPackageName = savedInstanceState.getString(KEY_CLIENT_PACKAGE);
-        } else {
-            mClientPackageName = customTabsConnection.getClientPackageNameForSession(
-                    intentDataProvider.getSession());
-        }
-        assert mClientPackageName != null;
-
         // TODO(peconn): See if we can get rid of the dependency on Web Contents.
         WebContents webContents =
                 tabProvider.getTab() != null ? tabProvider.getTab().getWebContents() : null;
-        mOriginVerifier =
-                originVerifierFactory.create(mClientPackageName, RELATIONSHIP, webContents);
+        mOriginVerifier = originVerifierFactory.create(
+                clientPackageNameProvider.get(), RELATIONSHIP, webContents);
 
         lifecycleDispatcher.register(this);
     }
 
-    @Override
-    public String getClientPackageName() {
-        return mClientPackageName;
-    }
-
     @Override
     public boolean wasPreviouslyVerified(Origin origin) {
         return mOriginVerifier.wasPreviouslyVerified(origin);
@@ -82,8 +61,8 @@ public class TwaVerifierDelegate implements VerifierDelegate, Destroyable, Nativ
     @Override
     public Promise<Boolean> verify(Origin origin) {
         Promise<Boolean> promise = new Promise<>();
-        mOriginVerifier.start((packageName, unused, verified, online) -> promise.fulfill(verified),
-                origin);
+        mOriginVerifier.start(
+                (packageName, unused, verified, online) -> promise.fulfill(verified), origin);
         return promise;
     }
 
@@ -93,12 +72,6 @@ public class TwaVerifierDelegate implements VerifierDelegate, Destroyable, Nativ
         mOriginVerifier.removeListener();
     }
 
-    @Override
-    public void onSaveInstanceState(Bundle outState) {
-        // TODO(pshmakov): address this problem in a more general way, http://crbug.com/952221
-        outState.putString(KEY_CLIENT_PACKAGE, mClientPackageName);
-    }
-
     @Override
     public void onFinishNativeInitialization() {
         // This doesn't belong here, but doesn't deserve a separate class. Do extract it if more

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

@@ -112,14 +112,6 @@ public class Verifier implements NativeInitObserver {
         lifecycleDispatcher.register(this);
     }
 
-    /**
-     * @return package name of the client app hosting this Trusted Web Activity.
-     */
-    public String getClientPackageName() {
-        // TODO(peconn): Remove this method.
-        return mDelegate.getClientPackageName();
-    }
-
     /**
      * @return the {@link VerificationState} of the origin we are currently in.
      * Since resolving the origin requires native, returns null before native is loaded.

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

@@ -18,8 +18,4 @@ public interface VerifierDelegate {
 
     /** Synchronously checks whether verification was successful for the given Origin. */
     boolean wasPreviouslyVerified(Origin origin);
-
-    // TODO(peconn): Get rid of this method.
-    /** Gets the package name of the connected app. */
-    String getClientPackageName();
 }

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

@@ -19,12 +19,6 @@ class TestVerifierDelegate implements VerifierDelegate {
     private final Set<Origin> mPreviouslyVerifiedOrigins = new HashSet<>();
     private final Map<Origin, Promise<Boolean>> mPendingVerifications = new HashMap<>();
 
-    private final String mPackageName;
-
-    TestVerifierDelegate(String packageName) {
-        mPackageName = packageName;
-    }
-
     @Override
     public Promise<Boolean> verify(Origin origin) {
         Promise<Boolean> promise = new Promise<>();
@@ -37,11 +31,6 @@ class TestVerifierDelegate implements VerifierDelegate {
         return mPreviouslyVerifiedOrigins.contains(origin);
     }
 
-    @Override
-    public String getClientPackageName() {
-        return mPackageName;
-    }
-
     public void passVerification(Origin origin) {
         completeVerification(origin, true);
     }

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

@@ -47,23 +47,22 @@ public class TrustedWebActivityDisclosureControllerTest {
     @Mock public ActivityLifecycleDispatcher mLifecycleDispatcher;
     @Mock public Verifier mVerifier;
     @Mock public TrustedWebActivityUmaRecorder mRecorder;
+    @Mock public ClientPackageNameProvider mClientPackageNameProvider;
 
     @Captor public ArgumentCaptor<Runnable> mVerificationObserverCaptor;
 
     public TrustedWebActivityModel mModel = new TrustedWebActivityModel();
 
-    private TrustedWebActivityDisclosureController mDisclosureController;
-
     @Before
     public void setUp() {
         MockitoAnnotations.initMocks(this);
 
-        doReturn(CLIENT_PACKAGE).when(mVerifier).getClientPackageName();
+        doReturn(CLIENT_PACKAGE).when(mClientPackageNameProvider).get();
         doNothing().when(mVerifier).addVerificationObserver(mVerificationObserverCaptor.capture());
         doReturn(false).when(mPreferences).hasUserAcceptedTwaDisclosureForPackage(anyString());
 
-        mDisclosureController = new TrustedWebActivityDisclosureController(
-                mPreferences, mModel, mLifecycleDispatcher, mVerifier, mRecorder);
+        new TrustedWebActivityDisclosureController(mPreferences, mModel, mLifecycleDispatcher,
+                mVerifier, mRecorder, mClientPackageNameProvider);
     }
 
     @Test

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

@@ -65,9 +65,11 @@ public class VerifierTest {
     @Mock CustomTabActivityTabProvider mTabProvider;
     @Mock CustomTabIntentDataProvider mIntentDataProvider;
     @Mock Tab mTab;
+    @Mock
+    ClientPackageNameProvider mClientPackageNameProvider;
     @Captor ArgumentCaptor<TabObserver> mTabObserverCaptor;
 
-    TestVerifierDelegate mVerifierDelegate = new TestVerifierDelegate(PACKAGE_NAME);
+    TestVerifierDelegate mVerifierDelegate = new TestVerifierDelegate();
 
     private Verifier mVerifier;
 
@@ -75,6 +77,7 @@ public class VerifierTest {
     public void setUp() {
         MockitoAnnotations.initMocks(this);
         when(mTabProvider.getTab()).thenReturn(mTab);
+        when(mClientPackageNameProvider.get()).thenReturn(PACKAGE_NAME);
         doNothing().when(mTabObserverRegistrar).registerTabObserver(mTabObserverCaptor.capture());
         when(mIntentDataProvider.getTrustedWebActivityAdditionalOrigins())
                 .thenReturn(Collections.singletonList("https://www.origin2.com/"));