Switch WebVR to handle GvrApi management through VrShellDelegate

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

 // Copyright 2016 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.vr_shell;
 
 import android.app.Activity;
 import android.content.Intent;
 import android.content.pm.ActivityInfo;
 import android.os.StrictMode;
 import android.view.View;
 import android.view.ViewGroup;
 import android.view.ViewGroup.LayoutParams;
 import android.view.WindowManager;
 import android.widget.FrameLayout;
 
 import org.chromium.base.Log;
+import org.chromium.base.annotations.CalledByNative;
+import org.chromium.base.annotations.JNINamespace;
 import org.chromium.chrome.browser.ChromeTabbedActivity;
 import org.chromium.chrome.browser.tab.Tab;
 
 import java.lang.reflect.Constructor;
 import java.lang.reflect.InvocationTargetException;
 
 /**
  * Manages interactions with the VR Shell.
  */
+@JNINamespace("vr_shell")
 public class VrShellDelegate {
     private static final String TAG = "VrShellDelegate";
 
     private ChromeTabbedActivity mActivity;
 
     private boolean mVrShellEnabled;
 
     private Class<? extends VrShellInterface> mVrShellClass;
     private VrShellInterface mVrShell;
     private boolean mInVr;
     private int mRestoreSystemUiVisibilityFlag = -1;
     private String mVrExtra;
+    private long mNativeVrShellDelegate;
 
     public VrShellDelegate(ChromeTabbedActivity activity) {
         mActivity = activity;
 
         mVrShellClass = maybeFindVrShell();
         if (mVrShellClass != null) {
             mVrShellEnabled = true;
             try {
                 mVrExtra = (String) mVrShellClass.getField("VR_EXTRA").get(null);
             } catch (IllegalAccessException | IllegalArgumentException | NoSuchFieldException e) {
                 Log.e(TAG, "Unable to read VR_EXTRA field", e);
                 mVrShellEnabled = false;
             }
         }
     }
 
+    /**
+     * Should be called once the native library is loaded so that the native portion of this
+     * class can be initialized.
+     */
+    public void onNativeLibraryReady() {
+        mNativeVrShellDelegate = nativeInit();

[Ted C, 2016/09/21 17:59:24]

do you need a check for mVrShellEnabled here?  otherwise, I believe you'll be
attempting to initialize native code that won't exist (when the build flag is
off).

+    }
+
     @SuppressWarnings("unchecked")
     private Class<? extends VrShellInterface> maybeFindVrShell() {
         try {
             return (Class<? extends VrShellInterface>) Class
                     .forName("org.chromium.chrome.browser.vr_shell.VrShell");
         } catch (ClassNotFoundException e) {
             return null;
         }
     }
 
     /**
      * Enters VR Shell, displaying browser UI and tab contents in VR.
      *
      * This function performs native initialization, and so must only be called after native
      * libraries are ready.
      * @Returns Whether or not we are in VR when this function returns.

[Ted C, 2016/09/21 17:59:24]

We should add an @param for inWebVR here

Also if we are here, can you change this to @return instead of @Returns

      */
-    public boolean enterVRIfNecessary() {
-        if (!mVrShellEnabled) return false;
+    @CalledByNative
+    public boolean enterVRIfNecessary(boolean inWebVR) {
+        if (!mVrShellEnabled || mNativeVrShellDelegate == 0) return false;
         Tab tab = mActivity.getActivityTab();
         // TODO(mthiesse): When we have VR UI for opening new tabs, etc., allow VR Shell to be
         // entered without any current tabs.
         if (tab == null || tab.getContentViewCore() == null) {
             return false;
         }
         if (mInVr) return true;
         // VrShell must be initialized in Landscape mode due to a bug in the GVR library.
         mActivity.setRequestedOrientation(ActivityInfo.SCREEN_ORIENTATION_LANDSCAPE);
         if (!createVrShell()) {
             mActivity.setRequestedOrientation(ActivityInfo.SCREEN_ORIENTATION_UNSPECIFIED);
             return false;
         }
         addVrViews();
         setupVrModeWindowFlags();
-        mVrShell.onNativeLibraryReady(tab);
+        mVrShell.onNativeLibraryReady(tab, this);

[Ted C, 2016/09/21 17:59:25]

to me, it's weird to have this not in the onNativeLibraryReady call above.  Do
these mean different things now?  Should this method be called initializeForTab
or "something"?

[bajones, 2016/09/21 19:16:25]

While "something" seems like a great function name, after consulting with the
other VrShell devlopers I went with "initializeNative" instead. ;)

+        if (inWebVR) {

[Ted C, 2016/09/21 17:59:25]

in java land (if it is readable), you can put the condition and statement on a
single line if it all fits

+            mVrShell.setWebVrModeEnabled(true);
+        }
         mVrShell.setVrModeEnabled(true);
         mInVr = true;
         return true;
     }
 
+    @CalledByNative
+    public boolean exitWebVR() {

[Ted C, 2016/09/21 17:59:25]

does this only get accessed by native?  If so, you can make it private.

+        if (!mInVr) return false;
+        mVrShell.setWebVrModeEnabled(false);

[Ted C, 2016/09/21 17:59:25]

Could we just move this line into shutdownVR (is it bad to call in non-webVR
cases)?  Then we could expose that via @CalledByNative and we could get rid of
the need for this method?

Ahh...based on the comment below it looks like exiting WebVR won't be calling
shutdown long term, so maybe my question above isn't right.

[bajones, 2016/09/21 19:16:25]

I think the comment below illustrates the reason for this. In future CLs we will
want to exit from WebVR back to the VrShell rather than exiting VR entirely. As
such, we want to disambiguate whether we're only exiting the WebVR content or VR
as a whole.

+        // TODO(bajones): Once VR Shell can be invoked outside of WebVR this
+        // should no longer exit the shell outright. Need a way to determine
+        // how VrShell was created.
+        shutdownVR();
+        return true;
+    }
+
     /**
      * Resumes VR Shell.
      */
     public void resumeVR() {
         setupVrModeWindowFlags();
         mVrShell.resume();
     }
 
     /**
      * Pauses VR Shell.
      */
     public void pauseVR() {
         mVrShell.pause();
     }
 
     /**
+     * Exits the current VR mode (WebVR or VRShell)
+     * @Returns Whether or not we exited VR.

[Ted C, 2016/09/21 17:59:25]

@return

+     */
+    public boolean exitVRIfNecessary() {
+        if (!mInVr) return false;
+        // If WebVR is presenting instruct it to exit. VR mode should not
+        // exit in this scenario, in case we want to return to the VrShell.
+        if (!nativeExitWebVRIfNecessary(mNativeVrShellDelegate)) {
+            // If WebVR was not presenting, shutdown VR mode entirely.
+            shutdownVR();
+        }
+
+        return true;
+    }
+
+    /**
      * Exits VR Shell, performing all necessary cleanup.
-     * @Returns Whether or not we exited VR.
      */
-    public boolean exitVRIfNecessary() {
-        if (!mInVr) return false;
+    private void shutdownVR() {
+        if (!mInVr) return;
         mActivity.setRequestedOrientation(ActivityInfo.SCREEN_ORIENTATION_UNSPECIFIED);
         mVrShell.setVrModeEnabled(false);
         mVrShell.pause();
         removeVrViews();
         clearVrModeWindowFlags();
         destroyVrShell();
         mInVr = false;
-        return true;
     }
 
     private boolean createVrShell() {
         StrictMode.ThreadPolicy oldPolicy = StrictMode.allowThreadDiskReads();
         try {
             Constructor<?> vrShellConstructor = mVrShellClass.getConstructor(Activity.class);
             mVrShell = (VrShellInterface) vrShellConstructor.newInstance(mActivity);
         } catch (InstantiationException | IllegalAccessException | IllegalArgumentException
                 | InvocationTargetException | NoSuchMethodException e) {
             Log.e(TAG, "Unable to instantiate VrShell", e);
@@ skipping 64 matching lines @@
     public boolean isInVR() {
         return mInVr;
     }
 
     /**
      * @return Whether or not VR Shell is currently enabled.
      */
     public boolean isVrShellEnabled() {
         return mVrShellEnabled;
     }
+
+    /**
+     * @return Pointer to the native VrShellDelegate object.
+     */
+    @CalledByNative
+    public long getNativePointer() {

[Ted C, 2016/09/21 17:59:24]

should be private

+        return mNativeVrShellDelegate;
+    }
+
+    private native long nativeInit();
+    private native boolean nativeExitWebVRIfNecessary(long nativeVrShellDelegate);
 }