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

chrome/browser/android/vr_shell/vr_shell.cc

Index: chrome/browser/android/vr_shell/vr_shell.cc
diff --git a/chrome/browser/android/vr_shell/vr_shell.cc b/chrome/browser/android/vr_shell/vr_shell.cc
index 049855683b7f4467971c4447786c09ea36982a8f..14f2bfc0baad90b431c563dbe25b131d6fafcfe6 100644
--- a/chrome/browser/android/vr_shell/vr_shell.cc
+++ b/chrome/browser/android/vr_shell/vr_shell.cc
@@ -46,14 +46,15 @@ class GLThread : public base::Thread {
            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();
@@ -63,12 +64,12 @@ class GLThread : public base::Thread {
 
  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();
@@ -83,14 +84,12 @@ class GLThread : public base::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
@@ -122,7 +121,8 @@ VrShell::VrShell(JNIEnv* env,
                                 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;
@@ -133,6 +133,7 @@ VrShell::VrShell(JNIEnv* env,
   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));
@@ -178,6 +179,13 @@ VrShell::~VrShell() {
   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()) {
@@ -252,6 +260,9 @@ 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 {
@@ -292,7 +303,25 @@ void VrShell::SurfacesChanged(jobject content_surface, jobject 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,
@@ -300,13 +329,8 @@ void VrShell::ContentBoundsChanged(JNIEnv* env,
                                    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);
 }
 
@@ -314,13 +338,8 @@ 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);
 }
 
@@ -385,26 +404,18 @@ 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() {