Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2646793006:
[Media>UI] Don't show media metadata on lockscreen in Incognito mode (Closed)
|
Created:
3 years, 11 months ago by Zhiqiang Zhang (Slow) Modified:
3 years, 11 months ago Reviewers:
whywhat CC:
agrieve+watch_chromium.org, chromium-reviews, feature-media-reviews_chromium.org Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
Description[Media>UI] Don't show media metadata on lockscreen in Incognito mode
Media notification content is marked as private in Incognito mode, so the
content is hidden on lockscreen. For the same reason, we shouldn't show the
artwork image on lockscreen and wearable devices.
BUG=683111
Review-Url: https://codereview.chromium.org/2646793006
Cr-Commit-Position: refs/heads/master@{#445745}
Committed: https://chromium.googlesource.com/chromium/src/+/233f53ad273ef485ff5b12510b421a5ab4d429ec
Patch Set 1 #Patch Set 2 : don't show metadata at all #Patch Set 3 : don't show notification on wearable at all #
Total comments: 1
Patch Set 4 : addressed nit #Patch Set 5 : falling back to remove all metadata #Messages
Total messages: 25 (15 generated)
zqzhang@chromium.org changed reviewers: + avayvod@chromium.org
PTAL Another one after branch point :(
Description was changed from ========== [Media>UI] Don't show media artwork on lockscreen in Incognito mode Media notification content is marked as private in Incognito mode, so the content is hidden on lockscreen. For the same reason, we shouldn't show the artwork image on lockscreen. BUG=683111 ========== to ========== [Media>UI] Don't show media artwork on lockscreen in Incognito mode Media notification content is marked as private in Incognito mode, so the content is hidden on lockscreen. For the same reason, we shouldn't show the artwork image on lockscreen. BUG=683111 ==========
this change lg do you mean to add the Wear logic here as discussed offline?
On 2017/01/20 15:37:54, whywhat wrote: > this change lg > do you mean to add the Wear logic here as discussed offline? Yes :)
Description was changed from ========== [Media>UI] Don't show media artwork on lockscreen in Incognito mode Media notification content is marked as private in Incognito mode, so the content is hidden on lockscreen. For the same reason, we shouldn't show the artwork image on lockscreen. BUG=683111 ========== to ========== [Media>UI] Don't show media metadata on lockscreen in Incognito mode Media notification content is marked as private in Incognito mode, so the content is hidden on lockscreen. For the same reason, we shouldn't show the artwork image on lockscreen and wearable devices. BUG=683111 ==========
PTAL
The CQ bit was checked by zqzhang@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Hm, I'd rather check for isPrivate before calling setMetadata on MediaSession and leave the method as non-nullable. It seems that setMetadata is tolerable to null from the first glance at the MediaSessionCompat code but it gets passed around to some callbacks so I wouldn't bet on it. lgtm if you apply this change :)
FYI I also prepared patch #3 which disables all wearable notification in Incognito mode. I'm confirming with dahlke on the bug and will land it after decision :)
https://codereview.chromium.org/2646793006/diff/40001/chrome/android/java/src... File chrome/android/java/src/org/chromium/chrome/browser/media/ui/MediaNotificationManager.java (right): https://codereview.chromium.org/2646793006/diff/40001/chrome/android/java/src... chrome/android/java/src/org/chromium/chrome/browser/media/ui/MediaNotificationManager.java:672: @Nullable nit: remove?
The CQ bit was checked by zqzhang@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by zqzhang@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from avayvod@chromium.org Link to the patchset: https://codereview.chromium.org/2646793006/#ps80001 (title: "falling back to remove all metadata")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Landing the "remove all metadata" solution as we reached decision on the bug :)
CQ is committing da patch.
Bot data: {"patchset_id": 80001, "attempt_start_ts": 1485277239850040,
"parent_rev": "f2bb61c5104755025adb0f1c5327de420311d32e", "commit_rev":
"233f53ad273ef485ff5b12510b421a5ab4d429ec"}
Message was sent while issue was closed.
Description was changed from ========== [Media>UI] Don't show media metadata on lockscreen in Incognito mode Media notification content is marked as private in Incognito mode, so the content is hidden on lockscreen. For the same reason, we shouldn't show the artwork image on lockscreen and wearable devices. BUG=683111 ========== to ========== [Media>UI] Don't show media metadata on lockscreen in Incognito mode Media notification content is marked as private in Incognito mode, so the content is hidden on lockscreen. For the same reason, we shouldn't show the artwork image on lockscreen and wearable devices. BUG=683111 Review-Url: https://codereview.chromium.org/2646793006 Cr-Commit-Position: refs/heads/master@{#445745} Committed: https://chromium.googlesource.com/chromium/src/+/233f53ad273ef485ff5b12510b42... ==========
Message was sent while issue was closed.
Committed patchset #5 (id:80001) as https://chromium.googlesource.com/chromium/src/+/233f53ad273ef485ff5b12510b42... |
