VR: Wire VrShell UI-related state to the scene manager.

chrome/browser/android/vr_shell/vr_shell.cc

 // 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.
 
 #include "chrome/browser/android/vr_shell/vr_shell.h"
 
 #include <android/native_window_jni.h>
 
 #include <string>
 #include <utility>
@@ skipping 34 matching lines @@
 #include "jni/VrShellImpl_jni.h"
 #include "third_party/WebKit/public/platform/WebInputEvent.h"
 #include "ui/android/view_android.h"
 #include "ui/android/window_android.h"
 #include "ui/base/page_transition_types.h"
 #include "ui/display/display.h"
 #include "ui/display/screen.h"
 #include "ui/gfx/codec/png_codec.h"
 #include "ui/gfx/geometry/size.h"
 #include "ui/gfx/native_widget_types.h"
+#include "url/gurl.h"
 
 using base::android::JavaParamRef;
 using base::android::JavaRef;
 
 namespace vr_shell {
 
 namespace {
 vr_shell::VrShell* g_instance;
 
 void SetIsInVR(content::WebContents* contents, bool is_in_vr) {
@@ skipping 30 matching lines @@
       gvr_api_(gvr_api),
       weak_ptr_factory_(this) {
   DVLOG(1) << __FUNCTION__ << "=" << this;
   DCHECK(g_instance == nullptr);
   g_instance = this;
   j_vr_shell_.Reset(env, obj);
 
   gl_thread_ = base::MakeUnique<VrGLThread>(weak_ptr_factory_.GetWeakPtr(),
                                             main_thread_task_runner_, gvr_api,
                                             for_web_vr, reprojected_rendering_);
+  ui_ = gl_thread_.get();
 
   base::Thread::Options options(base::MessageLoop::TYPE_DEFAULT, 0);
   options.priority = base::ThreadPriority::DISPLAY;
   gl_thread_->StartWithOptions(options);
 
-  ui_ = base::MakeUnique<UiInterface>(for_web_vr ? UiInterface::Mode::WEB_VR
-                                                 : UiInterface::Mode::STANDARD);
 
   content::BrowserThread::PostTask(
       content::BrowserThread::FILE, FROM_HERE,
       base::Bind(LoadControllerModelTask, weak_ptr_factory_.GetWeakPtr(),
                  main_thread_task_runner_));
 }
 
 void VrShell::Destroy(JNIEnv* env, const JavaParamRef<jobject>& obj) {
   delete this;
 }
@@ skipping 21 matching lines @@
     android_ui_gesture_target_ = base::MakeUnique<AndroidUiGestureTarget>(
         j_motion_event_synthesizer_.obj(),
         Java_VrShellImpl_getNativePageScrollRatio(env, j_vr_shell_.obj()));
     input_manager_ = nullptr;
     vr_web_contents_observer_ = nullptr;
     metrics_helper_ = nullptr;
     return;
   }
   input_manager_ = base::MakeUnique<VrInputManager>(web_contents_);
   vr_web_contents_observer_ =
-      base::MakeUnique<VrWebContentsObserver>(web_contents_, ui_.get(), this);
+      base::MakeUnique<VrWebContentsObserver>(web_contents_, ui_, this);
   // TODO(billorr): Make VrMetricsHelper tab-aware and able to track multiple
   // tabs. crbug.com/684661
   metrics_helper_ = base::MakeUnique<VrMetricsHelper>(web_contents_);
   metrics_helper_->SetVRActive(true);
   metrics_helper_->SetWebVREnabled(webvr_mode_);
 }
 
 void VrShell::SetUiState() {
   if (!web_contents_) {
     // TODO(mthiesse): Properly handle native page URLs.
     ui_->SetURL(GURL());
     ui_->SetLoading(false);
     ui_->SetFullscreen(false);
+    ui_->SetURL(GURL());
   } else {
     ui_->SetURL(web_contents_->GetVisibleURL());
     ui_->SetLoading(web_contents_->IsLoading());
     ui_->SetFullscreen(web_contents_->IsFullscreen());
   }
 }
 
 bool RegisterVrShell(JNIEnv* env) {
   return RegisterNativesImpl(env);
 }
@@ skipping 17 matching lines @@
     // that IO is allowed to prevent jank, but there shouldn't be any concerns
     // regarding jank in this case, because we're switching from 3D to 2D,
     // adding/removing a bunch of Java views, and probably changing device
     // orientation here.
     base::ThreadRestrictions::ScopedAllowIO allow_io;
     gl_thread_.reset();
   }
   g_instance = nullptr;
 }
 
 void VrShell::PostToGlThreadWhenReady(const base::Closure& task) {

[ymalik, 2017/05/08 14:05:00]

If we're encapsulating the calls between vr shell and the gl thread behind
interfaces implemented by VrGlThread, it may make sense to wrap these as well?

thoughts?

[cjgrant, 2017/05/08 14:14:37]

I thought about that, and didn't for the following reason:  The UI calls don't
really care about threading - VrShell wants to instantiate some object to manage
the UI, and in this case, the calls happen to be queued to the GL thread.  But,
for the WebVR system, there are explicit dependencies between the main and GL
threads, and I think it's good if that relationship stays crystal clear (ie.
ui_->DoWebVrImplementationThing() doesn't make sense to me).  That's my
reasoning anyway.

[mthiesse, 2017/05/08 16:04:55]

A future CL should probably add another interface for the VrShell/VrShellGl
communication.

[amp, 2017/05/08 18:30:11]

+1, it was confusing to see VrShellGl as the implementation of the browser, yet
it's also the way that VrShell communicates with the UI.  Splitting things out
would reduce confusion.

Did anyone scribble some boxes on a whiteboard somewhere about how it should
look like (even if we can't get there immediately)?

[cjgrant, 2017/05/08 19:17:22]

We basically have 3 components - UI, Shell and GL.  We could have an interface
for each of the 6 possible interaction paths (although at least one of them
isn't used).  As for keeping them in one class, the goal was to keep all the
bind calls and "who's on what thread" in one spot, rather than strewn around. 
Ie, the GL thread is supplying all the cross-thread help.  So I'm not sure
splitting them out would "reduce" confusion, but we should certainly discuss.

Whatever we do, we need to keep the interfaces structured so that blocks can
easily be unit tested.

[amp, 2017/05/08 20:30:06]

I suppose we don't need to pull anything out, but perhaps just name them
differently (per Michael's other comment about the name above).  This is fine to
leave to future technical debt.

Re: GL thread supplying cross-thread help, that feels like the GL thread is
taking on a lot of responsibility.  It's not just the gl thread, it's also the
thing that knows about all the other threads and how to run things on them. (or
am I reading too much into your response?)

The separate interfaces to access those features of GL thread is a nice
abstraction, but it's not what I would imagine is happening if I just read the
name VrGLThread.

And yes keeping things structured for better testing is definitely good.  

   // TODO(mthiesse): Remove this blocking wait. Queue up events if thread isn't
   // finished starting?
   gl_thread_->WaitUntilThreadStarted();
   gl_thread_->task_runner()->PostTask(FROM_HERE, task);
 }
 
 void VrShell::OnContentPaused(bool paused) {
   if (!vr_shell_enabled_)
     return;
 
@@ skipping 45 matching lines @@
 }
 
 void VrShell::SetWebVrMode(JNIEnv* env,
                            const JavaParamRef<jobject>& obj,
                            bool enabled) {
   webvr_mode_ = enabled;
   if (metrics_helper_)
     metrics_helper_->SetWebVREnabled(enabled);
   PostToGlThreadWhenReady(base::Bind(&VrShellGl::SetWebVrMode,
                                      gl_thread_->GetVrShellGl(), enabled));
-
-  ui_->SetMode(enabled ? UiInterface::Mode::WEB_VR
-                       : UiInterface::Mode::STANDARD);
-  PostToGlThreadWhenReady(base::Bind(&UiSceneManager::SetWebVRMode,
-                                     gl_thread_->GetSceneManager(), enabled));
+  ui_->SetWebVr(enabled);
 }
 
 bool VrShell::GetWebVrMode(JNIEnv* env, const JavaParamRef<jobject>& obj) {
   return webvr_mode_;
 }
 
 void VrShell::OnLoadProgressChanged(JNIEnv* env,
                                     const JavaParamRef<jobject>& obj,
                                     double progress) {
   ui_->SetLoadProgress(progress);
 }
 
 void VrShell::OnTabListCreated(JNIEnv* env,
                                const JavaParamRef<jobject>& obj,
                                jobjectArray tabs,
                                jobjectArray incognito_tabs) {
-  ui_->InitTabList();
   ProcessTabArray(env, tabs, false);
   ProcessTabArray(env, incognito_tabs, true);
   ui_->FlushTabList();
 }
 
 void VrShell::ProcessTabArray(JNIEnv* env, jobjectArray tabs, bool incognito) {
   size_t len = env->GetArrayLength(tabs);
   for (size_t i = 0; i < len; ++i) {
     jobject jtab = env->GetObjectArrayElement(tabs, i);
     TabAndroid* tab =
@@ skipping 13 matching lines @@
 }
 
 void VrShell::OnTabRemoved(JNIEnv* env,
                            const JavaParamRef<jobject>& obj,
                            jboolean incognito,
                            jint id) {
   ui_->RemoveTab(incognito, id);
 }
 
 void VrShell::SetWebVRSecureOrigin(bool secure_origin) {
-  PostToGlThreadWhenReady(base::Bind(&UiSceneManager::SetWebVRSecureOrigin,
-                                     gl_thread_->GetSceneManager(),
-                                     secure_origin));
+  ui_->SetWebVRSecureOrigin(secure_origin);
 }
 
 void VrShell::SubmitWebVRFrame(int16_t frame_index,
                                const gpu::MailboxHolder& mailbox) {
   TRACE_EVENT1("gpu", "SubmitWebVRFrame", "frame", frame_index);
 
   PostToGlThreadWhenReady(base::Bind(&VrShellGl::SubmitWebVRFrame,
                                      gl_thread_->GetVrShellGl(), frame_index,
                                      mailbox));
 }
@@ skipping 54 matching lines @@
   content_surface_ = surface;
   compositor_->SurfaceChanged(surface);
   JNIEnv* env = base::android::AttachCurrentThread();
   Java_VrShellImpl_contentSurfaceChanged(env, j_vr_shell_.obj());
 }
 
 void VrShell::GvrDelegateReady() {
   delegate_provider_->SetPresentingDelegate(this, gvr_api_);
 }
 
-void VrShell::AppButtonGesturePerformed(UiInterface::Direction direction) {
-  if (vr_shell_enabled_)
-    ui_->HandleAppButtonGesturePerformed(direction);
-}
-
 void VrShell::ContentPhysicalBoundsChanged(JNIEnv* env,
                                            const JavaParamRef<jobject>& object,
                                            jint width,
                                            jint height,
                                            jfloat dpr) {
   TRACE_EVENT0("gpu", "VrShell::ContentPhysicalBoundsChanged");
   // TODO(acondor): Set the device scale factor for font rendering on the
   // VR Shell textures.
   PostToGlThreadWhenReady(base::Bind(&VrShellGl::ContentPhysicalBoundsChanged,
                                      gl_thread_->GetVrShellGl(), width,
@@ skipping 141 matching lines @@
            jboolean for_web_vr,
            jlong gvr_api,
            jboolean reprojected_rendering) {
   return reinterpret_cast<intptr_t>(new VrShell(
       env, obj, reinterpret_cast<ui::WindowAndroid*>(window_android),
       for_web_vr, VrShellDelegate::GetNativeVrShellDelegate(env, delegate),
       reinterpret_cast<gvr_context*>(gvr_api), reprojected_rendering));
 }
 
 }  // namespace vr_shell