[merge to 3809] Dismiss consent dialog on Android on page navigation
TBR=bsheedy@chromium.org
TBR=mthiesse@chromium.org
Bug: 968802
Change-Id: I934004dcd1b9069d5164b8f6dfccbe184f3f98c4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1649637
Reviewed-by: Michael Thiessen <mthiesse@chromium.org>
Reviewed-by: Brian Sheedy <bsheedy@chromium.org>
Reviewed-by: Bill Orr <billorr@chromium.org>
Commit-Queue: Suman Kancherla <sumankancherla@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#668168}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1657188
Cr-Commit-Position: refs/branch-heads/3809@{#273}
Cr-Branched-From: d82dec1a818f378c464ba307ddd9c92133eac355-refs/heads/master@{#665002}
diff --git a/chrome/android/features/vr/java/src/org/chromium/chrome/browser/vr/VrConsentDialog.java b/chrome/android/features/vr/java/src/org/chromium/chrome/browser/vr/VrConsentDialog.java
index c459cd6..bae3d61 100644
--- a/chrome/android/features/vr/java/src/org/chromium/chrome/browser/vr/VrConsentDialog.java
+++ b/chrome/android/features/vr/java/src/org/chromium/chrome/browser/vr/VrConsentDialog.java
@@ -14,7 +14,9 @@
import org.chromium.chrome.R;
import org.chromium.chrome.browser.ChromeActivity;
import org.chromium.chrome.browser.tab.Tab;
+import org.chromium.content_public.browser.NavigationHandle;
import org.chromium.content_public.browser.WebContents;
+import org.chromium.content_public.browser.WebContentsObserver;
import org.chromium.ui.modaldialog.DialogDismissalCause;
import org.chromium.ui.modaldialog.ModalDialogManager;
import org.chromium.ui.modaldialog.ModalDialogProperties;
@@ -24,7 +26,8 @@
* Provides a consent dialog shown before entering an immersive VR session.
*/
@JNINamespace("vr")
-public class VrConsentDialog implements ModalDialogProperties.Controller {
+public class VrConsentDialog
+ extends WebContentsObserver implements ModalDialogProperties.Controller {
@NativeMethods
/* package */ interface VrConsentUiHelperImpl {
void onUserConsentResult(long nativeGvrConsentHelperImpl, boolean allowed);
@@ -37,6 +40,7 @@
private String mUrl;
private VrConsentDialog(long instance, WebContents webContents) {
+ super(webContents);
mNativeInstance = instance;
mMetrics = new ConsentFlowMetrics(webContents);
mUrl = webContents.getLastCommittedUrl();
@@ -59,6 +63,12 @@
VrConsentDialogJni.get().onUserConsentResult(mNativeInstance, allowed);
}
+ @Override
+ public void didStartNavigation(NavigationHandle navigationHandle) {
+ mModalDialogManager.dismissAllDialogs(DialogDismissalCause.UNKNOWN);
+ onUserGesture(false);
+ }
+
public void show(@NonNull ChromeActivity activity, @NonNull VrConsentListener listener) {
mListener = listener;
@@ -90,6 +100,8 @@
@Override
public void onDismiss(PropertyModel model, int dismissalCause) {
+ if (dismissalCause == DialogDismissalCause.UNKNOWN) return;
+
if (dismissalCause == DialogDismissalCause.POSITIVE_BUTTON_CLICKED) {
mListener.onUserConsent(true);
mMetrics.logUserAction(ConsentDialogAction.USER_ALLOWED);
diff --git a/chrome/android/javatests/src/org/chromium/chrome/browser/vr/WebXrVrTestFramework.java b/chrome/android/javatests/src/org/chromium/chrome/browser/vr/WebXrVrTestFramework.java
index 718f905..c71d1d0 100644
--- a/chrome/android/javatests/src/org/chromium/chrome/browser/vr/WebXrVrTestFramework.java
+++ b/chrome/android/javatests/src/org/chromium/chrome/browser/vr/WebXrVrTestFramework.java
@@ -4,6 +4,8 @@
package org.chromium.chrome.browser.vr;
+import android.support.annotation.IntDef;
+
import org.junit.Assert;
import org.chromium.chrome.browser.vr.rules.VrTestRule;
@@ -12,10 +14,28 @@
import org.chromium.chrome.test.ChromeActivityTestRule;
import org.chromium.content_public.browser.WebContents;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+
/**
* Extension of VrTestFramework containing WebXR for VR-specific functionality.
*/
public class WebXrVrTestFramework extends WebXrTestFramework {
+ @Retention(RetentionPolicy.SOURCE)
+ @IntDef({CONSENT_DIALOG_ACTION_DO_NOTHING, CONSENT_DIALOG_ACTION_ALLOW,
+ CONSENT_DIALOG_ACTION_DENY})
+ public @interface ConsentDialogAction {}
+
+ public static final int CONSENT_DIALOG_ACTION_DO_NOTHING = 0;
+ public static final int CONSENT_DIALOG_ACTION_ALLOW = 1;
+ public static final int CONSENT_DIALOG_ACTION_DENY = 2;
+
+ // If set, a consent dialog is expected on all enterSessionWithUserGesture* methods.
+ protected boolean mShouldExpectConsentDialog = true;
+
+ @ConsentDialogAction
+ protected int mConsentDialogAction = CONSENT_DIALOG_ACTION_ALLOW;
+
public WebXrVrTestFramework(ChromeActivityTestRule rule) {
super(rule);
if (!TestVrShellDelegate.isOnStandalone()) {
@@ -24,6 +44,15 @@
}
/**
+ * Set the default action to be taken when the consent dialog is displayed.
+ *
+ * @param action The action to take on a consent dialog.
+ */
+ public void setConsentDialogAction(@ConsentDialogAction int action) {
+ mConsentDialogAction = action;
+ }
+
+ /**
* VR-specific implementation of enterSessionWithUserGesture that includes a workaround for
* receiving broadcasts late.
*
@@ -39,6 +68,13 @@
VrShellDelegateUtils.getDelegateInstance().setExpectingBroadcast();
}
super.enterSessionWithUserGesture(webContents);
+
+ if (!mShouldExpectConsentDialog) return;
+ PermissionUtils.waitForConsentPrompt(getRule().getActivity());
+ if (mConsentDialogAction == CONSENT_DIALOG_ACTION_ALLOW)
+ PermissionUtils.acceptConsentPrompt(getRule().getActivity());
+ else if (mConsentDialogAction == CONSENT_DIALOG_ACTION_DENY)
+ PermissionUtils.declineConsentPrompt(getRule().getActivity());
}
/**
diff --git a/chrome/android/javatests/src/org/chromium/chrome/browser/vr/WebXrVrTransitionTest.java b/chrome/android/javatests/src/org/chromium/chrome/browser/vr/WebXrVrTransitionTest.java
index cdd01eb..74e3836 100644
--- a/chrome/android/javatests/src/org/chromium/chrome/browser/vr/WebXrVrTransitionTest.java
+++ b/chrome/android/javatests/src/org/chromium/chrome/browser/vr/WebXrVrTransitionTest.java
@@ -47,6 +47,7 @@
import org.chromium.chrome.browser.vr.rules.XrActivityRestriction;
import org.chromium.chrome.browser.vr.util.NativeUiUtils;
import org.chromium.chrome.browser.vr.util.NfcSimUtils;
+import org.chromium.chrome.browser.vr.util.PermissionUtils;
import org.chromium.chrome.browser.vr.util.VrSettingsServiceUtils;
import org.chromium.chrome.browser.vr.util.VrTestRuleUtils;
import org.chromium.chrome.browser.vr.util.VrTransitionUtils;
@@ -508,4 +509,29 @@
NativeUiUtils.performActionAndWaitForVisibilityStatus(
UserFriendlyElementName.APP_BUTTON_EXIT_TOAST, true /* visible*/, () -> {});
}
+
+ /**
+ * Tests that a consent dialog dismisses by itself when the page navigates away from
+ * the current page.
+ */
+ @Test
+ @MediumTest
+ @CommandLineFlags
+ .Remove({"enable-webvr"})
+ @CommandLineFlags.Add({"enable-features=WebXR"})
+ @XrActivityRestriction({XrActivityRestriction.SupportedActivity.ALL})
+ public void testConsentDialogIsDismissedWhenPageNavigatesAwayInMainFrame()
+ throws InterruptedException {
+ mWebXrVrTestFramework.setConsentDialogAction(
+ WebXrVrTestFramework.CONSENT_DIALOG_ACTION_DO_NOTHING);
+ mWebXrVrTestFramework.loadUrlAndAwaitInitialization(
+ WebXrVrTestFramework.getFileUrlForHtmlTestFile("generic_webxr_page"),
+ PAGE_LOAD_TIMEOUT_S);
+ mWebXrVrTestFramework.enterSessionWithUserGesture();
+ mWebXrVrTestFramework.runJavaScriptOrFail(
+ "window.location.href = 'https://google.com'", POLL_TIMEOUT_SHORT_MS);
+ PermissionUtils.waitForConsentPromptDismissal(
+ mWebXrVrTestFramework.getRule().getActivity());
+ mWebXrVrTestFramework.assertNoJavaScriptErrors();
+ }
}
diff --git a/chrome/android/javatests/src/org/chromium/chrome/browser/vr/util/PermissionUtils.java b/chrome/android/javatests/src/org/chromium/chrome/browser/vr/util/PermissionUtils.java
index dfb5575..5ffbc76b 100644
--- a/chrome/android/javatests/src/org/chromium/chrome/browser/vr/util/PermissionUtils.java
+++ b/chrome/android/javatests/src/org/chromium/chrome/browser/vr/util/PermissionUtils.java
@@ -60,6 +60,16 @@
}
/**
+ * Blocks until the consent prompt is dismissed.
+ */
+ public static void waitForConsentPromptDismissal(ChromeActivity activity) {
+ CriteriaHelper.pollUiThread(()
+ -> { return !isConsentDialogShown(activity); },
+ "Consent prompt did not get dismissed in allotted time",
+ CriteriaHelper.DEFAULT_MAX_TIME_TO_POLL, DIALOG_POLLING_INTERVAL_MS);
+ }
+
+ /**
* Accepts the currently displayed session consent prompt.
*/
public static void acceptConsentPrompt(ChromeActivity activity) {
diff --git a/chrome/browser/android/vr/gvr_consent_helper_impl.cc b/chrome/browser/android/vr/gvr_consent_helper_impl.cc
index 80d4806..23dfa84 100644
--- a/chrome/browser/android/vr/gvr_consent_helper_impl.cc
+++ b/chrome/browser/android/vr/gvr_consent_helper_impl.cc
@@ -56,11 +56,10 @@
on_user_consent_callback_ = std::move(on_user_consent_callback);
JNIEnv* env = AttachCurrentThread();
- ScopedJavaLocalRef<jobject> jdelegate =
- Java_VrConsentDialog_promptForUserConsent(
- env, reinterpret_cast<jlong>(this),
- GetTabFromRenderer(render_process_id, render_frame_id));
- if (jdelegate.is_null()) {
+ jdelegate_ = Java_VrConsentDialog_promptForUserConsent(
+ env, reinterpret_cast<jlong>(this),
+ GetTabFromRenderer(render_process_id, render_frame_id));
+ if (jdelegate_.is_null()) {
std::move(on_user_consent_callback_).Run(false);
return;
}
@@ -70,8 +69,9 @@
JNIEnv* env,
const base::android::JavaParamRef<jobject>& j_caller,
jboolean is_granted) {
- DCHECK(on_user_consent_callback_);
- std::move(on_user_consent_callback_).Run(!!is_granted);
+ if (on_user_consent_callback_)
+ std::move(on_user_consent_callback_).Run(!!is_granted);
+ jdelegate_.Reset();
}
} // namespace vr
diff --git a/chrome/browser/android/vr/gvr_consent_helper_impl.h b/chrome/browser/android/vr/gvr_consent_helper_impl.h
index 64c07842..2fb028d 100644
--- a/chrome/browser/android/vr/gvr_consent_helper_impl.h
+++ b/chrome/browser/android/vr/gvr_consent_helper_impl.h
@@ -29,6 +29,7 @@
private:
OnUserConsentCallback on_user_consent_callback_;
+ base::android::ScopedJavaGlobalRef<jobject> jdelegate_;
DISALLOW_COPY_AND_ASSIGN(GvrConsentHelperImpl);
};