New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 789447 link

Starred by 1 user
Status: Fixed
Owner:
liber...@chromium.org
Closed: Mar 2018
Cc:
█ w...@chromium.org
sande...@chromium.org
xhw...@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Bug

Blocked on:
issue 783973


Sign in to add a comment

Remove service_manager::ServiceContextRef usage in MediaCodecVideoDecoder

Project Member Reported by xhw...@chromium.org, Nov 29 2017 
Currently MediaCodecVideoDecoder holds a service_manager::ServiceContextRef which seems like a layer violation because MCVD should not care about mojo services. I guess this is because we want to make sure the service thread is not destroyed prematurely (see  issue 783973  for the context).

Now  issue 783973  is fixed, and the service runs on a sequence, we should be able to just hold a ref to the task runner instead of using service_manager::ServiceContextRef.
Project Member

Comment 1 by bugdroid1@chromium.org, Dec 1 2017 

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

commit 9a635ee7853ee307ccfd6a9ad4f0de09ec2f4616
Author: liberato@chromium.org <liberato@chromium.org>
Date: Fri Dec 01 21:38:50 2017

Remove ServiceContextRef from MCVD.

Previously, MCVD ran on a thread that was owned by the mojo service.
If the service was destroyed, then the thread would be also.  This
made it difficult for things to outlive MCVD (e.g., overlays).

Now, it is sufficient to hold the task runner instead.  This CL
removes the ServiceContextRef, in favor of the task runner.

Bug:  789447 
Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: I1f07382a3977685d6a92f6f13a92d5455602d6ec
Reviewed-on: https://chromium-review.googlesource.com/801334
Reviewed-by: Sergey Volk <servolk@chromium.org>
Reviewed-by: Kenneth Russell <kbr@chromium.org>
Reviewed-by: Chris Watkins <watk@chromium.org>
Reviewed-by: Xiaohan Wang <xhwang@chromium.org>
Reviewed-by: Dan Sanders <sandersd@chromium.org>
Commit-Queue: Frank Liberato <liberato@chromium.org>
Cr-Commit-Position: refs/heads/master@{#521067}
[modify] https://crrev.com/9a635ee7853ee307ccfd6a9ad4f0de09ec2f4616/chromecast/media/service/cast_mojo_media_client.cc
[modify] https://crrev.com/9a635ee7853ee307ccfd6a9ad4f0de09ec2f4616/chromecast/media/service/cast_mojo_media_client.h
[modify] https://crrev.com/9a635ee7853ee307ccfd6a9ad4f0de09ec2f4616/content/gpu/gpu_child_thread.cc
[modify] https://crrev.com/9a635ee7853ee307ccfd6a9ad4f0de09ec2f4616/content/gpu/gpu_child_thread.h
[modify] https://crrev.com/9a635ee7853ee307ccfd6a9ad4f0de09ec2f4616/media/base/android_overlay_mojo_factory.h
[modify] https://crrev.com/9a635ee7853ee307ccfd6a9ad4f0de09ec2f4616/media/gpu/android/android_video_decode_accelerator.cc
[modify] https://crrev.com/9a635ee7853ee307ccfd6a9ad4f0de09ec2f4616/media/gpu/android/codec_wrapper_unittest.cc
[modify] https://crrev.com/9a635ee7853ee307ccfd6a9ad4f0de09ec2f4616/media/gpu/android/media_codec_video_decoder.cc
[modify] https://crrev.com/9a635ee7853ee307ccfd6a9ad4f0de09ec2f4616/media/gpu/android/media_codec_video_decoder.h
[modify] https://crrev.com/9a635ee7853ee307ccfd6a9ad4f0de09ec2f4616/media/gpu/android/media_codec_video_decoder_unittest.cc
[modify] https://crrev.com/9a635ee7853ee307ccfd6a9ad4f0de09ec2f4616/media/mojo/clients/mojo_android_overlay.cc
[modify] https://crrev.com/9a635ee7853ee307ccfd6a9ad4f0de09ec2f4616/media/mojo/clients/mojo_android_overlay.h
[modify] https://crrev.com/9a635ee7853ee307ccfd6a9ad4f0de09ec2f4616/media/mojo/clients/mojo_android_overlay_unittest.cc
[modify] https://crrev.com/9a635ee7853ee307ccfd6a9ad4f0de09ec2f4616/media/mojo/services/gpu_mojo_media_client.cc
[modify] https://crrev.com/9a635ee7853ee307ccfd6a9ad4f0de09ec2f4616/media/mojo/services/gpu_mojo_media_client.h
[modify] https://crrev.com/9a635ee7853ee307ccfd6a9ad4f0de09ec2f4616/media/mojo/services/media_service.cc
[modify] https://crrev.com/9a635ee7853ee307ccfd6a9ad4f0de09ec2f4616/media/mojo/services/mojo_media_client.cc
[modify] https://crrev.com/9a635ee7853ee307ccfd6a9ad4f0de09ec2f4616/media/mojo/services/mojo_media_client.h
[modify] https://crrev.com/9a635ee7853ee307ccfd6a9ad4f0de09ec2f4616/media/mojo/services/test_mojo_media_client.cc
[modify] https://crrev.com/9a635ee7853ee307ccfd6a9ad4f0de09ec2f4616/media/mojo/services/test_mojo_media_client.h

Comment 2 by liber...@chromium.org, Mar 8 2018

Status: Fixed (was: Assigned)
►

Sign in to add a comment