1184292 - 1,200 instances of "Failed to unlock the wakelock.: '!rv.Failed()'" emitted from dom/html/HTMLMediaElement.cpp during linux64 debug testing

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Eric Rahm [:erahm]]

+++ This bug was initially created as a clone of Bug #952079 +++

> 1154 [NNNNN] WARNING: Failed to unlock the wakelock.: '!rv.Failed()', file dom/html/HTMLMediaElement.cpp, line 2367

This warning [1], introduced in bug 952079, shows up in the following test suites:

> mozilla-central_ubuntu64_vm-debug_test-mochitest-2-bm67-tests1-linux64-build1.txt:454
> mozilla-central_ubuntu64_vm-debug_test-mochitest-e10s-2-bm53-tests1-linux64-build12.txt:436
> mozilla-central_ubuntu64_vm-debug_test-crashtest-bm53-tests1-linux64-build28.txt:211
> mozilla-central_ubuntu64_vm-debug_test-reftest-3-bm52-tests1-linux64-build0.txt:13
> mozilla-central_ubuntu64_vm-debug_test-mochitest-1-bm117-tests1-linux64-build1.txt:10
> mozilla-central_ubuntu64_vm-debug_test-mochitest-e10s-1-bm123-tests1-linux64-build25.txt:5
> mozilla-central_ubuntu64_vm-debug_test-mochitest-gl-bm113-tests1-linux64-build14.txt:4
> mozilla-central_ubuntu64_vm-debug_test-mochitest-e10s-browser-chrome-3-bm118-tests1-linux64-build0.txt:3
> mozilla-central_ubuntu64_vm-debug_test-mochitest-e10s-5-bm52-tests1-linux64-build2.txt:3
> mozilla-central_ubuntu64_vm-debug_test-mochitest-browser-chrome-2-bm117-tests1-linux64-build8.txt:3
> mozilla-central_ubuntu64_vm-debug_test-mochitest-5-bm115-tests1-linux64-build29.txt:3
> mozilla-central_ubuntu64_vm-debug_test-reftest-e10s-1-bm68-tests1-linux64-build0.txt:2
> mozilla-central_ubuntu64_vm-debug_test-reftest-2-bm53-tests1-linux64-build27.txt:2
> mozilla-central_ubuntu64_vm-debug_test-mochitest-e10s-browser-chrome-2-bm67-tests1-linux64-build0.txt:2
> mozilla-central_ubuntu64_vm-debug_test-mochitest-browser-chrome-1-bm122-tests1-linux64-build19.txt:2
> mozilla-central_ubuntu64_vm-debug_test-mochitest-other-bm53-tests1-linux64-build0.txt:1

It shows up in 223 tests. A few of the most prevalent:

> 200 - file:///builds/slave/test/build/tests/reftest/tests/dom/smil/crashtests/483584-1.svg
> 22 - dom/media/test/test_invalid_reject_play.html
> 22 - dom/media/tests/mochitest/test_peerConnection_twoAudioVideoStreamsCombined.html
> 20 - dom/media/tests/mochitest/test_peerConnection_twoVideoTracksInOneStream.html
> 18 - dom/media/tests/mochitest/test_peerConnection_twoAudioVideoStreams.html
> 17 - dom/media/tests/mochitest/ipc/test_ipc.html
> 16 - dom/media/tests/mochitest/test_peerConnection_twoVideoStreams.html
> 14 - dom/media/tests/mochitest/test_peerConnection_basicH264Video.html
> 14 - dom/media/tests/mochitest/test_peerConnection_basicAudioVideoCombined.html
> 14 - dom/media/tests/mochitest/test_dataChannel_basicAudioVideoCombined.html

[1] https://hg.mozilla.org/mozilla-central/annotate/49683d4e9ebd/dom/html/HTMLMediaElement.cpp#l2367

Comment 1

[Eric Rahm [:erahm]]

This is the #4 most verbose warning during testing.

Comment 2

[Eric Rahm [:erahm]]

:baku, it looks like this warning was introduced in your patch for bug 952079. Any thoughts on what's going on here?

Comment 3

[Andrea Marchesini [:baku]]

Attachment: warning.patch

This is the only exception we throw in Unlock(). And we want to ignore it mainly anywhere. This is also not exposed to content. I vote to remove the ErrorResult.

Comment 4

[Andrea Marchesini [:baku]]

Attachment: warning.patch

Comment 5

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8652823 [details] [diff] [review]
warning.patch

Please don't change the JS API just because of some warning.

Comment 6

[Andrea Marchesini [:baku]]

Are you proposing to ignore this issue or are you proposing something like:

NS_WARN_IF(rv.Failed() && rv.ErrorCode() != NS_ERROR_DOM_INVALID_STATE_ERR);

Keep in mind that none of the current implementation is taking care about the error code properly.

Comment 7

[Olli Pettay [:smaug][[e-mail]]]

Since we don't care about the return value anyway, just not having the warning should be ok.

Comment 8

[Andrea Marchesini [:baku]]

Attachment: warning.patch

Comment 9

[Andrea Marchesini [:baku]]

Attachment: warning.patch

Comment 10

[Andrea Marchesini [:baku]]

No try-push needed because this code removes some NS_WARN_IF lines.

Comment 11

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/fea87cbeaa6b

Comment 12

[Ryan VanderMeulen [:RyanVM][PTO June 24-28]]

https://hg.mozilla.org/mozilla-central/rev/fea87cbeaa6b