Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1648653002:
Carry WebMediaSession in WebMediaPlayerParams (Closed)
|
Created:
4 years, 10 months ago by davve Modified:
4 years, 10 months ago Reviewers:
whywhat, philipj_slow, jochen (gone - plz use gerrit), DaleCurtis, mlamouri (slow - plz ping) CC:
chromium-reviews, creis+watch_chromium.org, darin-cc_chromium.org, feature-media-reviews_chromium.org, jam, mkwst+moarreviews-renderer_chromium.org, mlamouri+watch-content_chromium.org, nasko+codewatch_chromium.org Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionCarry WebMediaSession in WebMediaPlayerParams
While general, this is Android focused in the short-term. It prepares
WebMediaPlayerAndroid for propagating its media session id, if any,
over IPC to the browser process where the media session integration
with the platform happens.
BUG=497735, 581728
Committed: https://crrev.com/9db17a3c0461e8d0119a2594fdb142c543dcbdee
Cr-Commit-Position: refs/heads/master@{#376707}
Patch Set 1 #
Total comments: 2
Patch Set 2 : Rebase; add const to getter #
Messages
Total messages: 29 (14 generated)
Description was changed from ========== Carry media session in webmediaplayer_params ========== to ========== Carry media session in WebMediaPlayerParams ==========
Description was changed from ========== Carry media session in WebMediaPlayerParams ========== to ========== Carry WebMediaSession in WebMediaPlayerParams This is preparation for letting WebMediaPlayerAndroid propagate the its session id over IPC to the browser process. BUG=497735 ==========
Description was changed from ========== Carry WebMediaSession in WebMediaPlayerParams This is preparation for letting WebMediaPlayerAndroid propagate the its session id over IPC to the browser process. BUG=497735 ========== to ========== Carry WebMediaSession in WebMediaPlayerParams While general, this is Android focused short-term. It prepares WebMediaPlayerAndroid for propagating its media session id, if any, over IPC to the browser process where the media session integration with the platform happens. BUG=497735, 581728 ==========
Description was changed from ========== Carry WebMediaSession in WebMediaPlayerParams While general, this is Android focused short-term. It prepares WebMediaPlayerAndroid for propagating its media session id, if any, over IPC to the browser process where the media session integration with the platform happens. BUG=497735, 581728 ========== to ========== Carry WebMediaSession in WebMediaPlayerParams While general, this is Android focused in the short-term. It prepares WebMediaPlayerAndroid for propagating its media session id, if any, over IPC to the browser process where the media session integration with the platform happens. BUG=497735, 581728 ==========
davve@opera.com changed reviewers: + avayvod@chromium.org, jochen@chromium.org, mlamouri@chromium.org, philipj@opera.com
Not sure who of you that's around atm, but PTAL if you have a moment.
Non-owner LGTM
lgtm
lgtm
The CQ bit was checked by davve@opera.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1648653002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1648653002/1
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: ios_dbg_simulator_ninja on tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...) mac_chromium_gn_rel on tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_gn_r...) mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
https://codereview.chromium.org/1648653002/diff/1/media/blink/webmediaplayer_... File media/blink/webmediaplayer_params.h (right): https://codereview.chromium.org/1648653002/diff/1/media/blink/webmediaplayer_... media/blink/webmediaplayer_params.h:100: blink::WebMediaSession* media_session() const { return media_session_; } nit: returning a non-const object from a const method usually makes me suspicious but it seems to be a common pattern around WebMediaPlayerParams here :/
https://codereview.chromium.org/1648653002/diff/1/media/blink/webmediaplayer_... File media/blink/webmediaplayer_params.h (right): https://codereview.chromium.org/1648653002/diff/1/media/blink/webmediaplayer_... media/blink/webmediaplayer_params.h:100: blink::WebMediaSession* media_session() const { return media_session_; } On 2016/02/17 16:52:02, whywhat wrote: > nit: returning a non-const object from a const method usually makes me > suspicious but it seems to be a common pattern around WebMediaPlayerParams here > :/ Returning a const WebMediaSession sounds fine to me, at least for now.
The CQ bit was checked by davve@opera.com
The patchset sent to the CQ was uploaded after l-g-t-m from philipj@opera.com, jochen@chromium.org, mlamouri@chromium.org Link to the patchset: https://codereview.chromium.org/1648653002/#ps20001 (title: "Rebase; add const to getter")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1648653002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1648653002/20001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: chromium_presubmit on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
davve@opera.com changed reviewers: + dalecurtis@chromium.org
+dalecurtis for media/
lgtm, but be aware WMPA will be deleted over the next few milestones.
The CQ bit was checked by davve@opera.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1648653002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1648653002/20001
Message was sent while issue was closed.
Description was changed from ========== Carry WebMediaSession in WebMediaPlayerParams While general, this is Android focused in the short-term. It prepares WebMediaPlayerAndroid for propagating its media session id, if any, over IPC to the browser process where the media session integration with the platform happens. BUG=497735, 581728 ========== to ========== Carry WebMediaSession in WebMediaPlayerParams While general, this is Android focused in the short-term. It prepares WebMediaPlayerAndroid for propagating its media session id, if any, over IPC to the browser process where the media session integration with the platform happens. BUG=497735, 581728 ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
Description was changed from ========== Carry WebMediaSession in WebMediaPlayerParams While general, this is Android focused in the short-term. It prepares WebMediaPlayerAndroid for propagating its media session id, if any, over IPC to the browser process where the media session integration with the platform happens. BUG=497735, 581728 ========== to ========== Carry WebMediaSession in WebMediaPlayerParams While general, this is Android focused in the short-term. It prepares WebMediaPlayerAndroid for propagating its media session id, if any, over IPC to the browser process where the media session integration with the platform happens. BUG=497735, 581728 Committed: https://crrev.com/9db17a3c0461e8d0119a2594fdb142c543dcbdee Cr-Commit-Position: refs/heads/master@{#376707} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/9db17a3c0461e8d0119a2594fdb142c543dcbdee Cr-Commit-Position: refs/heads/master@{#376707} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
