media: Fix FirstApiLevel check in provisioning [chromium/src : main]
Vikram Pasupathy (Gerrit)
Vikram Pasupathy voted Commit-Queue+1![]( "Open in Gerrit")
| Commit-Queue | +1 |
Related details
- Xiaohan Wang
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Xiaohan Wang (Gerrit)
Xiaohan Wang added 2 comments![]( "Open in Gerrit")
base::UmaHistogramSparse("Media.EME.MediaDrm.FirstApiLevel", first_api_level);This should be integers with a defined range? Maybe it's not "sparse"?
// released before "ro.product.first_api_level" was introduced.In this case, is it possible that the SKD version is less than O? I don't know when the first_api_level was introduced...
Related details
- Vikram Pasupathy
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Vikram Pasupathy (Gerrit)
Vikram Pasupathy added 2 comments![]( "Open in Gerrit")
base::UmaHistogramSparse("Media.EME.MediaDrm.FirstApiLevel", first_api_level);This should be integers with a defined range? Maybe it's not "sparse"?
AW team does the same here for their ApiLevel:
So I just copied that, which I think makes sense.
// released before "ro.product.first_api_level" was introduced.In this case, is it possible that the SKD version is less than O? I don't know when the first_api_level was introduced...
I also took a look at that, this document http://shortn/_gsiTlfzxII says it was introduced in between MR and N.
I sanity checked it by googling "ro.product.first_api_level" Nougat and saw some dumped builds where this was in fact set to Nougats value, which is pre O.
I looked at the lifetime support for a couple of devices, and it looks like most of them ended support at Pie, which is actually being deprecated currently, but the Pixel is supported til Quince, which Chromium actively supports.
So thats why this UMA will be useful to see how many of these old clients still use this flow, and if we can remove this or not.
Related details
- Xiaohan Wang
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Xiaohan Wang (Gerrit)
Xiaohan Wang voted and added 3 comments![]( "Open in Gerrit")
Votes added by Xiaohan Wang
| Code-Review | +1 |
3 comments
base::UmaHistogramSparse("Media.EME.MediaDrm.FirstApiLevel", first_api_level);Vikram PasupathyThis should be integers with a defined range? Maybe it's not "sparse"?
AW team does the same here for their ApiLevel:
So I just copied that, which I think makes sense.
Thanks for the example. I don't know what's the recommendataion is here.
Please keep this unresolved so the metrics reviewer can take a look.
} else if (first_api_level == 0) {nit: No need to use else as we return early.
```
if (first_api_level >= base::android::android_info::SDK_VERSION_OREO) {
return true;
}
if (first_api_level == 0) {
return ...
}return false;
```
// released before "ro.product.first_api_level" was introduced.Vikram PasupathyIn this case, is it possible that the SKD version is less than O? I don't know when the first_api_level was introduced...
I also took a look at that, this document http://shortn/_gsiTlfzxII says it was introduced in between MR and N.
I sanity checked it by googling "ro.product.first_api_level" Nougat and saw some dumped builds where this was in fact set to Nougats value, which is pre O.
I looked at the lifetime support for a couple of devices, and it looks like most of them ended support at Pie, which is actually being deprecated currently, but the Pixel is supported til Quince, which Chromium actively supports.
So thats why this UMA will be useful to see how many of these old clients still use this flow, and if we can remove this or not.
- Vikram Pasupathy
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |