VRDisplay.requestPresent fails on Samsung S8+
Reported by
mckay.da...@gmail.com,
May 5 2017
|
||||||||||||||||
Issue descriptionSteps to reproduce the problem: 1. On a Samsung Galaxy S8+, visit https://webvr.info/samples/03-vr-presentation.html 2. Press the 'Enter VR' button to trigger the call to vrDevice.requestPresent on line 145 of the html file What is the expected behavior? The requestPresent call succeeds and Chrome successfully enters WebVR presentation mode. The webgl canvas fills the entire screen with left and right eye views of the scene. What went wrong? The error callback of the requestPresent promise is returned instead with the following error: DOMException: Presentation request was denied. {code: 0, name: "NotAllowedError", message: "Presentation request was denied." Furthermore, Chrome is now `stuck` in landscape mode and cannot be re-oriented to portrait mode. Did this work before? N/A Does this work in other browsers? Yes Chrome version: 58.0.3029.83 Channel: stable OS Version: 7.0.0; SM-G955U Build/NRD90M Flash Version: The same Chrome build & steps work correctly on a Galaxy S5. Although the origin-trail token on webvr.info enables WebVR, I've also manually enabled the chrome://flags#enable-vr flag to enabled. Prior to the requestPresent call, navigator.getVRDisplays is returning a single VRDisplay w/ displayName: "Google, Inc. Default Cardboard"
,
May 5 2017
Confirmed on an S8 with Beta 59.0.3071.36. (I had to enable the flag due to the OT issue.) If I refresh the page and then hit Enter VR, I get a red box with the message "ERROR: requestPresent failed. Presentation request was denied." It works on an S8 on (out of date) Stable 57.0.2987.132. It appears we have a regression from 57 to 58. Google VR Services 1.5.153389785
,
May 5 2017
It works in Canary 60.0.3089.0 on a 5X
,
May 5 2017
Can you include a bug report or logs?
,
May 5 2017
Assigning to klausw@ as I wasn't able to get the s8 connecting to my machine.
,
May 6 2017
I've attached a bugreport. Some random observations:
ChromeTabbedActivity and WindowManager seem to be stuck in an update loop, with multiple log messages per second like these:
05-05 16:34:07.116 13494 13494 D ViewRootImpl@3f2bb9[ChromeTabbedActivity]: Relayout returned: oldFrame=[0,0][2960,1440] newFrame=[0,0][2960,1440] result=0x1 surface={isValid=true -552710144} surfaceGenerationChanged=false
05-05 16:34:07.125 1173 2747 V WindowManager: Relayout Window{ed7dcadd0 u0 com.chrome.beta/org.chromium.chrome.browser.ChromeTabbedActivity}: viewVisibility=0 req=2960x1440 WM.LayoutParams{(0,0)(fillxfill) sim=#13 ty=1 fl=#1810180 pfl=0x20000 fmt=-3 wanim=0x1030465 vsysu
i=0x1716 sysuil=true needsMenuKey=2 naviIconColor=0}
05-05 16:34:07.132 13494 13494 D ViewRootImpl@3f2bb9[ChromeTabbedActivity]: Relayout returned: oldFrame=[0,0][2960,1440] newFrame=[0,0][2960,1440] result=0x1 surface={isValid=true -552710144} surfaceGenerationChanged=false
05-05 16:34:07.142 1173 2787 V WindowManager: Relayout Window{ed7dcadd0 u0 com.chrome.beta/org.chromium.chrome.browser.ChromeTabbedActivity}: viewVisibility=0 req=2960x1440 WM.LayoutParams{(0,0)(fillxfill) sim=#13 ty=1 fl=#1810180 pfl=0x20000 fmt=-3 wanim=0x1030465 vsysu
i=0x1716 sysuil=true needsMenuKey=2 naviIconColor=0}
05-05 16:48:30.174 1186 1186 I Ion : [vr/gvr/capi/src/gvr_core_api_loader_android.cc:174] Successfully loaded GVR library version 1.40.0 from VrCore (target was 1.10.0).
05-05 16:48:30.222 1186 1186 I GVR : [vr/gvr/capi/src/gvr.cc:96] Initialized GVR version 1.40.0
Also, Javascript seems to be in a crazy multithreaded mode where it keeps animating while allegedly paused in magic window mode, I filed the separate issue 719101 for that one.
,
May 6 2017
Is Android VR mode being enabled on the S8? You might try not enabling VR mode to see if that makes any difference (either remove the "enableVrMode" manifest entry for your Activity, or don't call Activity.setVrModeEnabled()). Samsung might be doing something behind the scenes which is causing the thrashing.
,
May 8 2017
Correction: #2-#6 were using an S8+.
,
May 10 2017
I see same behavior on S8 (& S8+) as well. Thru Canary: 60.0.3094.0.
,
May 12 2017
Attaching bug report. I initially thought this was related to the Do not disturb and notification access permissions, which are not granted by default on Samsung devices. I thought I had seen everything working once I enabled those permissions, but now even with those permissions granted to vr services the issue still repro's on recent versions of Chrome. It does not repro on Chrome 57 though. One of the changes that occured around that timeframe was treating the vr device as a virtual display instead of manually overriding sizes directly. That might have different behavior on a samsung device than on a pixel, but it's only a theory. I was able to take a bug report of using Chrome canary (see attached). Some notes as I was trying to look into the permissions: vr services 1.5.153389785 chrome version 57.0.2987.132 - prompts for permissions about vr core (after first install only). - works fine in cardboard mode without permissions chrome 59.0.3071.36 - does not prompt for permissions - does not transition to presentation (goes into a weird landscape mode and apparently has multiple rAFs going, see issue 719101 ) - You can add permissions directly (from Settings > Apps > Menu (3 dots) > Special Access > [Do not disturb permission, Notification access] > Toggle and Allow Google Vr Services), but it does not get it working again.
,
May 13 2017
Might be onto something there w.r.t. display handling.. On a S8-proto device I have that seemingly handles the display resolution a bit differently than production devices works normally, entering VR Cardboard successfully. Stable: 58.0.3029.83 Canary: 60.0.3097.0 VR services 1.5.153389785 Note that with S8/+ Samsung introduced new screen resolution & aspect ratio, with special handling of reported resolutions to apps (See 'Full screen apps' in settings) which supposed to be transparent but could potentially have side effects in certain special use cases. (Also though, I swear I observed this landscape+non-present behavior right after got Nougat update for GS7 a couple days ago.. but can no longer reproduce.)
,
May 15 2017
The display changes I were thinking of actually went in last Dec, (with https://codereview.chromium.org/2428383006) which was well before the 57 branch point so I don't think those could be the reason things are breaking in 58+. There may have been some other display changes after later though, so I'll try to add some logging in related areas and see if anything looks suspicious.
,
May 16 2017
Some more digging revealed the following. We are getting into a loop trying to set the system ui flags and checking to make sure they match what we set (but they never do). We set them here: https://cs.chromium.org/chromium/src/chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrShellDelegate.java?l=861 and retrieve them here to make sure they are set correctly: https://cs.chromium.org/chromium/src/chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrShellDelegate.java?l=534 The thing is, on the S8 the flag 0x10 is always set, but it isn't defined in the current SDK: https://developer.android.com/reference/android/view/View.html#getSystemUiVisibility() I can't figure out why that value is getting set, but if I change the flag to mask only the ones we set, then everything works great. Perhaps there is a custom Samsung implementation for mActivity.getWindow().getDecorView().getSystemUiVisibility() that returns that flag always set? Interestingly, the logs show the following which corresponds to the flag which shouldn't be there (but searching for fullscreenStackSysUiFlag got me nowhere): D WindowManager: set systemUiVisibility of statusbar : systemUiFlags= 0x418 fullscreenStackSysUiFlag= 0x10 The change that causes this to break was the one that added the check mentioned above, https://codereview.chromium.org/2785493006. This was merged back to 58, but not 57 which explains why it still works on 57 builds (we didn't check the flags there, just set them and trusted they were right). I'll prepare a fix using the masks, but I'm not sure if we want to push it all the way back to 58 or not.
,
May 16 2017
,
May 16 2017
Thanks, amp@! This isn't critical enough to merge to stable (M58).
,
May 17 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/edc6da3999ae141bef700d80b69323849e2290de commit edc6da3999ae141bef700d80b69323849e2290de Author: amp <amp@chromium.org> Date: Wed May 17 00:08:21 2017 [VrShell] Mask system ui flags before checking. This fixes an issue where extraneous flags were being set on some devices and causing the flag check to fail. BUG= 719013 Review-Url: https://codereview.chromium.org/2881403003 Cr-Commit-Position: refs/heads/master@{#472257} [modify] https://crrev.com/edc6da3999ae141bef700d80b69323849e2290de/chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrShellDelegate.java
,
May 17 2017
I'm the original finder / filer of this bug and I'm just chiming in to say *much thanks @amp*! (as well everyone else who worked to find it) @ddorwin, I must ask why you say: "This isn't critical enough to merge to stable (M58)."? The risk if the bug remains in stable is that it allows easy authoring of semi-malicious html+js pages. Any page with an OT, when clicked to enter WebVR on S8 phones (already @ 5 million+ devices), will immediately send Chrome into an infinite loop and lock its UI into landscape mode that the user cannot escape from. This landscape mode seems to persist even after killing Chrome. (Admittedly, the page author would need a Origins trial token to enable this behavior...) As a developer who is very actively creating WebVR code for my S8, I eagerly await the day when this fix is deployed to the entire fleet of S8s. :)
,
May 17 2017
Thank you for your concern and description of the risk. The bar for a stable merge is high, and M58 is more than half way through its stable lifetime. In addition to a malicious site requesting an origin token and getting a user to visit and click on the site, the users would need to (at some point) manually install and enable Google VR Services. +amineer in case he feels differently. Also, FYI, we will be requesting a merge to M59 after this bakes.
,
May 17 2017
Based on a quick skim of the description this is a denial of service attack rather than an escalation of privilege that puts the user at risk, exposes data, etc etc. While I understand the concern, shipping a new Chrome release means sending bits to more than a billion users - and in a lot of places downloading those bits costs our users cash (think emerging markets) and time. As such we only respin stable for critical issues and vulnerabilities, and I don't think this meets the bar. Re-CC me if you have further concerns.
,
May 17 2017
I appreciate the consideration of my merge into 58 request. Those are all very reasonable explanations as to why it's not wise. Hopefully there are no issues w/ it in M59. Thanks @all! Looking forward to developing WebVR on the S8+!
,
May 17 2017
,
May 17 2017
Your change meets the bar and is auto-approved for M59. Please go ahead and merge the CL to branch 3071 manually. Please contact milestone owner if you have questions. Owners: amineer@(Android), cmasso@(iOS), gkihumba@(ChromeOS), Abdul Syed@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
May 17 2017
mckay.davis: The fix is now in Canary 60.0.3102.0.
,
May 17 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/142aaf16c7ae26acda3ef8c0072520916a390aa8 commit 142aaf16c7ae26acda3ef8c0072520916a390aa8 Author: amp <amp@chromium.org> Date: Wed May 17 21:12:14 2017 [VrShell] Mask system ui flags before checking. This fixes an issue where extraneous flags were being set on some devices and causing the flag check to fail. BUG= 719013 NOTRY=true NOPRESUBMIT=true Review-Url: https://codereview.chromium.org/2881403003 Cr-Original-Commit-Position: refs/heads/master@{#472257} Review-Url: https://codereview.chromium.org/2888973002 Cr-Commit-Position: refs/branch-heads/3071@{#611} Cr-Branched-From: a106f0abbf69dad349d4aaf4bcc4f5d376dd2377-refs/heads/master@{#464641} [modify] https://crrev.com/142aaf16c7ae26acda3ef8c0072520916a390aa8/chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrShellDelegate.java
,
May 18 2017
Marking fixed as this has now landed on the 59 branch, but it will still need to be verified in the next beta release.
,
May 25 2017
The fix was released to 59 beta as of version 59.0.3071.71 and verified there. Please let us know if you see anything similar after testing with that version or later.
,
Jul 4
|
||||||||||||||||
►
Sign in to add a comment |
||||||||||||||||
Comment 1 by mckay.da...@gmail.com
, May 5 2017