Clean up some VrShell threading issues and remove unnecessary WeakPtr types.

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 "base/metrics/histogram_macros.h"
 #include "base/threading/platform_thread.h"
 #include "base/threading/thread.h"
 #include "base/threading/thread_restrictions.h"
@@ skipping 24 matching lines @@
 
 namespace vr_shell {
 
 namespace {
 vr_shell::VrShell* g_instance;
 
 static const char kVrShellUIURL[] = "chrome://vr-shell-ui";
 
 class GLThread : public base::Thread {
  public:
   GLThread(VrShell* vr_shell, const base::WeakPtr<VrShell>& weak_vr_shell,

[bshe, 2016/12/14 16:06:46]

looks like you can remove vr_shell too?

[mthiesse, 2016/12/14 16:20:43]

Done.

            const base::WeakPtr<VrInputManager>& content_input_manager,
            const base::WeakPtr<VrInputManager>& ui_input_manager,
            scoped_refptr<base::SingleThreadTaskRunner> main_thread_task_runner,
-           gvr_context* gvr_api)
+           gvr_context* gvr_api,
+           bool initially_web_vr)
       : base::Thread("VrShellGL"),
-        vr_shell_(vr_shell),
         weak_vr_shell_(weak_vr_shell),
         content_input_manager_(content_input_manager),
         ui_input_manager_(ui_input_manager),
         main_thread_task_runner_(std::move(main_thread_task_runner)),
-        gvr_api_(gvr_api) {}
+        gvr_api_(gvr_api),
+        initially_web_vr_(initially_web_vr) {}
 
   ~GLThread() override {
     Stop();
   }
   base::WeakPtr<VrShellGl> GetVrShellGl() { return weak_vr_shell_gl_; }
   VrShellGl* GetVrShellGlUnsafe() { return vr_shell_gl_.get(); }
 
  protected:
   void Init() override {
-    vr_shell_gl_.reset(new VrShellGl(vr_shell_,
-                                     std::move(weak_vr_shell_),
+    vr_shell_gl_.reset(new VrShellGl(std::move(weak_vr_shell_),
                                      std::move(content_input_manager_),
                                      std::move(ui_input_manager_),
                                      std::move(main_thread_task_runner_),
-                                     gvr_api_));
+                                     gvr_api_,
+                                     initially_web_vr_));
     weak_vr_shell_gl_ = vr_shell_gl_->GetWeakPtr();
     if (!vr_shell_gl_->Initialize()) {
       vr_shell_gl_.reset();
     }
   }
   void CleanUp() override {
     vr_shell_gl_.reset();
   }
 
  private:
   // Created on GL thread.
   std::unique_ptr<VrShellGl> vr_shell_gl_;
   base::WeakPtr<VrShellGl> weak_vr_shell_gl_;
 
-  // Created on main thread.
-  // TODO(mthiesse): Remove vr_shell_.
-  VrShell* vr_shell_;
   base::WeakPtr<VrShell> weak_vr_shell_;
   base::WeakPtr<VrInputManager> content_input_manager_;
   base::WeakPtr<VrInputManager> ui_input_manager_;
   scoped_refptr<base::SingleThreadTaskRunner> main_thread_task_runner_;
   gvr_context* gvr_api_;
+  bool initially_web_vr_;
 };
 
 }  // namespace
 
 VrShell::VrShell(JNIEnv* env,
                  jobject obj,
                  content::WebContents* main_contents,
                  ui::WindowAndroid* content_window,
                  content::WebContents* ui_contents,
                  ui::WindowAndroid* ui_window,
@@ skipping 11 matching lines @@
   g_instance = this;
   j_vr_shell_.Reset(env, obj);
 
   content_input_manager_.reset(new VrInputManager(main_contents_));
   ui_input_manager_.reset(new VrInputManager(ui_contents_));
 
   gl_thread_.reset(new GLThread(this, weak_ptr_factory_.GetWeakPtr(),
                                 content_input_manager_->GetWeakPtr(),
                                 ui_input_manager_->GetWeakPtr(),
                                 main_thread_task_runner_,
-                                gvr_api));
+                                gvr_api,
+                                for_web_vr));
 
   base::Thread::Options options(base::MessageLoop::TYPE_DEFAULT, 0);
   options.priority = base::ThreadPriority::DISPLAY;
   gl_thread_->StartWithOptions(options);
 
   if (for_web_vr)
     metrics_helper_->SetWebVREnabled(true);
   html_interface_.reset(new UiInterface(
       for_web_vr ? UiInterface::Mode::WEB_VR : UiInterface::Mode::STANDARD,
       main_contents_->IsFullscreen()));
+
   content_compositor_.reset(new VrCompositor(content_window, false));
   content_compositor_->SetLayer(main_contents_);
   ui_compositor_.reset(new VrCompositor(ui_window, true));
   ui_compositor_->SetLayer(ui_contents_);
   vr_web_contents_observer_.reset(new VrWebContentsObserver(
       main_contents, html_interface_.get(), this));
 
   SetShowingOverscrollGlow(false);
 }
 
@@ skipping 25 matching lines @@
     // 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();
   }
   delegate_->RemoveDelegate();
   g_instance = nullptr;
 }
 
+void VrShell::PostToGlThreadWhenReady(const base::Closure& task) {
+  // 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::SetGvrPoseForWebVr(const gvr::Mat4f& pose, uint32_t pose_num) {
   GLThread* thread = static_cast<GLThread*>(gl_thread_.get());
   if (thread->GetVrShellGlUnsafe()) {
     thread->GetVrShellGlUnsafe()->SetGvrPoseForWebVr(pose, pose_num);
   }
 }
 
 void VrShell::SetWebVRRenderSurfaceSize(int width, int height) {
   GLThread* thread = static_cast<GLThread*>(gl_thread_.get());
   if (thread->GetVrShellGlUnsafe()) {
@@ skipping 54 matching lines @@
 void VrShell::OnDomContentsLoaded() {
   html_interface_->SetURL(main_contents_->GetVisibleURL());
   html_interface_->SetLoading(main_contents_->IsLoading());
   html_interface_->OnDomContentsLoaded();
 }
 
 void VrShell::SetWebVrMode(JNIEnv* env,
                            const base::android::JavaParamRef<jobject>& obj,
                            bool enabled) {
   metrics_helper_->SetWebVREnabled(enabled);
+  GLThread* thread = static_cast<GLThread*>(gl_thread_.get());
+  PostToGlThreadWhenReady(
+      base::Bind(&VrShellGl::SetWebVrMode, thread->GetVrShellGl(), enabled));

[bshe, 2016/12/14 16:06:46]

is thread->GetVrShellGl() evaluated when the task actually runs? If not,
wouldn't it be null and the task is just ignored?

[mthiesse, 2016/12/14 16:20:43]

If the WeakPtr at thread->GetVrShellGl() is null, the task will get ignored. I
think this is what we want in this case.

[bshe, 2016/12/14 17:36:51]

For optional tasks, I would prefer to use IsRunning method to make it
more readable. But this is not blocking.

[mthiesse, 2016/12/14 21:25:36]

Done.

   if (enabled) {
     html_interface_->SetMode(UiInterface::Mode::WEB_VR);
   } else {
     html_interface_->SetMode(UiInterface::Mode::STANDARD);
   }
 }
 
 void VrShell::SetWebVRSecureOrigin(bool secure_origin) {
   // TODO(cjgrant): Align this state with the logic that drives the omnibox.
   html_interface_->SetWebVRSecureOrigin(secure_origin);
@@ skipping 20 matching lines @@
   CHECK(false);
   return nullptr;
 }
 
 void VrShell::SurfacesChanged(jobject content_surface, jobject ui_surface) {
   content_compositor_->SurfaceChanged(content_surface);
   ui_compositor_->SurfaceChanged(ui_surface);
 }
 
 void VrShell::GvrDelegateReady() {
-  delegate_->SetDelegate(weak_ptr_factory_.GetWeakPtr());
+  delegate_->SetDelegate(this);
+}
+
+void VrShell::AppButtonPressed() {
+#if defined(ENABLE_VR_SHELL)
+  html_interface_->SetMenuMode(!html_interface_->GetMenuMode());
+
+  // TODO(mthiesse): The page is no longer visible when in menu mode. We
+  // should unfocus or otherwise let it know it's hidden.
+  if (html_interface_->GetMode() == UiInterface::Mode::WEB_VR) {
+    if (delegate_->device_provider()) {
+      if (html_interface_->GetMenuMode()) {
+        delegate_->device_provider()->OnDisplayBlur();
+      } else {
+        delegate_->device_provider()->OnDisplayFocus();
+      }
+    }
+  }
+#endif
 }
 
 void VrShell::ContentBoundsChanged(JNIEnv* env,
                                    const JavaParamRef<jobject>& object,
                                    jint width, jint height, jfloat dpr) {
   TRACE_EVENT0("gpu", "VrShell::ContentBoundsChanged");
   GLThread* thread = static_cast<GLThread*>(gl_thread_.get());
-  // TODO(mthiesse): Remove this blocking wait. Queue up events if thread isn't
-  // finished starting?
-  thread->WaitUntilThreadStarted();
-  CHECK(thread->task_runner()->PostTask(
-      FROM_HERE, base::Bind(&VrShellGl::ContentPhysicalBoundsChanged,
-                            thread->GetVrShellGl(),
-                            width, height)));
+  PostToGlThreadWhenReady(base::Bind(&VrShellGl::ContentPhysicalBoundsChanged,
+                                     thread->GetVrShellGl(), width, height));
   content_compositor_->SetWindowBounds(width, height);
 }
 
 void VrShell::UIBoundsChanged(JNIEnv* env,
                               const JavaParamRef<jobject>& object,
                               jint width, jint height, jfloat dpr) {
   GLThread* thread = static_cast<GLThread*>(gl_thread_.get());
-  // TODO(mthiesse): Remove this blocking wait. Queue up events if thread isn't
-  // finished starting?
-  thread->WaitUntilThreadStarted();
-  thread->task_runner()->PostTask(
-      FROM_HERE, base::Bind(&VrShellGl::UIPhysicalBoundsChanged,
-                            thread->GetVrShellGl(),
-                            width, height));
+  PostToGlThreadWhenReady(base::Bind(&VrShellGl::UIPhysicalBoundsChanged,
+                                     thread->GetVrShellGl(), width, height));
   ui_compositor_->SetWindowBounds(width, height);
 }
 
 UiScene* VrShell::GetScene() {
   GLThread* thread = static_cast<GLThread*>(gl_thread_.get());
   // TODO(mthiesse): Remove this blocking wait. Queue up events if thread isn't
   // finished starting?
   thread->WaitUntilThreadStarted();
   if (thread->GetVrShellGlUnsafe()) {
     return thread->GetVrShellGlUnsafe()->GetScene();
@@ skipping 44 matching lines @@
 
 void VrShell::RenderViewHostChanged(content::RenderViewHost* old_host,
                                     content::RenderViewHost* new_host) {
   new_host->GetWidget()->GetView()->SetBackgroundColor(SK_ColorTRANSPARENT);
 }
 
 void VrShell::MainFrameWasResized(bool width_changed) {
   display::Display display = display::Screen::GetScreen()
       ->GetDisplayNearestWindow(ui_contents_->GetNativeView());
   GLThread* thread = static_cast<GLThread*>(gl_thread_.get());
-  // TODO(mthiesse): Remove this blocking wait. Queue up events if thread isn't
-  // finished starting?
-  thread->WaitUntilThreadStarted();
-  thread->task_runner()->PostTask(
-      FROM_HERE, base::Bind(&VrShellGl::UIBoundsChanged,
-                            thread->GetVrShellGl(),
-                            display.size().width(), display.size().height()));
+  PostToGlThreadWhenReady(
+      base::Bind(&VrShellGl::UIBoundsChanged, thread->GetVrShellGl(),
+                 display.size().width(), display.size().height()));
 }
 
 void VrShell::ContentFrameWasResized(bool width_changed) {
   display::Display display = display::Screen::GetScreen()
       ->GetDisplayNearestWindow(main_contents_->GetNativeView());
   GLThread* thread = static_cast<GLThread*>(gl_thread_.get());
-  // TODO(mthiesse): Remove this blocking wait. Queue up events if thread isn't
-  // finished starting?
-  thread->WaitUntilThreadStarted();
-  thread->task_runner()->PostTask(
-      FROM_HERE, base::Bind(&VrShellGl::ContentBoundsChanged,
-                            thread->GetVrShellGl(),
-                            display.size().width(), display.size().height()));
+  PostToGlThreadWhenReady(
+      base::Bind(&VrShellGl::ContentBoundsChanged, thread->GetVrShellGl(),
+                 display.size().width(), display.size().height()));
 }
 
 void VrShell::WebContentsDestroyed() {
   ui_input_manager_.reset();
   ui_contents_ = nullptr;
   // TODO(mthiesse): Handle web contents being destroyed.
   ForceExitVR();
 }
 
 void VrShell::ContentWebContentsDestroyed() {
@@ skipping 39 matching lines @@
   return reinterpret_cast<intptr_t>(new VrShell(
       env, obj, content::WebContents::FromJavaWebContents(content_web_contents),
       reinterpret_cast<ui::WindowAndroid*>(content_window_android),
       content::WebContents::FromJavaWebContents(ui_web_contents),
       reinterpret_cast<ui::WindowAndroid*>(ui_window_android),
       for_web_vr, VrShellDelegate::GetNativeDelegate(env, delegate),
       reinterpret_cast<gvr_context*>(gvr_api)));
 }
 
 }  // namespace vr_shell