Intercept WebVR api calls and check for VrCore compatibility.

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.app.PendingIntent;
 import android.content.ComponentName;
 import android.content.Context;
@@ skipping 141 matching lines @@
                     mVrDaydreamApi.createVrIntent(new ComponentName(mActivity, VR_ACTIVITY_ALIAS));
         }
         mVrSupportLevel = mVrDaydreamApi.isDaydreamReadyDevice() ? VR_DAYDREAM : VR_CARDBOARD;
     }
 
     /**
      * 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();
         updateVrSupportLevel();
         if (mVrSupportLevel == VR_NOT_AVAILABLE) return;

[mthiesse, 2017/02/21 15:56:18]

What if we added a new VrSupportLevel for VR_DAYDREAM_NEEDS_UPDATE, so that
don't create the native VrShellDelegate when it's not needed.

[amp, 2017/02/21 19:50:03]

Aside from limiting the creation of thenative VrShellDelegate, that's kind of
what I did in https://codereview.chromium.org/2699163003/

I could update that to update the overall vr support level, but I don't think we
want the general vr support level to be sdk specific.

[mthiesse, 2017/02/21 20:23:00]

Well one concern with this change is that now we're creating the native
VrShellDelegate on platforms where daydream support isn't currently possible (or
even planned to be possible). Do we have a way to check that VR support is even
possible before we initialize more VR code than is necessary?

Maybe I'm crazy to worry about this at all, but it is unnecessary memory usage
on lower-end devices.

[amp, 2017/02/21 22:15:09]

I don't think we do, at least not that I'm aware of.
I agree that is one of the main concerns of this change.  The current infobar
implementation is triggered through the Java side, so if we want to use the
current implementation we would have to somehow get the native WebVR API calls
to call back up to the Java side.

The problem is that if VrCore is not up to date (or not installed), the native
vr delegate isn't created and then there is no link back to the java side to
create the infobar asking the user to install it or update it.

We could try to switch to a native infobar implementation (which I'm not sure
exactly how it works, but it would have it's own java integrations for actually
displaying the visuals correctly) which wouldn't require a native side
integration in our vr shell code.

It would require wiring the WebVR API up to 'something', though, and we could
have that be a simpler (in theory smaller memory footprint) listener of the
WebVR API that could figure out if VR support is possible and whether to notify
the user about it or not.

[ddorwin, 2017/02/23 23:23:32]

I think something like `VR_[SDK_]NEEDS_UPDATE` would address the need without
being platform-specific. This will help us separate no support (J) from needs an
install/update (K+).

Does that give us enough information to disambiguate the install/update, UI,
though?

Regarding the memory usage issue:
* We are already doing this for a subset of (usually high-end) devices
regardless of whether VR will be used. We should fix this.
* We should avoid doing this on the majority of devices.

The issue is described in
https://bugs.chromium.org/p/chromium/issues/detail?id=655297#c17. mthiesse is
going to work on that. I think we should try to drive this CL to a state where
it's ready to land on top of his fix. Hopefully, mthiesse can provide guidance
on what that will look like.

[amp, 2017/02/24 01:20:01]

I think we can update the VR_SUPPORT_LEVEL to include a NEEDS_UPDATE level that
can separate out devices which will never have support.  Then when we actually
show the prompt the VrCoreCompatibility checker will differentiate between
update vs install.
That sounds great.  I was trying to look at how to lazy initialize things, but
it was a little much for me to wrap my head around.  I'll sync up with Michael
about what that will look like and build this change on top of it.

-        mNativeVrShellDelegate = nativeInit();
         Choreographer choreographer = Choreographer.getInstance();
         choreographer.postFrameCallback(new FrameCallback() {
             @Override
             public void doFrame(long frameTimeNanos) {
                 Display display = ((WindowManager) mActivity.getSystemService(
                         Context.WINDOW_SERVICE)).getDefaultDisplay();
                 nativeUpdateVSyncInterval(mNativeVrShellDelegate, frameTimeNanos,
                         1.0d / display.getRefreshRate());
             }
         });
@@ skipping 350 matching lines @@
         }
         if (mRestoreOrientation != null) mActivity.setRequestedOrientation(mRestoreOrientation);
         mRestoreOrientation = null;
         mVrShell.pause();
         removeVrViews();
         clearVrModeWindowFlags();
         destroyVrShell();
         mActivity.getFullscreenManager().setPositionsForTabToNonFullscreen();
     }
 
+    /**
+     * @return Whether VrCore exists and is up to date.
+     */
+    @CalledByNative

[mthiesse, 2017/02/21 15:56:18]

I think you would want to check for the support level being the new
VR_DAYDREAM_NEEDS_UPDATE instead of checking isVrCoreCompatible. If you use
isVrCoreCompatible(), then aren't you breaking cardboard support?

[amp, 2017/02/21 19:50:03]

I could wire it up to be a bit more specific.

I'm not sure this method of calling back on 'SetDeviceProvider' is a good idea
and perhaps we should just setup a WebVR API observer (which could then check
the VR support level and show the infobar as needed).

[mthiesse, 2017/02/21 20:23:00]

I don't follow what a WebVR API Observer would look like, or what function it
would serve. Checking on SetDeviceProvider seems fine. If support changes we
could then remove the device and destroy the delegate, rather than having an
observer API.

[amp, 2017/02/21 22:15:09]

See comment above on not initializing native vr delegate if possible.

If it is reasonable to assume that SetDeviceProdiver is always called before any
WebVR API (this doesn't intuitively make sense to me just from the names on the
methods) then the 'Observer' of the API could be a static vr support checker
called from the WebVR API side implementation, rather than adding full support
of generic observers which could be registered etc.

I'm not sure how the dependencies would work out, though, so we may still need a
generic observer pattern so the API impl code can call back into the browser
code (via registered observers).

[ddorwin, 2017/02/23 23:23:31]

My understanding of
https://bugs.chromium.org/p/chromium/issues/detail?id=655297#c17 is that we
would lazily invoke this code when it is needed, such as getVRDisplays().

An advantage of doing it here is that all VR-related code will benefit from the
prompts rather than having to add checks or observers to each VR-related API or
feature.

[amp, 2017/02/24 01:20:01]

Ok.  I'll wait for the underlying lazy init parts from Michael.  I agree that
doing it here once rather than for each API is cleaner in that regard and if the
dependencies work out then I'm fine with a more hard wired path like I have now.

I did change it from the 'isVrCoreCompatible' check to something that returns
the VrSupportLevel as mentioned in the other comment.

     private boolean isVrCoreCompatible() {
         assert mVrClassesWrapper != null;
         if (mVrCoreVersionChecker == null) {
             mVrCoreVersionChecker = mVrClassesWrapper.createVrCoreVersionChecker();
         }
 
         return verifyOrUpdateVrServices(
                 mVrCoreVersionChecker.isVrCoreCompatible(), mActivity.getActivityTab());
     }
 
@@ skipping 150 matching lines @@
     }
 
     private native long nativeInit();
     private native void nativeSetPresentResult(long nativeVrShellDelegate, boolean result);
     private native void nativeDisplayActivate(long nativeVrShellDelegate);
     private native void nativeUpdateVSyncInterval(long nativeVrShellDelegate, long timebaseNanos,
             double intervalSeconds);
     private native void nativeOnPause(long nativeVrShellDelegate);
     private native void nativeOnResume(long nativeVrShellDelegate);
 }