New issue
Advanced search Search tips

Issue 897401 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Oct 23
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android , Windows
Pri: 3
Type: Bug
Proj-VR
Proj-XR
Proj-XR-VR



Sign in to add a comment

vr_display.cc: immersive SessionClientBinding() is created with is_immersive = false

Reported by artyo...@gmail.com, Oct 20

Issue description

vr_display.cc: immersive SessionClientBinding() is created with is_immersive = false

Steps to reproduce the problem:
Check vr_display.cc, line 569:
https://cs.chromium.org/chromium/src/third_party/blink/renderer/modules/vr/vr_display.cc?type=cs&sq=package:chromium&g=0&l=569

Here SessionClientBinding is supposed to be immersive, but the 'false' is passed an 'immersive' parameter.

What is the expected behavior?

What went wrong?
A typo (?) in the code.

Did this work before? N/A 
 
Cc: rbasuvula@chromium.org
Components: UI>Browser>VR
Labels: TE-NeedsTriageHelp
This looks like code related issue, hence adding the respective label for it to  triage further.

Thank You!
Components: -UI>Browser>VR Blink>WebXR>VR
Labels: -Stability-Crash -Restrict-View-EditIssue -TE-NeedsTriageHelp OS-Android OS-Windows
Owner: billorr@chromium.org
Status: Started (was: Unconfirmed)
Thanks for reporting.  This looks like a copy/paste error.

The idea here is that we have potentially multiple "runtimes" backing a single VRDisplay for WebVR so we don't need GVR for magic window sessions.

However, that means we have to aggregate multiple display info changes coming from multiple runtimes and decide which display info the aggregate device reports.  This flag is used as part of that logic - mostly we take the presenting display info.

I coded up a quick fix: https://chromium-review.googlesource.com/c/chromium/src/+/1295317.


Project Member

Comment 3 by bugdroid1@chromium.org, Oct 23

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/bb19b3637494fe1f9c1efa2a3c3b993b7129c1b1

commit bb19b3637494fe1f9c1efa2a3c3b993b7129c1b1
Author: Bill Orr <billorr@chromium.org>
Date: Tue Oct 23 22:39:42 2018

Fix SessionClientBinding constructor parameter for immersive.

The idea here is that we have potentially multiple "runtimes" backing a
single VRDisplay for WebVR so we don't need GVR for magic window sessions.

However, that means we have to aggregate multiple display info changes
coming from multiple runtimes and decide which display info the
aggregate device reports.  This flag is used as part of that logic -
mostly we take the presenting display info.

This was a simple copy-paste error that causes us to treat immersive
sessions as non-immersive for these updates.

BUG= 897401 

Change-Id: If4685282e8756875b85b808aa61aca1e9622cb16
Reviewed-on: https://chromium-review.googlesource.com/c/1295317
Commit-Queue: Bill Orr <billorr@chromium.org>
Reviewed-by: Klaus Weidner <klausw@chromium.org>
Cr-Commit-Position: refs/heads/master@{#602146}
[modify] https://crrev.com/bb19b3637494fe1f9c1efa2a3c3b993b7129c1b1/third_party/blink/renderer/modules/vr/vr_display.cc
[modify] https://crrev.com/bb19b3637494fe1f9c1efa2a3c3b993b7129c1b1/third_party/blink/renderer/modules/vr/vr_display.h

Status: Fixed (was: Started)

Sign in to add a comment