New issue
Advanced search Search tips

Issue 762588 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Mar 2018
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 3
Type: Bug
Proj-VR
Proj-XR
Proj-XR-VR



Sign in to add a comment

Call requires API level 17 (current min is 16): `android.view.Display#getRealMetrics`: NewApi

Project Member Reported by wnwen@chromium.org, Sep 6 2017

Issue description

org/chromium/chrome/browser/vr_shell/VrShellImpl.class Call requires API level 17 (current min is 16): `android.view.Display#getRealMetrics`: NewApi [warning]

CL: https://chromium-review.googlesource.com/c/chromium/src/+/608877
 

Comment 1 by tiborg@chromium.org, Sep 15 2017

Labels: -Pri-1 M-64 Pri-2

Comment 2 by wnwen@chromium.org, Sep 18 2017

Please reconsider landing a P1 fix for M63. Unless you know for sure that the android API was available for API level 16, or no users on API level 16 will hit this code path, it will result in many crashes.

Comment 3 by tiborg@chromium.org, Sep 18 2017

We are only targeting Android K and newer. This code should never be executed on devices with API level 16 or lower.

Comment 4 by wnwen@chromium.org, Sep 18 2017

Thanks for the clarification!

Comment 5 by tiborg@chromium.org, Oct 20 2017

Components: UI>Browser>VR
Labels: -Pri-2 Pri-3
Setting that as not time critical since we should never hit that code with an API older than 17.
What are the next steps? Do we need to change the way we use this API or suppress the warning?

Comment 7 by wnwen@chromium.org, Oct 30 2017

Please update the comment here to confirm, preferably with a reason, why you are sure the code is not run on K or lower: https://cs.chromium.org/chromium/src/chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrShellImpl.java?q=vrshellimpl&sq=package:chromium&l=320

Thanks!

Comment 8 by tiborg@chromium.org, Oct 30 2017

We can also check for the API level (e.g. like here https://cs.chromium.org/chromium/src/ui/android/java/src/org/chromium/ui/base/DeviceFormFactor.java?rcl=dd64a192d8b8a180fb40f389f4ecf8680467e968&l=62) and use default values here https://cs.chromium.org/chromium/src/chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrShellImpl.java?rcl=02ed3cc1d6da1dc9263ad44f9c1bc662044d78c9&l=328. The default values should never be used. However, it would give us a backup that we won't crash in case we still hit this code path.

Comment 9 by wnwen@chromium.org, Oct 30 2017

That would be best, thanks!
Labels: -M-64 M-65
Labels: -M-65 M-66
Project Member

Comment 12 by bugdroid1@chromium.org, Mar 1 2018

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

commit a7e4fc0f8be6dc90425927d054ab9a246282f507
Author: Tibor Goldschwendt <tiborg@chromium.org>
Date: Thu Mar 01 01:44:00 2018

[vr] Make sure Display.getRealMetrics() is only used on API Level 17+

Bug:  762588 
Change-Id: I73723544788847a2209155eec756c7573dfe7b1d
Reviewed-on: https://chromium-review.googlesource.com/788891
Reviewed-by: Michael Thiessen <mthiesse@chromium.org>
Commit-Queue: Tibor Goldschwendt <tiborg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#539974}
[modify] https://crrev.com/a7e4fc0f8be6dc90425927d054ab9a246282f507/chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrShellImpl.java

Status: Fixed (was: Assigned)
Labels: Test-Complete

Sign in to add a comment