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

This reverts commit 0062abf0.

Reason for revert: Fixing...

Original change's description:
> Revert "VR: Ensure we remove black overlay if VR is exited while entering."
>
> This reverts commit 9f8158c1.
>
> Reason for revert: Culprit of failed VR tests https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Nougat%20Phone%20Tester/6884
>
> Original change's description:
> > 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/1138726
> > Reviewed-by: Tibor Goldschwendt <tiborg@chromium.org>
> > Commit-Queue: Michael Thiessen <mthiesse@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#575388}
>
> TBR=mthiesse@chromium.org,tiborg@chromium.org
>
> Change-Id: Ibe67ee8c09dc44ac34d40c3eaf890884b400fc24
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug: 862829
> Reviewed-on: https://chromium-review.googlesource.com/1140813
> Reviewed-by: Brian Sheedy <bsheedy@chromium.org>
> Commit-Queue: Brian Sheedy <bsheedy@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#575725}

Change-Id: I71738aafff0b703083f465085fb71f54d047729b
Bug: 862829
Reviewed-on: https://chromium-review.googlesource.com/1142167Reviewed-by: Brian Sheedy <bsheedy@chromium.org>
Commit-Queue: Michael Thiessen <mthiesse@chromium.org>
Cr-Commit-Position: refs/heads/master@{#576173}

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;
@@ -202,6 +203,7 @@ public class VrShellDelegate
     // Gets run when the user exits VR mode by clicking the Gear button.
     private Runnable mSettingsButtonListener;
 
+    @VisibleForTesting
     protected boolean mTestWorkaroundDontCancelVrEntryOnResume;
 
     private long mNativeVrShellDelegate;
@@ -869,7 +871,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;
@@ -1005,8 +1007,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();
@@ -1122,6 +1124,7 @@ public class VrShellDelegate
                 buttonText, null, true);
     }
 
+    @VisibleForTesting
     protected boolean isVrBrowsingEnabled() {
         return isVrBrowsingEnabled(mActivity, getVrSupportLevel());
     }
@@ -1178,16 +1181,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);
 
@@ -1207,11 +1205,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);
 
@@ -1253,6 +1251,14 @@ public class VrShellDelegate
         // Nothing to do if we were launched by an internal intent.
         if (mInternalIntentUsedToStartVr) {
             mInternalIntentUsedToStartVr = false;
+
+            // TODO(mthiesse): This shouldn't be necessary. This is another instance of b/65681875,
+            // where the intent is received after we're resumed.
+            if (mInVr) return;
+
+            // This is extremely unlikely in practice. Some code must have called shutdownVR() while
+            // we were entering VR through NFC insertion.
+            if (!mDonSucceeded) cancelPendingVrEntry();
             return;
         }
 
@@ -1263,14 +1269,12 @@ public class VrShellDelegate
             return;
         }
 
-        if (mInVr) return;
+        if (!mInVr) nativeRecordVrStartAction(mNativeVrShellDelegate, VrStartAction.INTENT_LAUNCH);
 
         mStartedFromVrIntent = true;
         // Setting DON succeeded will cause us to enter VR when resuming.
         mDonSucceeded = true;
 
-        nativeRecordVrStartAction(mNativeVrShellDelegate, VrStartAction.INTENT_LAUNCH);
-
         if (!mPaused) {
             // Note that canceling the animation below is what causes us to enter VR mode. We start
             // an intermediate activity to cancel the animation which causes onPause and onResume to
@@ -1507,6 +1511,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;
@@ -1627,7 +1632,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;
@@ -1718,13 +1723,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;
@@ -1741,8 +1742,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;
@@ -1787,7 +1787,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();
 
@@ -1904,11 +1904,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;
@@ -1935,6 +1937,7 @@ public class VrShellDelegate
         mActivity.onEnterVr();
     }
 
+    @VisibleForTesting
     protected boolean isBlackOverlayVisible() {
         View overlay = mActivity.getWindow().findViewById(R.id.vr_overlay_view);
         return overlay != null;
@@ -1960,6 +1963,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;
     }
@@ -1967,6 +1971,7 @@ public class VrShellDelegate
     /**
      * @return The VrShell for the VrShellDelegate instance
      */
+    @VisibleForTesting
     protected VrShell getVrShell() {
         return mVrShell;
     }
@@ -1974,6 +1979,7 @@ public class VrShellDelegate
     /**
      * @param versionChecker The VrCoreVersionChecker object this delegate will use
      */
+    @VisibleForTesting
     protected void overrideVrCoreVersionChecker(VrCoreVersionChecker versionChecker) {
         sVrCoreVersionChecker = versionChecker;
         updateVrSupportLevel(null);
@@ -1982,24 +1988,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);
     }