Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2050323002:
media: Fix mojo DecoderBuffer type converter (Closed)
|
Created:
4 years, 6 months ago by xhwang Modified:
4 years, 6 months ago CC:
chromium-reviews, feature-media-reviews_chromium.org, qsr+mojo_chromium.org, viettrungluu+watch_chromium.org, yzshen+watch_chromium.org, abarth-chromium, Aaron Boodman, darin (slow to review), ben+mojo_chromium.org Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
Descriptionmedia: Fix mojo DecoderBuffer type converter
When side_data is empty, we should not called front(), otherwise it's undefined behavior:
https://cs.chromium.org/chromium/src/mojo/public/cpp/bindings/array.h?rcl=0&l=97
The crash is like this:
#2 0x00007ffff442dfe5 in __gnu_debug::_Error_formatter::_M_error (this=0x7fffffffc058)
at ../../../../../src/libstdc++-v3/src/c++11/debug.cc:781
#3 0x0000000000550062 in std::__debug::vector<unsigned char, std::allocator<unsigned char> >::front
(this=0x362504b6f3a8)
at ../../build/linux/debian_wheezy_amd64-sysroot/usr/lib/gcc/x86_64-linux-gnu/4.6/../../../../in
clude/c++/4.6/debug/vector:329
#4 0x000000000054d905 in mojo::Array<unsigned char>::front (this=0x362504b6f3a8)
at ../../mojo/public/cpp/bindings/array.h:99
#5 0x000000000054b9b4 in mojo::TypeConverter<scoped_refptr<media::DecoderBuffer>, mojo::StructPtr<m
edia::mojom::DecoderBuffer> >::Convert (input=...)
at ../../media/mojo/common/media_type_converters.cc:476
I'll enable media_mojo_unittests on bots shortly so that we can catch
regressions.
BUG=617204
TEST=fixes a unittest
Committed: https://crrev.com/03fa1fc9ad9bc265dcebb93b0e8d5a47f764b8a6
Cr-Commit-Position: refs/heads/master@{#399054}
Patch Set 1 #
Messages
Total messages: 23 (10 generated)
xhwang@chromium.org changed reviewers: + sandersd@chromium.org
PTAL
On 2016/06/09 17:48:27, xhwang wrote: > PTAL Can you comment in the description what the bug actually is? It's not obvious from the code that there even is a bug.
Description was changed from ========== media: Fix mojo DecoderBuffer type converter I'll enable media_mojo_unittests on bots shortly so that we can catch regressions. BUG=617204 TEST=fixes a unittest ========== to ========== media: Fix mojo DecoderBuffer type converter When side_data is empty, we should not called front(), otherwise it's undefined behavior: https://cs.chromium.org/chromium/src/mojo/public/cpp/bindings/array.h?rcl=0&l=97 #2 0x00007ffff442dfe5 in __gnu_debug::_Error_formatter::_M_error (this=0x7fffffffc058) at ../../../../../src/libstdc++-v3/src/c++11/debug.cc:781 #3 0x0000000000550062 in std::__debug::vector<unsigned char, std::allocator<unsigned char> >::front (this=0x362504b6f3a8) at ../../build/linux/debian_wheezy_amd64-sysroot/usr/lib/gcc/x86_64-linux-gnu/4.6/../../../../in clude/c++/4.6/debug/vector:329 #4 0x000000000054d905 in mojo::Array<unsigned char>::front (this=0x362504b6f3a8) at ../../mojo/public/cpp/bindings/array.h:99 #5 0x000000000054b9b4 in mojo::TypeConverter<scoped_refptr<media::DecoderBuffer>, mojo::StructPtr<m edia::mojom::DecoderBuffer> >::Convert (input=...) at ../../media/mojo/common/media_type_converters.cc:476 I'll enable media_mojo_unittests on bots shortly so that we can catch regressions. BUG=617204 TEST=fixes a unittest ==========
Description was changed from ========== media: Fix mojo DecoderBuffer type converter When side_data is empty, we should not called front(), otherwise it's undefined behavior: https://cs.chromium.org/chromium/src/mojo/public/cpp/bindings/array.h?rcl=0&l=97 #2 0x00007ffff442dfe5 in __gnu_debug::_Error_formatter::_M_error (this=0x7fffffffc058) at ../../../../../src/libstdc++-v3/src/c++11/debug.cc:781 #3 0x0000000000550062 in std::__debug::vector<unsigned char, std::allocator<unsigned char> >::front (this=0x362504b6f3a8) at ../../build/linux/debian_wheezy_amd64-sysroot/usr/lib/gcc/x86_64-linux-gnu/4.6/../../../../in clude/c++/4.6/debug/vector:329 #4 0x000000000054d905 in mojo::Array<unsigned char>::front (this=0x362504b6f3a8) at ../../mojo/public/cpp/bindings/array.h:99 #5 0x000000000054b9b4 in mojo::TypeConverter<scoped_refptr<media::DecoderBuffer>, mojo::StructPtr<m edia::mojom::DecoderBuffer> >::Convert (input=...) at ../../media/mojo/common/media_type_converters.cc:476 I'll enable media_mojo_unittests on bots shortly so that we can catch regressions. BUG=617204 TEST=fixes a unittest ========== to ========== media: Fix mojo DecoderBuffer type converter When side_data is empty, we should not called front(), otherwise it's undefined behavior: https://cs.chromium.org/chromium/src/mojo/public/cpp/bindings/array.h?rcl=0&l=97 The crash is like this: #2 0x00007ffff442dfe5 in __gnu_debug::_Error_formatter::_M_error (this=0x7fffffffc058) at ../../../../../src/libstdc++-v3/src/c++11/debug.cc:781 #3 0x0000000000550062 in std::__debug::vector<unsigned char, std::allocator<unsigned char> >::front (this=0x362504b6f3a8) at ../../build/linux/debian_wheezy_amd64-sysroot/usr/lib/gcc/x86_64-linux-gnu/4.6/../../../../in clude/c++/4.6/debug/vector:329 #4 0x000000000054d905 in mojo::Array<unsigned char>::front (this=0x362504b6f3a8) at ../../mojo/public/cpp/bindings/array.h:99 #5 0x000000000054b9b4 in mojo::TypeConverter<scoped_refptr<media::DecoderBuffer>, mojo::StructPtr<m edia::mojom::DecoderBuffer> >::Convert (input=...) at ../../media/mojo/common/media_type_converters.cc:476 I'll enable media_mojo_unittests on bots shortly so that we can catch regressions. BUG=617204 TEST=fixes a unittest ==========
On 2016/06/09 17:51:30, sandersd wrote: > On 2016/06/09 17:48:27, xhwang wrote: > > PTAL > > Can you comment in the description what the bug actually is? It's not obvious > from the code that there even is a bug. Done.
lgtm
xhwang@chromium.org changed reviewers: + dcheng@chromium.org
dcheng@chromium.org: Please OWNERS review this minor change. As promised, I'll enable this test on bots shortly (this week).
Yikes. This fix LGTM; when can we switch this over to StructTraits =)
On 2016/06/09 18:25:42, dcheng wrote: > Yikes. This fix LGTM; when can we switch this over to StructTraits =) That's scheduled, but we are lacking resources to do it :(
The CQ bit was checked by xhwang@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2050323002/1
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: win_chromium_rel_ng on tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
The CQ bit was checked by xhwang@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2050323002/1
Message was sent while issue was closed.
Description was changed from ========== media: Fix mojo DecoderBuffer type converter When side_data is empty, we should not called front(), otherwise it's undefined behavior: https://cs.chromium.org/chromium/src/mojo/public/cpp/bindings/array.h?rcl=0&l=97 The crash is like this: #2 0x00007ffff442dfe5 in __gnu_debug::_Error_formatter::_M_error (this=0x7fffffffc058) at ../../../../../src/libstdc++-v3/src/c++11/debug.cc:781 #3 0x0000000000550062 in std::__debug::vector<unsigned char, std::allocator<unsigned char> >::front (this=0x362504b6f3a8) at ../../build/linux/debian_wheezy_amd64-sysroot/usr/lib/gcc/x86_64-linux-gnu/4.6/../../../../in clude/c++/4.6/debug/vector:329 #4 0x000000000054d905 in mojo::Array<unsigned char>::front (this=0x362504b6f3a8) at ../../mojo/public/cpp/bindings/array.h:99 #5 0x000000000054b9b4 in mojo::TypeConverter<scoped_refptr<media::DecoderBuffer>, mojo::StructPtr<m edia::mojom::DecoderBuffer> >::Convert (input=...) at ../../media/mojo/common/media_type_converters.cc:476 I'll enable media_mojo_unittests on bots shortly so that we can catch regressions. BUG=617204 TEST=fixes a unittest ========== to ========== media: Fix mojo DecoderBuffer type converter When side_data is empty, we should not called front(), otherwise it's undefined behavior: https://cs.chromium.org/chromium/src/mojo/public/cpp/bindings/array.h?rcl=0&l=97 The crash is like this: #2 0x00007ffff442dfe5 in __gnu_debug::_Error_formatter::_M_error (this=0x7fffffffc058) at ../../../../../src/libstdc++-v3/src/c++11/debug.cc:781 #3 0x0000000000550062 in std::__debug::vector<unsigned char, std::allocator<unsigned char> >::front (this=0x362504b6f3a8) at ../../build/linux/debian_wheezy_amd64-sysroot/usr/lib/gcc/x86_64-linux-gnu/4.6/../../../../in clude/c++/4.6/debug/vector:329 #4 0x000000000054d905 in mojo::Array<unsigned char>::front (this=0x362504b6f3a8) at ../../mojo/public/cpp/bindings/array.h:99 #5 0x000000000054b9b4 in mojo::TypeConverter<scoped_refptr<media::DecoderBuffer>, mojo::StructPtr<m edia::mojom::DecoderBuffer> >::Convert (input=...) at ../../media/mojo/common/media_type_converters.cc:476 I'll enable media_mojo_unittests on bots shortly so that we can catch regressions. BUG=617204 TEST=fixes a unittest ==========
Message was sent while issue was closed.
Committed patchset #1 (id:1)
Message was sent while issue was closed.
CQ bit was unchecked
Message was sent while issue was closed.
Description was changed from ========== media: Fix mojo DecoderBuffer type converter When side_data is empty, we should not called front(), otherwise it's undefined behavior: https://cs.chromium.org/chromium/src/mojo/public/cpp/bindings/array.h?rcl=0&l=97 The crash is like this: #2 0x00007ffff442dfe5 in __gnu_debug::_Error_formatter::_M_error (this=0x7fffffffc058) at ../../../../../src/libstdc++-v3/src/c++11/debug.cc:781 #3 0x0000000000550062 in std::__debug::vector<unsigned char, std::allocator<unsigned char> >::front (this=0x362504b6f3a8) at ../../build/linux/debian_wheezy_amd64-sysroot/usr/lib/gcc/x86_64-linux-gnu/4.6/../../../../in clude/c++/4.6/debug/vector:329 #4 0x000000000054d905 in mojo::Array<unsigned char>::front (this=0x362504b6f3a8) at ../../mojo/public/cpp/bindings/array.h:99 #5 0x000000000054b9b4 in mojo::TypeConverter<scoped_refptr<media::DecoderBuffer>, mojo::StructPtr<m edia::mojom::DecoderBuffer> >::Convert (input=...) at ../../media/mojo/common/media_type_converters.cc:476 I'll enable media_mojo_unittests on bots shortly so that we can catch regressions. BUG=617204 TEST=fixes a unittest ========== to ========== media: Fix mojo DecoderBuffer type converter When side_data is empty, we should not called front(), otherwise it's undefined behavior: https://cs.chromium.org/chromium/src/mojo/public/cpp/bindings/array.h?rcl=0&l=97 The crash is like this: #2 0x00007ffff442dfe5 in __gnu_debug::_Error_formatter::_M_error (this=0x7fffffffc058) at ../../../../../src/libstdc++-v3/src/c++11/debug.cc:781 #3 0x0000000000550062 in std::__debug::vector<unsigned char, std::allocator<unsigned char> >::front (this=0x362504b6f3a8) at ../../build/linux/debian_wheezy_amd64-sysroot/usr/lib/gcc/x86_64-linux-gnu/4.6/../../../../in clude/c++/4.6/debug/vector:329 #4 0x000000000054d905 in mojo::Array<unsigned char>::front (this=0x362504b6f3a8) at ../../mojo/public/cpp/bindings/array.h:99 #5 0x000000000054b9b4 in mojo::TypeConverter<scoped_refptr<media::DecoderBuffer>, mojo::StructPtr<m edia::mojom::DecoderBuffer> >::Convert (input=...) at ../../media/mojo/common/media_type_converters.cc:476 I'll enable media_mojo_unittests on bots shortly so that we can catch regressions. BUG=617204 TEST=fixes a unittest Committed: https://crrev.com/03fa1fc9ad9bc265dcebb93b0e8d5a47f764b8a6 Cr-Commit-Position: refs/heads/master@{#399054} ==========
Message was sent while issue was closed.
Patchset 1 (id:??) landed as https://crrev.com/03fa1fc9ad9bc265dcebb93b0e8d5a47f764b8a6 Cr-Commit-Position: refs/heads/master@{#399054}
Message was sent while issue was closed.
Patchset #2 (id:20001) has been deleted |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
