Refactor Vr activity into ChromeTabbedActivity.

chrome/android/java/src/org/chromium/chrome/browser/ChromeTabbedActivity.java

Index: chrome/android/java/src/org/chromium/chrome/browser/ChromeTabbedActivity.java
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/ChromeTabbedActivity.java b/chrome/android/java/src/org/chromium/chrome/browser/ChromeTabbedActivity.java
index e4516704b844dbf988ed4a45b330130a1d1772ca..86fcebb128dbd0623c1b7eb71eb6488e8b9d43d0 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/ChromeTabbedActivity.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/ChromeTabbedActivity.java
@@ -9,10 +9,13 @@ import android.app.Activity;
 import android.app.ActivityManager;
 import android.content.Context;
 import android.content.Intent;
+import android.content.pm.ActivityInfo;
 import android.graphics.Color;
+import android.graphics.PixelFormat;
 import android.net.Uri;
 import android.os.Build;
 import android.os.Bundle;
+import android.os.StrictMode;
 import android.os.SystemClock;
 import android.provider.Browser;
 import android.support.annotation.IntDef;
@@ -95,6 +98,7 @@ import org.chromium.chrome.browser.tabmodel.TabWindowManager;
 import org.chromium.chrome.browser.toolbar.ToolbarControlContainer;
 import org.chromium.chrome.browser.util.FeatureUtilities;
 import org.chromium.chrome.browser.util.IntentUtils;
+import org.chromium.chrome.browser.vr_shell.VrShellInterface;
 import org.chromium.chrome.browser.widget.emptybackground.EmptyBackgroundViewWrapper;
 import org.chromium.chrome.browser.widget.findinpage.FindToolbarManager;
 import org.chromium.content.browser.ContentVideoView;
@@ -109,6 +113,7 @@ import org.chromium.ui.widget.Toast;
 import java.lang.annotation.Retention;
 import java.lang.annotation.RetentionPolicy;
 import java.lang.ref.WeakReference;
+import java.lang.reflect.Constructor;
 import java.util.List;
 
 /**
@@ -215,6 +220,12 @@ public class ChromeTabbedActivity extends ChromeActivity implements OverviewMode
     // Time at which an intent was received and handled.
     private long mIntentHandlingTimeMs = 0;
 
+    Class<?> mVrShellClass = null;

[Ted C, 2016/09/01 22:38:37]

should probably be private, also should this be "? extends VrShellInterface"

null and false are defaults, so we let's not set them (this class doesn't really
do a good job with that though).

[mthiesse, 2016/09/02 01:05:33]

Done.

+    private VrShellInterface mVrShellView = null;
+    private FrameLayout mVrShellFrameLayout = null;
+    private boolean mInVr = false;
+    private int mRestoreSystemUiVisibilityFlag = -1;
+
     private class TabbedAssistStatusHandler extends AssistStatusHandler {
         public TabbedAssistStatusHandler(Activity activity) {
             super(activity);
@@ -367,12 +378,15 @@ public class ChromeTabbedActivity extends ChromeActivity implements OverviewMode
             }
             mMergeTabsOnResume = false;
         }
+        if (mInVr) resumeVR();
+        if (isVrIntent(getIntent())) enterVR();

[bsheedy, 2016/09/01 20:24:03]

Should this be an 'else if'? enterVR() returns immediately if mInVr is true, but
this could prevent an unnecessary function call.

[mthiesse, 2016/09/02 01:05:33]

Done.

     }
 
     @Override
     public void onPauseWithNative() {
         mTabModelSelectorImpl.commitAllTabClosures();
         CookiesFetcher.persistCookies(this);
+        if (mInVr) pauseVR();
         super.onPauseWithNative();
     }
 
@@ -940,6 +954,8 @@ public class ChromeTabbedActivity extends ChromeActivity implements OverviewMode
 
         mUndoBarPopupController = new UndoBarController(this, mTabModelSelectorImpl,
                 getSnackbarManager());
+
+        maybeFindVrShell();

[Ted C, 2016/09/01 22:38:37]

this seems unnecessary here, I would just have this be a first time first use
sort of thing.

[mthiesse, 2016/09/02 01:05:33]

Done.

     }
 
     @Override
@@ -1155,6 +1171,11 @@ public class ChromeTabbedActivity extends ChromeActivity implements OverviewMode
         if (!mUIInitialized) return false;
         final Tab currentTab = getActivityTab();
 
+        if (mInVr) {
+            exitVR();
+            return true;
+        }
+
         if (currentTab == null) {
             recordBackPressedUma("currentTab is null", BACK_PRESSED_TAB_IS_NULL);
             moveTaskToBack(true);
@@ -1370,6 +1391,8 @@ public class ChromeTabbedActivity extends ChromeActivity implements OverviewMode
             mUndoBarPopupController = null;
         }
 
+        destroyVrShell();
+
         super.onDestroyInternal();
     }
 
@@ -1565,4 +1588,125 @@ public class ChromeTabbedActivity extends ChromeActivity implements OverviewMode
         RecordUserAction.record("Android.MergeState.Live");
         mTabModelSelectorImpl.mergeState();
     }
+
+    // VR shell related code.
+    @Override
+    public boolean isVrShellEnabled() {

[Ted C, 2016/09/01 22:38:37]

I'd vote for doing a tri-state boolean like accessibility mode (capital B
Boolean).

if (mVrShellEnabled != null) return mVrShellEnabled;
mVrShellEnabled = maybeFindVrShell() != null;
return mVrShellEnabled;

Doing reflection every time for something that will always either be true or
false seems bad to me.

[mthiesse, 2016/09/02 01:05:33]

Done.

+        if (mVrShellClass == null) {
+            maybeFindVrShell();
+        }
+        return mVrShellClass != null;

[Ted C, 2016/09/01 22:38:37]

looking at this a bit more, I think we should move all of this code into a
VrShellDelegate/VrShellHelper.

The code in this class seems like it could be very much smaller (pass
R.id.content as a parent container to make it a bit cleaner).

I think having this class just have to call some VrShellDelegate for
enter/exit/pause/resume would be nice

[mthiesse, 2016/09/02 01:05:33]

Done.

+    }
+
+    @Override
+    public void enterVR() {
+        if (mVrShellClass == null || mInVr) return;
+        mInVr = true;
+        setRequestedOrientation(ActivityInfo.SCREEN_ORIENTATION_LANDSCAPE);
+        createVrShell();
+        addVrViews();
+        setupVrModeWindowFlags();
+        mVrShellView.onNativeLibraryReady();
+        mVrShellView.setVrModeEnabled(true);
+    }
+
+    /**
+     * Resumes VR Shell.
+     */
+    public void resumeVR() {
+        setupVrModeWindowFlags();
+        mVrShellView.resume();
+    }
+
+    /**
+     * Pauses VR Shell.
+     */
+    public void pauseVR() {
+        mVrShellView.pause();
+    }
+
+    /**
+     * Exits Vr Shell, performing all necessary cleanup.
+     */
+    public void exitVR() {
+        if (!mInVr) return;
+        mInVr = false;
+        setRequestedOrientation(ActivityInfo.SCREEN_ORIENTATION_UNSPECIFIED);
+        mVrShellView.setVrModeEnabled(false);
+        mVrShellView.pause();
+        removeVrViews();
+        clearVrModeWindowFlags();
+        destroyVrShell();
+    }
+
+    private void maybeFindVrShell() {
+        try {
+            mVrShellClass = Class.forName("org.chromium.chrome.browser.vr_shell.VrShell");
+        } catch (ClassNotFoundException e) {
+            mVrShellClass = null;
+        }
+    }
+
+    private void createVrShell() {
+        StrictMode.ThreadPolicy oldPolicy = StrictMode.allowThreadDiskReads();
+        try {
+            Constructor<?> vrShellConstructor = mVrShellClass.getConstructor(Activity.class);
+            mVrShellView = (VrShellInterface) vrShellConstructor.newInstance(this);
+        } catch (Exception ex) {

[Ted C, 2016/09/01 22:38:37]

don't catch generic exception, catch the specific ones.  And you can | the
exception types together to avoid a bunch of catches.

[mthiesse, 2016/09/02 01:05:33]

Done.

+            ex.printStackTrace();

[Ted C, 2016/09/01 22:38:37]

Use chrome logging for this instead of just bare stack trace printing.

[mthiesse, 2016/09/02 01:05:33]

Done.

+        }
+        mVrShellFrameLayout = (FrameLayout) mVrShellView;

[Ted C, 2016/09/01 22:38:37]

Put this in the body of the try (casting null won't NPE, but this just reads
oddly to me).

Also, instead of having to cast to both a VrShellInterface and FrameLayout, I
think it would be cleaner to expose:

FrameLayout getContainer();

on VrShellInterface, and call that (and that method would just look like "return
this;").

[mthiesse, 2016/09/02 01:05:33]

Done.

+        StrictMode.setThreadPolicy(oldPolicy);

[Ted C, 2016/09/01 22:38:37]

put this in a finally in your try catch block.

[mthiesse, 2016/09/02 01:05:33]

Done.

+    }
+
+    private void addVrViews() {
+        WindowManager.LayoutParams params = new WindowManager.LayoutParams(

[Ted C, 2016/09/01 22:38:37]

Hmm...do you actually need a WindowManager.LayoutParams?

android.R.id.content is a FrameLayout, and I wouldn't think that it would use
anything defined here except for width and height.

Wondering if we should just use a vanilla ViewGroup.LayoutParams or
ViewGroup.MarginLayoutParams.

[mthiesse, 2016/09/02 01:05:33]

Done.

+                WindowManager.LayoutParams.MATCH_PARENT, WindowManager.LayoutParams.MATCH_PARENT,
+                WindowManager.LayoutParams.TYPE_APPLICATION, 0, PixelFormat.OPAQUE);
+        FrameLayout content = (FrameLayout) findViewById(android.R.id.content);
+        content.addView(mVrShellFrameLayout, content.getChildCount(), params);
+    }
+
+    private void removeVrViews() {
+        ((ViewGroup) mVrShellFrameLayout.getParent()).removeView(mVrShellFrameLayout);
+    }
+
+    private void setupVrModeWindowFlags() {
+        if (mRestoreSystemUiVisibilityFlag == -1) {
+            mRestoreSystemUiVisibilityFlag = getWindow().getDecorView().getSystemUiVisibility();
+        }
+        getWindow().addFlags(WindowManager.LayoutParams.FLAG_KEEP_SCREEN_ON);
+        getWindow().getDecorView().setSystemUiVisibility(View.SYSTEM_UI_FLAG_LAYOUT_STABLE
+                | View.SYSTEM_UI_FLAG_LAYOUT_HIDE_NAVIGATION
+                | View.SYSTEM_UI_FLAG_LAYOUT_FULLSCREEN | View.SYSTEM_UI_FLAG_HIDE_NAVIGATION
+                | View.SYSTEM_UI_FLAG_FULLSCREEN | View.SYSTEM_UI_FLAG_IMMERSIVE_STICKY);
+    }
+
+    private void clearVrModeWindowFlags() {
+        getWindow().clearFlags(WindowManager.LayoutParams.FLAG_KEEP_SCREEN_ON);
+        if (mRestoreSystemUiVisibilityFlag != -1) {
+            getWindow().getDecorView().setSystemUiVisibility(mRestoreSystemUiVisibilityFlag);
+        }
+        mRestoreSystemUiVisibilityFlag = -1;
+    }
+
+    private void destroyVrShell() {
+        if (mVrShellView != null) {
+            mVrShellView.teardown();
+            mVrShellView = null;
+        }
+    }
+
+    private boolean isVrIntent(Intent intent) {
+        boolean enterVR = false;
+        if (mVrShellClass != null && intent != null) {
+            try {
+                String vrExtra = (String) mVrShellClass.getField("VR_EXTRA").get(null);

[Ted C, 2016/09/01 22:38:37]

cache this value as well.  we wouldn't want to introduce reflection for each
intent call

[mthiesse, 2016/09/02 01:05:33]

Done.

+                enterVR = intent.getBooleanExtra(vrExtra, false);
+            } catch (IllegalAccessException | IllegalArgumentException | NoSuchFieldException e) {
+                e.printStackTrace();

[Ted C, 2016/09/01 22:38:37]

same comment about using chrome logging

[mthiesse, 2016/09/02 01:05:33]

Done.

+            }
+        }
+        return enterVR;
+    }
 }