VR: Ensure we remove black overlay if VR is exited while entering.

This fixes a failure mode where we shutdownVR after handling the VR NFC
broadcast, and fail to properly cancel VR entry.

Also does some other random cleanup of comments/visibility.

Bug: 862829
Change-Id: Ic382692d7255aa5893391a602a6c949b4b4d498b
Reviewed-on: https://chromium-review.googlesource.com/1138726Reviewed-by: Tibor Goldschwendt <tiborg@chromium.org>
Commit-Queue: Michael Thiessen <mthiesse@chromium.org>
Cr-Commit-Position: refs/heads/master@{#575388}

chrome/android/java/src/org/chromium/chrome/browser/vr/VrAlertDialog.java

@@ -16,19 +16,15 @@ import org.chromium.chrome.browser.modaldialog.ModalDialogView;
  * This class implements a VrAlertDialog which is similar to Android AlertDialog in VR.
  */
 public class VrAlertDialog extends AlertDialog {
-    private VrDialogManager mVrDialogManager;
     private ModalDialogManager mModalDialogManager;
     private ModalDialogView mModalDialogView;
-    private boolean mIsShowing;
     private CharSequence mMessage;
     private DialogButton mButtonPositive;
     private DialogButton mButtonNegative;
     protected View mView;
 
-    public VrAlertDialog(Context context, VrDialogManager vrDialogManager,
-            ModalDialogManager modalDialogManager) {
+    public VrAlertDialog(Context context, ModalDialogManager modalDialogManager) {
         super(context);
-        mVrDialogManager = vrDialogManager;
         mModalDialogManager = modalDialogManager;
     }
 
@@ -60,7 +56,6 @@ public class VrAlertDialog extends AlertDialog {
     public void show() {
         mModalDialogView = createView();
         mModalDialogManager.showDialog(mModalDialogView, ModalDialogManager.APP_MODAL);
-        mIsShowing = true;
     }
 
     /**
@@ -103,7 +98,6 @@ public class VrAlertDialog extends AlertDialog {
      */
     @Override
     public void dismiss() {
-        mIsShowing = false;
         mModalDialogManager.cancelDialog(mModalDialogView);
     }
 

chrome/android/java/src/org/chromium/chrome/browser/vr/VrFirstRunActivity.java

@@ -24,8 +24,6 @@ public class VrFirstRunActivity extends Activity {
 
     private static final BooleanHistogramSample sFreNotCompleteBrowserHistogram =
             new BooleanHistogramSample("VRFreNotComplete.Browser");
-    private static final BooleanHistogramSample sFreNotCompleteAutopresentHistogram =
-            new BooleanHistogramSample("VRFreNotComplete.WebVRAutopresent");
 
     @Override
     protected void onCreate(Bundle savedInstanceState) {

chrome/android/java/src/org/chromium/chrome/browser/vr/VrShellDelegate.java

@@ -42,6 +42,7 @@ import org.chromium.base.ContextUtils;
 import org.chromium.base.Log;
 import org.chromium.base.PackageUtils;
 import org.chromium.base.ThreadUtils;
+import org.chromium.base.VisibleForTesting;
 import org.chromium.base.annotations.CalledByNative;
 import org.chromium.base.annotations.JNINamespace;
 import org.chromium.base.library_loader.LibraryLoader;
@@ -206,6 +207,7 @@ public class VrShellDelegate
 
     private static final List<VrModeObserver> sVrModeObservers = new ArrayList<>();
 
+    @VisibleForTesting
     protected boolean mTestWorkaroundDontCancelVrEntryOnResume;
 
     private long mNativeVrShellDelegate;
@@ -906,7 +908,7 @@ public class VrShellDelegate
     }
 
     /**
-     *  @return Whether or not VR is supported on this platform.
+     *  @return Whether or not VR is enabled in this build.
      */
     /* package */ static boolean isVrEnabled() {
         return getVrClassesWrapper() != null;
@@ -1042,8 +1044,8 @@ public class VrShellDelegate
         // If an activity isn't resumed at the point, it must have been paused.
         mPaused = ApplicationStatus.getStateForActivity(activity) != ActivityState.RESUMED;
         mVisible = activity.hasWindowFocus();
-        updateVrSupportLevel(null);
         mNativeVrShellDelegate = nativeInit();
+        updateVrSupportLevel(null);
         mFeedbackFrequency = VrFeedbackStatus.getFeedbackFrequency();
         ensureLifecycleObserverInitialized();
         if (!mPaused) onResume();
@@ -1159,6 +1161,7 @@ public class VrShellDelegate
                 buttonText, null, true);
     }
 
+    @VisibleForTesting
     protected boolean isVrBrowsingEnabled() {
         return isVrBrowsingEnabled(mActivity, getVrSupportLevel());
     }
@@ -1215,16 +1218,11 @@ public class VrShellDelegate
     }
 
     private void enterVr(final boolean tentativeWebVrMode) {
-        // We can't enter VR before the application resumes, or we encounter bizarre crashes
-        // related to gpu surfaces.
-        // TODO(mthiesse): Is the above comment still accurate? It may have been tied to our HTML
-        // UI which is gone.
+        // We should only enter VR when we're the resumed Activity or our changes to things like
+        // system UI flags might get lost.
         assert !mPaused;
+        assert mNativeVrShellDelegate != 0;
         if (mInVr) return;
-        if (mNativeVrShellDelegate == 0) {
-            cancelPendingVrEntry();
-            return;
-        }
         mInVr = true;
         setVrModeEnabled(mActivity, true);
 
@@ -1244,11 +1242,11 @@ public class VrShellDelegate
         mExitedDueToUnsupportedMode = false;
 
         addVrViews();
-        boolean webVrMode = mRequestedWebVr || tentativeWebVrMode;
         // Make sure that assets component is registered when creating native VR shell.
         if (!sRegisteredVrAssetsComponent) {
             registerVrAssetsComponentIfDaydreamUser(isDaydreamCurrentViewer());
         }
+        boolean webVrMode = mRequestedWebVr || tentativeWebVrMode;
         mVrShell.initializeNative(webVrMode, getVrClassesWrapper().bootsToVr());
         mVrShell.setWebVrModeEnabled(webVrMode);
 
@@ -1290,6 +1288,9 @@ public class VrShellDelegate
         // Nothing to do if we were launched by an internal intent.
         if (mInternalIntentUsedToStartVr) {
             mInternalIntentUsedToStartVr = false;
+            // This is extremely unlikely in practice. Some code must have called shutdownVR() while
+            // we were entering VR through NFC insertion.
+            if (!mDonSucceeded) cancelPendingVrEntry();
             return;
         }
 
@@ -1544,6 +1545,7 @@ public class VrShellDelegate
         return true;
     }
 
+    @VisibleForTesting
     protected void onResume() {
         if (DEBUG_LOGS) Log.i(TAG, "onResume");
         if (Build.VERSION.SDK_INT < Build.VERSION_CODES.JELLY_BEAN_MR1) return;
@@ -1664,7 +1666,7 @@ public class VrShellDelegate
         if (mInVr) mVrShell.pause();
     }
 
-    protected void onPause() {
+    private void onPause() {
         if (DEBUG_LOGS) Log.i(TAG, "onPause");
         mPaused = true;
         if (mCancellingEntryAnimation) return;
@@ -1755,13 +1757,9 @@ public class VrShellDelegate
 
         mShowingDaydreamDoff = false;
 
-        if (mShowingDoffForGvrUpdate) {
-            mShowVrServicesUpdatePrompt = success;
-        }
+        if (mShowingDoffForGvrUpdate) mShowVrServicesUpdatePrompt = success;
 
-        if (success) {
-            shutdownVr(true /* disableVrMode */, true /* stayingInChrome */);
-        }
+        if (success) shutdownVr(true /* disableVrMode */, true /* stayingInChrome */);
 
         callOnExitVrRequestListener(success);
         mShowingDoffForGvrUpdate = false;
@@ -1778,8 +1776,7 @@ public class VrShellDelegate
     @CalledByNative
     private void setListeningForWebVrActivate(boolean listening) {
         if (DEBUG_LOGS) Log.i(TAG, "WebVR page listening for vrdisplayactivate: " + listening);
-        // Non-Daydream devices may not have the concept of display activate. So disable
-        // mListeningForWebVrActivate for them.
+        // Non-Daydream devices do not have the concept of activation.
         if (getVrSupportLevel() != VrSupportLevel.VR_DAYDREAM) return;
         if (mListeningForWebVrActivate == listening) return;
         mListeningForWebVrActivate = listening;
@@ -1824,7 +1821,7 @@ public class VrShellDelegate
     /**
      * Exits VR Shell, performing all necessary cleanup.
      */
-    protected void shutdownVr(boolean disableVrMode, boolean stayingInChrome) {
+    private void shutdownVr(boolean disableVrMode, boolean stayingInChrome) {
         if (DEBUG_LOGS) Log.i(TAG, "shuttdown VR");
         cancelPendingVrEntry();
 
@@ -1941,11 +1938,13 @@ public class VrShellDelegate
                 GVR_KEYBOARD_UPDATE_RESULT);
     }
 
+    @VisibleForTesting
     protected boolean canLaunch2DIntentsInternal() {
         return getVrClassesWrapper().supports2dInVr()
                 && !sVrModeEnabledActivitys.contains(sInstance.mActivity);
     }
 
+    @VisibleForTesting
     protected boolean createVrShell() {
         assert mVrShell == null;
         if (getVrClassesWrapper() == null) return false;
@@ -1972,6 +1971,7 @@ public class VrShellDelegate
         mActivity.onEnterVr();
     }
 
+    @VisibleForTesting
     protected boolean isBlackOverlayVisible() {
         View overlay = mActivity.getWindow().findViewById(R.id.vr_overlay_view);
         return overlay != null;
@@ -1997,6 +1997,7 @@ public class VrShellDelegate
     /**
      * @param api The VrDaydreamApi object this delegate will use instead of the default one
      */
+    @VisibleForTesting
     protected void overrideDaydreamApi(VrDaydreamApi api) {
         sVrDaydreamApi = api;
     }
@@ -2004,6 +2005,7 @@ public class VrShellDelegate
     /**
      * @return The VrShell for the VrShellDelegate instance
      */
+    @VisibleForTesting
     protected VrShell getVrShell() {
         return mVrShell;
     }
@@ -2011,6 +2013,7 @@ public class VrShellDelegate
     /**
      * @param versionChecker The VrCoreVersionChecker object this delegate will use
      */
+    @VisibleForTesting
     protected void overrideVrCoreVersionChecker(VrCoreVersionChecker versionChecker) {
         sVrCoreVersionChecker = versionChecker;
         updateVrSupportLevel(null);
@@ -2019,24 +2022,30 @@ public class VrShellDelegate
     /**
      * @param frequency Sets how often to show the feedback prompt.
      */
+    @VisibleForTesting
     protected void setFeedbackFrequency(int frequency) {
         mFeedbackFrequency = frequency;
     }
 
+    @VisibleForTesting
     protected boolean isListeningForWebVrActivate() {
         return mListeningForWebVrActivate;
     }
 
+    @VisibleForTesting
     protected boolean isVrEntryComplete() {
         return mInVr && !mProbablyInDon;
     }
 
+    @VisibleForTesting
     protected boolean isShowingDoff() {
         return mShowingDaydreamDoff;
     }
 
+    @VisibleForTesting
     protected void onBroadcastReceived() {}
 
+    @VisibleForTesting
     protected void setExpectingIntent(boolean expectingIntent) {}
 
     /**

chrome/android/java/src/org/chromium/chrome/browser/vr/VrToast.java

@@ -12,7 +12,6 @@ import android.widget.TextView;
 /**
  * This class implements a Toast which is similar to Android Toast in VR.
  */
-
 public class VrToast extends android.widget.Toast {
     private VrToastManager mVrToastManager;
 

chrome/android/java/src/org/chromium/chrome/browser/vr/VrUiWidgetFactory.java

@@ -36,7 +36,7 @@ public class VrUiWidgetFactory extends UiWidgetFactory {
 
     @Override
     public AlertDialog createAlertDialog(Context context) {
-        return new VrAlertDialog(context, mVrShell, mModalDialogManager);
+        return new VrAlertDialog(context, mModalDialogManager);
     }
 
     @Override

chrome/android/javatests/src/org/chromium/chrome/browser/vr/TestVrShellDelegate.java

@@ -48,11 +48,6 @@ public class TestVrShellDelegate extends VrShellDelegate {
         return TestVrShellDelegate.getInstance().getVrShell();
     }
 
-    @Override
-    public void shutdownVr(boolean disableVrMode, boolean stayingInChrome) {
-        super.shutdownVr(disableVrMode, stayingInChrome);
-    }
-
     public void overrideDaydreamApiForTesting(VrDaydreamApi api) {
         super.overrideDaydreamApi(api);
     }

chrome/android/javatests/src/org/chromium/chrome/browser/vr/util/TransitionUtils.java

@@ -69,8 +69,7 @@ public class TransitionUtils {
         ThreadUtils.runOnUiThreadBlocking(new Runnable() {
             @Override
             public void run() {
-                VrShellDelegateUtils.getDelegateInstance().shutdownVr(
-                        true /* disableVrMode */, true /* stayingInChrome */);
+                VrShellDelegate.forceExitVrImmediately();
             }
         });
     }