VrShellGl::PostToGlThreadWhenReady doesn't do what it intended to do
Reported by
artyo...@gmail.com,
May 17 2017
|
|||
Issue description
Steps to reproduce the problem:
PostToGlThreadWhenReady supposed to post a task to VrShellGl making sure the VR thread is started. For example:
PostToGlThreadWhenReady(base::Bind(&VrShellGl::SetWebVrMode,
gl_thread_->GetVrShellGl(), enabled));
What is the expected behavior?
What went wrong?
However, it assumes that gl_thread_->GetVrShellGl() is already not NULL, which is not true until the thread is started and its Init() method is called! Since it records the state of gl_thread_->GetVrShellGl() before waiting the thread start, the whole PostToGlThreadWhenReady is pretty much pointless.
Did this work before? No
Does this work in other browsers? N/A
Chrome version: M60 master Channel: dev
OS Version:
Flash Version:
The naive way to fix the issue is to use macro instead of the actual method.
There is second issue as well: VrShell::OnResume doesn't use PostToGlThreadWhenReady and uses a regular PostTask call for VrShellGl::OnResume posting. OnResume maybe called before the VR thread is started, thus VrShellGl::OnResume maybe missed. Need to use PostToGlThreadWhenReady (assuming the first issue is fixed and it works correctly).
,
May 17 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/5f7ac3af3be63d35ecba7c6814c842f4d1c12da3 commit 5f7ac3af3be63d35ecba7c6814c842f4d1c12da3 Author: mthiesse <mthiesse@chromium.org> Date: Wed May 17 16:32:31 2017 VR: Make sure GL thread is started before binding callbacks to it. BUG= 723262 Review-Url: https://codereview.chromium.org/2893623002 Cr-Commit-Position: refs/heads/master@{#472475} [modify] https://crrev.com/5f7ac3af3be63d35ecba7c6814c842f4d1c12da3/chrome/browser/android/vr_shell/vr_shell.cc [modify] https://crrev.com/5f7ac3af3be63d35ecba7c6814c842f4d1c12da3/chrome/browser/android/vr_shell/vr_shell.h
,
May 17 2017
,
Jul 4
|
|||
►
Sign in to add a comment |
|||
Comment 1 by ddorwin@chromium.org
, May 17 2017Owner: mthiesse@chromium.org
Status: Assigned (was: Unconfirmed)