🤝 Check for discloure acceptance update in the background.

The linked bug occurred because when the disclosure notification is
accepted, we update the SharedPreferences saying so, but the UX model
is not updated as well.

Because it would be quite difficult/involved to trigger an update in
the model from the BroadcastReceiver that is triggered by the
notification, we just check to see if the underlying data has been
updated when the Activity is stopped.

Change-Id: I30067ff39c064d9bedc872ab822f30a9d8749dbf
Bug: 1168306
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2640453Reviewed-by: Ella Ge <eirage@chromium.org>
Commit-Queue: Peter Conn <peconn@chromium.org>
Cr-Commit-Position: refs/heads/master@{#845202}

chrome/android/java/src/org/chromium/chrome/browser/browserservices/ui/controller/DisclosureController.java

@@ -21,16 +21,20 @@ import org.chromium.chrome.browser.browserservices.ui.controller.CurrentPageVeri
 import org.chromium.chrome.browser.browserservices.ui.controller.CurrentPageVerifier.VerificationStatus;
 import org.chromium.chrome.browser.lifecycle.ActivityLifecycleDispatcher;
 import org.chromium.chrome.browser.lifecycle.NativeInitObserver;
+import org.chromium.chrome.browser.lifecycle.StartStopWithNativeObserver;
 
 /**
  * Contains common implementation between WebappDisclosureController and
  * TrustedWebActivityDisclosureController.
  */
 public abstract class DisclosureController
-        implements NativeInitObserver, TrustedWebActivityModel.DisclosureEventsCallback {
+        implements NativeInitObserver, TrustedWebActivityModel.DisclosureEventsCallback,
+                   StartStopWithNativeObserver {
     private final TrustedWebActivityModel mModel;
     private final CurrentPageVerifier mCurrentPageVerifier;
 
+    private boolean mPreviousShouldShowDisclosure;
+
     public DisclosureController(TrustedWebActivityModel model,
             ActivityLifecycleDispatcher lifecycleDispatcher,
             CurrentPageVerifier currentPageVerifier, String packageName) {
@@ -56,6 +60,7 @@ public abstract class DisclosureController
 
     @Override
     public void onFinishNativeInitialization() {
+        mPreviousShouldShowDisclosure = shouldShowDisclosure();
         // We want to show disclosure ASAP, which is limited by SnackbarManager requiring
         // native.
         if (shouldShowInCurrentState()) {
@@ -103,6 +108,7 @@ public abstract class DisclosureController
             mModel.set(DISCLOSURE_STATE, DISCLOSURE_STATE_NOT_SHOWN);
         }
     }
+
     protected boolean isShowing() {
         return mModel.get(DISCLOSURE_STATE) == DISCLOSURE_STATE_SHOWN;
     }
@@ -110,4 +116,23 @@ public abstract class DisclosureController
     private void setDisclosureScope(@Nullable String scope) {
         mModel.set(DISCLOSURE_SCOPE, scope);
     }
+
+    @Override
+    public void onStopWithNative() {
+        // When the disclosure is accepted through a notification, we don't get the
+        // onDisclosureAccepted callback. We check for this case (through seeing if
+        // shouldShowDisclosure has been updated) and call it manually here.
+
+        // This is a bit ugly because there's no easy way for the broadcast receiver that catches
+        // the notification click to call `onDisclosureAccepted`.
+
+        if (mPreviousShouldShowDisclosure && !shouldShowDisclosure()) {
+            onDisclosureAccepted();
+
+            mPreviousShouldShowDisclosure = false;
+        }
+    }
+
+    @Override
+    public void onStartWithNative() {}
 }

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

@@ -16,6 +16,7 @@ import static org.chromium.chrome.browser.browserservices.ui.TrustedWebActivityM
 import static org.chromium.chrome.browser.browserservices.ui.TrustedWebActivityModel.DISCLOSURE_FIRST_TIME;
 import static org.chromium.chrome.browser.browserservices.ui.TrustedWebActivityModel.DISCLOSURE_SCOPE;
 import static org.chromium.chrome.browser.browserservices.ui.TrustedWebActivityModel.DISCLOSURE_STATE;
+import static org.chromium.chrome.browser.browserservices.ui.TrustedWebActivityModel.DISCLOSURE_STATE_DISMISSED_BY_USER;
 import static org.chromium.chrome.browser.browserservices.ui.TrustedWebActivityModel.DISCLOSURE_STATE_NOT_SHOWN;
 import static org.chromium.chrome.browser.browserservices.ui.TrustedWebActivityModel.DISCLOSURE_STATE_SHOWN;
 
@@ -62,6 +63,7 @@ public class TrustedWebActivityDisclosureControllerTest {
     public ArgumentCaptor<Runnable> mVerificationObserverCaptor;
 
     public TrustedWebActivityModel mModel = new TrustedWebActivityModel();
+    private TrustedWebActivityDisclosureController mController;
 
     @Before
     public void setUp() {
@@ -73,8 +75,8 @@ public class TrustedWebActivityDisclosureControllerTest {
                 .addVerificationObserver(mVerificationObserverCaptor.capture());
         doReturn(false).when(mStore).hasUserAcceptedTwaDisclosureForPackage(anyString());
 
-        new TrustedWebActivityDisclosureController(mStore, mModel, mLifecycleDispatcher,
-                mCurrentPageVerifier, mRecorder, mClientPackageNameProvider);
+        mController = new TrustedWebActivityDisclosureController(mStore, mModel,
+                mLifecycleDispatcher, mCurrentPageVerifier, mRecorder, mClientPackageNameProvider);
     }
 
     @Test
@@ -153,6 +155,19 @@ public class TrustedWebActivityDisclosureControllerTest {
         verify(mStore).setUserSeenTwaDisclosureForPackage(CLIENT_PACKAGE);
     }
 
+    @Test
+    @Feature("TrustedWebActivities")
+    public void noticesShouldShowDisclosureChanges() {
+        mController.onFinishNativeInitialization();
+        enterVerifiedOrigin();
+        assertSnackbarShown();
+
+        doReturn(true).when(mStore).hasUserAcceptedTwaDisclosureForPackage(anyString());
+        mController.onStopWithNative();
+
+        assertEquals(DISCLOSURE_STATE_DISMISSED_BY_USER, mModel.get(DISCLOSURE_STATE));
+    }
+
     private void enterVerifiedOrigin() {
         setVerificationState(new VerificationState(SCOPE, VerificationStatus.SUCCESS));
     }