New issue
Advanced search Search tips

Issue 861954 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Sep 20
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Task



Sign in to add a comment

Media Intent Handler: Add metrics

Project Member Reported by steimel@chromium.org, Jul 9

Issue description

Add metrics to the media intent handler
 
Project Member

Comment 1 by bugdroid1@chromium.org, Jul 13

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

commit 76ee32bd378f02564effa4ddc7b956cb0b134123
Author: Tommy Steimel <steimel@chromium.org>
Date: Fri Jul 13 21:19:53 2018

[Media Intent Handler] Add UMA metrics for media type

This CL adds a MediaLauncherActivity.MediaType histogram to track the
type of media the user opens with the MediaLauncherActivity
(audio/image/video).

Bug:  861954 
Change-Id: I28cc0a379ad2bc70d34f2d2f5751c3eb2ebbda45
Reviewed-on: https://chromium-review.googlesource.com/1135522
Reviewed-by: Steven Holte <holte@chromium.org>
Reviewed-by: Mounir Lamouri <mlamouri@chromium.org>
Commit-Queue: Tommy Steimel <steimel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#575071}
[modify] https://crrev.com/76ee32bd378f02564effa4ddc7b956cb0b134123/chrome/android/java/src/org/chromium/chrome/browser/media/MediaLauncherActivity.java
[modify] https://crrev.com/76ee32bd378f02564effa4ddc7b956cb0b134123/chrome/android/java/src/org/chromium/chrome/browser/media/MediaViewerUtils.java
[modify] https://crrev.com/76ee32bd378f02564effa4ddc7b956cb0b134123/tools/metrics/histograms/enums.xml
[modify] https://crrev.com/76ee32bd378f02564effa4ddc7b956cb0b134123/tools/metrics/histograms/histograms.xml

Project Member

Comment 2 by bugdroid1@chromium.org, Jul 13

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

commit 29f92361cbf018cc8af847ac3cc4d52224d5a2d5
Author: Tommy Steimel <steimel@chromium.org>
Date: Fri Jul 13 23:39:32 2018

Revert "[Media Intent Handler] Add UMA metrics for media type"

This reverts commit 76ee32bd378f02564effa4ddc7b956cb0b134123.

Reason for revert: Histogram recording crashes Chrome if it isn't already open because native isn't initialized.

Original change's description:
> [Media Intent Handler] Add UMA metrics for media type
> 
> This CL adds a MediaLauncherActivity.MediaType histogram to track the
> type of media the user opens with the MediaLauncherActivity
> (audio/image/video).
> 
> Bug:  861954 
> Change-Id: I28cc0a379ad2bc70d34f2d2f5751c3eb2ebbda45
> Reviewed-on: https://chromium-review.googlesource.com/1135522
> Reviewed-by: Steven Holte <holte@chromium.org>
> Reviewed-by: Mounir Lamouri <mlamouri@chromium.org>
> Commit-Queue: Tommy Steimel <steimel@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#575071}

TBR=mlamouri@chromium.org,holte@chromium.org,steimel@chromium.org

Change-Id: Ic090a8660d6139fc6043a8990b0c6966a9cb311e
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  861954 
Reviewed-on: https://chromium-review.googlesource.com/1137392
Reviewed-by: Tommy Steimel <steimel@chromium.org>
Commit-Queue: Tommy Steimel <steimel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#575116}
[modify] https://crrev.com/29f92361cbf018cc8af847ac3cc4d52224d5a2d5/chrome/android/java/src/org/chromium/chrome/browser/media/MediaLauncherActivity.java
[modify] https://crrev.com/29f92361cbf018cc8af847ac3cc4d52224d5a2d5/chrome/android/java/src/org/chromium/chrome/browser/media/MediaViewerUtils.java
[modify] https://crrev.com/29f92361cbf018cc8af847ac3cc4d52224d5a2d5/tools/metrics/histograms/enums.xml
[modify] https://crrev.com/29f92361cbf018cc8af847ac3cc4d52224d5a2d5/tools/metrics/histograms/histograms.xml

Components: -Internals>Media Internals>Media>UI
Project Member

Comment 4 by bugdroid1@chromium.org, Jul 23

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

commit 8a874e08fcdfbfe28bfb7aaf2e176c943726b3e4
Author: Tommy Steimel <steimel@chromium.org>
Date: Mon Jul 23 22:17:33 2018

[Media Intent Handler] Reland add UMA metrics for media type

This CL re-adds media type UMA metrics to the MediaLauncherActivity. In
order to prevent the native initialization issues from the previous CL,
this CL uses CachedMetrics to only record once native is initialized.

Bug:  861954 
Change-Id: I424dc1c6d8d65a031c573375f3cf1686ec1f6614
Reviewed-on: https://chromium-review.googlesource.com/1138978
Commit-Queue: Tommy Steimel <steimel@chromium.org>
Reviewed-by: Mounir Lamouri <mlamouri@chromium.org>
Reviewed-by: Ilya Sherman <isherman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#577291}
[modify] https://crrev.com/8a874e08fcdfbfe28bfb7aaf2e176c943726b3e4/chrome/android/java/src/org/chromium/chrome/browser/media/MediaLauncherActivity.java
[modify] https://crrev.com/8a874e08fcdfbfe28bfb7aaf2e176c943726b3e4/chrome/android/java/src/org/chromium/chrome/browser/media/MediaViewerUtils.java
[modify] https://crrev.com/8a874e08fcdfbfe28bfb7aaf2e176c943726b3e4/tools/metrics/histograms/enums.xml
[modify] https://crrev.com/8a874e08fcdfbfe28bfb7aaf2e176c943726b3e4/tools/metrics/histograms/histograms.xml

Do we intend to add more metrics for M69?
I was planning on adding something for content provider, though that doesn't necessarily need to be M69
Labels: -Pri-1 Pri-2
Moving this to P2 to reflect comment above.
Labels: Merge-Request-69
Merge request for the CL in comment 4. Adds important metrics for the media intent handler launching in M69. Thanks!
Project Member

Comment 9 by sheriffbot@chromium.org, Jul 26

Labels: -Merge-Request-69 Merge-Review-69 Hotlist-Merge-Review
This bug requires manual review: Reverts referenced in bugdroid comments after merge request.
Please contact the milestone owner if you have questions.
Owners: amineer@(Android), kariahda@(iOS), cindyb@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: -Merge-Review-69 Merge-Approved-69
Project Member

Comment 11 by bugdroid1@chromium.org, Jul 26

Labels: -merge-approved-69 merge-merged-3497
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b62839f547e65a808143a017bd77110fa64ba72b

commit b62839f547e65a808143a017bd77110fa64ba72b
Author: Tommy Steimel <steimel@chromium.org>
Date: Thu Jul 26 18:56:52 2018

[Media Intent Handler] Reland add UMA metrics for media type

This CL re-adds media type UMA metrics to the MediaLauncherActivity. In
order to prevent the native initialization issues from the previous CL,
this CL uses CachedMetrics to only record once native is initialized.

Bug:  861954 
Change-Id: I424dc1c6d8d65a031c573375f3cf1686ec1f6614
Reviewed-on: https://chromium-review.googlesource.com/1138978
Commit-Queue: Tommy Steimel <steimel@chromium.org>
Reviewed-by: Mounir Lamouri <mlamouri@chromium.org>
Reviewed-by: Ilya Sherman <isherman@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#577291}(cherry picked from commit 8a874e08fcdfbfe28bfb7aaf2e176c943726b3e4)
Reviewed-on: https://chromium-review.googlesource.com/1151910
Reviewed-by: Tommy Steimel <steimel@chromium.org>
Cr-Commit-Position: refs/branch-heads/3497@{#121}
Cr-Branched-From: 271eaf50594eb818c9295dc78d364aea18c82ea8-refs/heads/master@{#576753}
[modify] https://crrev.com/b62839f547e65a808143a017bd77110fa64ba72b/chrome/android/java/src/org/chromium/chrome/browser/media/MediaLauncherActivity.java
[modify] https://crrev.com/b62839f547e65a808143a017bd77110fa64ba72b/chrome/android/java/src/org/chromium/chrome/browser/media/MediaViewerUtils.java
[modify] https://crrev.com/b62839f547e65a808143a017bd77110fa64ba72b/tools/metrics/histograms/enums.xml
[modify] https://crrev.com/b62839f547e65a808143a017bd77110fa64ba72b/tools/metrics/histograms/histograms.xml

Project Member

Comment 12 by bugdroid1@chromium.org, Sep 13

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

commit eef68f3460b417923cdd96bca9816cfb6b1448cf
Author: Tommy Steimel <steimel@chromium.org>
Date: Thu Sep 13 20:39:56 2018

[Media Intent Handler] Add MediaLauncherActivity LaunchSourceType

This CL adds a new LaunchSourceType enum to CustomTabIntentDataProvider
that matches the WebappActivity.ActivityType enum but adds a new value
for apps launched from MediaLauncherActivity. This is used to
distinguish intents from the MLA from other MEDIA_VIEWER sources (e.g.
the Downloads Home UI). This will allow us to add Media-Intent-Handler-
specific metrics.

Bug:  861954 
Change-Id: I01b20fec65e6207de3d24d42b9b08787a582adcb
Reviewed-on: https://chromium-review.googlesource.com/1185633
Reviewed-by: Peter Kotwicz <pkotwicz@chromium.org>
Reviewed-by: Ilya Sherman <isherman@chromium.org>
Reviewed-by: Peter Conn <peconn@chromium.org>
Reviewed-by: Ted Choc <tedchoc@chromium.org>
Reviewed-by: Mounir Lamouri <mlamouri@chromium.org>
Commit-Queue: Tommy Steimel <steimel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#591140}
[modify] https://crrev.com/eef68f3460b417923cdd96bca9816cfb6b1448cf/chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabActivity.java
[modify] https://crrev.com/eef68f3460b417923cdd96bca9816cfb6b1448cf/chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabIntentDataProvider.java
[modify] https://crrev.com/eef68f3460b417923cdd96bca9816cfb6b1448cf/chrome/android/java/src/org/chromium/chrome/browser/externalnav/ExternalNavigationHandler.java
[modify] https://crrev.com/eef68f3460b417923cdd96bca9816cfb6b1448cf/chrome/android/java/src/org/chromium/chrome/browser/media/MediaLauncherActivity.java
[modify] https://crrev.com/eef68f3460b417923cdd96bca9816cfb6b1448cf/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappCustomTabTimeSpentLogger.java
[modify] https://crrev.com/eef68f3460b417923cdd96bca9816cfb6b1448cf/chrome/android/java_sources.gni
[add] https://crrev.com/eef68f3460b417923cdd96bca9816cfb6b1448cf/chrome/android/javatests/src/org/chromium/chrome/browser/webapps/WebappActivityTest.java
[modify] https://crrev.com/eef68f3460b417923cdd96bca9816cfb6b1448cf/tools/metrics/histograms/histograms.xml

Is there any else to merge here?
Status: Fixed (was: Assigned)
I think we're good to close this

Sign in to add a comment