New issue
Advanced search Search tips

Issue 842277 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: May 2018
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

Add a helper to create null callbacks

Project Member Reported by xhw...@chromium.org, May 11 2018

Issue description

Currently there are many places that null callbacks can be passed, therefore, in some code, especially test code, we need to pass in a null callback. Currently this can be done by creating an null/empty callback directly, but there are two minor issues:
1. The callback name can be long, e.g. AudioDecoder::WaitingForDecryptionKeyCB()
2. It's harder to read. To me it's not super intuitive that a default constructed object is a null object.

I am proposing to introduce a base::NullCallback which can be implicitly converted to any null callback, similar to how base::DoNothing works.
 
Project Member

Comment 2 by bugdroid1@chromium.org, May 14 2018

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

commit 4f568e77d832f87600f679d17396e7cc4d464d05
Author: Xiaohan Wang <xhwang@chromium.org>
Date: Mon May 14 22:36:04 2018

base: Introduce NullCallback

This helper makes it easier to pass in null callbacks by avoiding the
long callback names.

Also, base::NullCallback() would be more explicit that the callback is
null than FooCallback().

This CL only converts *Decoder::WaitingForDecryptionKeyCB() to use
base::NullCallback() as an example. More callbacks will be converted
later.

Bug:  842277 
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel
Change-Id: I03bdf9d3ff6be30161b9e24118827377f5350d40
Reviewed-on: https://chromium-review.googlesource.com/1055854
Reviewed-by: Taiju Tsuiki <tzik@chromium.org>
Commit-Queue: Xiaohan Wang <xhwang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#558494}
[modify] https://crrev.com/4f568e77d832f87600f679d17396e7cc4d464d05/base/bind_helpers.h
[modify] https://crrev.com/4f568e77d832f87600f679d17396e7cc4d464d05/media/cast/sender/h264_vt_encoder_unittest.cc
[modify] https://crrev.com/4f568e77d832f87600f679d17396e7cc4d464d05/media/filters/aom_video_decoder_unittest.cc
[modify] https://crrev.com/4f568e77d832f87600f679d17396e7cc4d464d05/media/filters/audio_decoder_unittest.cc
[modify] https://crrev.com/4f568e77d832f87600f679d17396e7cc4d464d05/media/filters/fake_video_decoder_unittest.cc
[modify] https://crrev.com/4f568e77d832f87600f679d17396e7cc4d464d05/media/filters/ffmpeg_video_decoder_unittest.cc
[modify] https://crrev.com/4f568e77d832f87600f679d17396e7cc4d464d05/media/filters/offloading_video_decoder_unittest.cc
[modify] https://crrev.com/4f568e77d832f87600f679d17396e7cc4d464d05/media/filters/vpx_video_decoder_fuzzertest.cc
[modify] https://crrev.com/4f568e77d832f87600f679d17396e7cc4d464d05/media/filters/vpx_video_decoder_unittest.cc
[modify] https://crrev.com/4f568e77d832f87600f679d17396e7cc4d464d05/media/gpu/android/media_codec_video_decoder_unittest.cc
[modify] https://crrev.com/4f568e77d832f87600f679d17396e7cc4d464d05/media/gpu/video_encode_accelerator_unittest.cc
[modify] https://crrev.com/4f568e77d832f87600f679d17396e7cc4d464d05/media/gpu/windows/d3d11_video_decoder_unittest.cc
[modify] https://crrev.com/4f568e77d832f87600f679d17396e7cc4d464d05/media/mojo/clients/mojo_audio_decoder_unittest.cc
[modify] https://crrev.com/4f568e77d832f87600f679d17396e7cc4d464d05/media/mojo/services/mojo_audio_decoder_service.cc
[modify] https://crrev.com/4f568e77d832f87600f679d17396e7cc4d464d05/media/mojo/services/mojo_video_decoder_service.cc
[modify] https://crrev.com/4f568e77d832f87600f679d17396e7cc4d464d05/media/mojo/test/mojo_video_decoder_integration_test.cc

Comment 3 by xhw...@chromium.org, May 14 2018

Status: Fixed (was: Assigned)

Sign in to add a comment