Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2539703002:
media: Add |bad_message_cb_| to MojoRendererService (Closed)
|
Created:
4 years ago by xhwang Modified:
4 years ago Reviewers:
tguilbert CC:
chromium-reviews, feature-media-reviews_chromium.org, qsr+mojo_chromium.org, viettrungluu+watch_chromium.org, yzshen+watch_chromium.org, abarth-chromium, Aaron Boodman, alokp+watch_chromium.org, darin (slow to review) Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
Descriptionmedia: Add |bad_message_cb_| to MojoRendererService
This helps move the StrongBindingPtr out of MojoRendererService, so that
we can bind MojoRendererService using other type of bindings, e.g.
mojo::Binding.
When we do use MojoRendererService::Create(), which uses
mojo::StrongBinding, now the StrongBindingPtr isimplicitly owned by the
MojoRendererService through the |bad_message_cb_|.
BUG=604912, 669606
TEST=Covered by media_mojo_unittests
Committed: https://crrev.com/a2db52064245983d71338e51d330edf256418e81
Cr-Commit-Position: refs/heads/master@{#435088}
Patch Set 1 #Patch Set 2 : add TODO and ref to bug #
Messages
Total messages: 27 (18 generated)
Description was changed from ========== media: Add |bad_message_cb_| to MojoRendererService This helps move the StrongBindingPtr out of MojoRendererService, so that we can bind MojoRendererService using other type of bindings, e.g. mojo::Binding. When we do use mojo::StrongBinding, now the StrongBindingPtr is implicitly owned by the MojoRendererService through the |bad_message_cb_|. BUG=604912 TEST=Covered by media_mojo_unittests ========== to ========== media: Add |bad_message_cb_| to MojoRendererService This helps move the StrongBindingPtr out of MojoRendererService, so that we can bind MojoRendererService using other type of bindings, e.g. mojo::Binding. When we do use MojoRendererService::Create(), which uses mojo::StrongBinding, now the StrongBindingPtr isimplicitly owned by the MojoRendererService through the |bad_message_cb_|. BUG=604912 TEST=Covered by media_mojo_unittests ==========
Description was changed from ========== media: Add |bad_message_cb_| to MojoRendererService This helps move the StrongBindingPtr out of MojoRendererService, so that we can bind MojoRendererService using other type of bindings, e.g. mojo::Binding. When we do use MojoRendererService::Create(), which uses mojo::StrongBinding, now the StrongBindingPtr isimplicitly owned by the MojoRendererService through the |bad_message_cb_|. BUG=604912 TEST=Covered by media_mojo_unittests ========== to ========== media: Add |bad_message_cb_| to MojoRendererService This helps move the StrongBindingPtr out of MojoRendererService, so that we can bind MojoRendererService using other type of bindings, e.g. mojo::Binding. When we do use MojoRendererService::Create(), which uses mojo::StrongBinding, now the StrongBindingPtr isimplicitly owned by the MojoRendererService through the |bad_message_cb_|. BUG=604912 TEST=Covered by media_mojo_unittests ==========
The CQ bit was checked by xhwang@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.
xhwang@chromium.org changed reviewers: + tguilbert@chromium.org
PTAL. You can see how we set the callback when using mojo::Binding (instead of StrongBinding) at https://chromiumcodereview.appspot.com/2530613003/
On 2016/11/29 10:22:29, xhwang wrote: > PTAL. > > You can see how we set the callback when using mojo::Binding (instead of > StrongBinding) at https://chromiumcodereview.appspot.com/2530613003/ LGTM Discussed offline with xhwang@. I will open a bug to capture our discussion (which is just that we agree on the idea he proposed in this comment https://chromiumcodereview.appspot.com/2530613003/?_ga=1.123982018.997515271....). I will update this CL with the bug#.
Description was changed from ========== media: Add |bad_message_cb_| to MojoRendererService This helps move the StrongBindingPtr out of MojoRendererService, so that we can bind MojoRendererService using other type of bindings, e.g. mojo::Binding. When we do use MojoRendererService::Create(), which uses mojo::StrongBinding, now the StrongBindingPtr isimplicitly owned by the MojoRendererService through the |bad_message_cb_|. BUG=604912 TEST=Covered by media_mojo_unittests ========== to ========== media: Add |bad_message_cb_| to MojoRendererService This helps move the StrongBindingPtr out of MojoRendererService, so that we can bind MojoRendererService using other type of bindings, e.g. mojo::Binding. When we do use MojoRendererService::Create(), which uses mojo::StrongBinding, now the StrongBindingPtr isimplicitly owned by the MojoRendererService through the |bad_message_cb_|. BUG=604912,669606 TEST=Covered by media_mojo_unittests ==========
The CQ bit was checked by xhwang@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
On 2016/11/29 20:13:12, commit-bot: I haz the power wrote: > CQ is trying da patch. Follow status at > > https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or... I meant to add a reference to the bug. crbug.com/669606
On 2016/11/29 20:13:12, commit-bot: I haz the power wrote: > CQ is trying da patch. Follow status at > > https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or... I meant to add a reference to the bug. crbug.com/669606
The CQ bit was unchecked by xhwang@chromium.org
On 2016/11/29 20:15:13, tguilbert wrote: > On 2016/11/29 20:13:12, commit-bot: I haz the power wrote: > > CQ is trying da patch. Follow status at > > > > > https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or... > > I meant to add a reference to the bug. crbug.com/669606 Done.
The CQ bit was checked by xhwang@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 xhwang@chromium.org
The CQ bit was checked by xhwang@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from tguilbert@chromium.org Link to the patchset: https://codereview.chromium.org/2539703002/#ps20001 (title: "add TODO and ref to bug")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 20001, "attempt_start_ts": 1480454487676320,
"parent_rev": "3b99381f4fb02ee020a5d963d8f8ad6e2a719253", "commit_rev":
"7855022f78c33224c3065b6ff5bcad3b9c331052"}
Message was sent while issue was closed.
Description was changed from ========== media: Add |bad_message_cb_| to MojoRendererService This helps move the StrongBindingPtr out of MojoRendererService, so that we can bind MojoRendererService using other type of bindings, e.g. mojo::Binding. When we do use MojoRendererService::Create(), which uses mojo::StrongBinding, now the StrongBindingPtr isimplicitly owned by the MojoRendererService through the |bad_message_cb_|. BUG=604912,669606 TEST=Covered by media_mojo_unittests ========== to ========== media: Add |bad_message_cb_| to MojoRendererService This helps move the StrongBindingPtr out of MojoRendererService, so that we can bind MojoRendererService using other type of bindings, e.g. mojo::Binding. When we do use MojoRendererService::Create(), which uses mojo::StrongBinding, now the StrongBindingPtr isimplicitly owned by the MojoRendererService through the |bad_message_cb_|. BUG=604912,669606 TEST=Covered by media_mojo_unittests ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
Description was changed from ========== media: Add |bad_message_cb_| to MojoRendererService This helps move the StrongBindingPtr out of MojoRendererService, so that we can bind MojoRendererService using other type of bindings, e.g. mojo::Binding. When we do use MojoRendererService::Create(), which uses mojo::StrongBinding, now the StrongBindingPtr isimplicitly owned by the MojoRendererService through the |bad_message_cb_|. BUG=604912,669606 TEST=Covered by media_mojo_unittests ========== to ========== media: Add |bad_message_cb_| to MojoRendererService This helps move the StrongBindingPtr out of MojoRendererService, so that we can bind MojoRendererService using other type of bindings, e.g. mojo::Binding. When we do use MojoRendererService::Create(), which uses mojo::StrongBinding, now the StrongBindingPtr isimplicitly owned by the MojoRendererService through the |bad_message_cb_|. BUG=604912,669606 TEST=Covered by media_mojo_unittests Committed: https://crrev.com/a2db52064245983d71338e51d330edf256418e81 Cr-Commit-Position: refs/heads/master@{#435088} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/a2db52064245983d71338e51d330edf256418e81 Cr-Commit-Position: refs/heads/master@{#435088} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
