Closed Bug 1184292 Opened 4 years ago Closed 4 years ago

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

Categories

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

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla43
Tracking Status
firefox42 --- affected
firefox43 --- fixed

People

(Reporter: erahm, Assigned: baku)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 3 obsolete files)

+++ 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
This is the #4 most verbose warning during testing.
:baku, it looks like this warning was introduced in your patch for bug 952079. Any thoughts on what's going on here?
Flags: needinfo?(amarchesini)
Attached patch warning.patch (obsolete) — Splinter Review
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.
Flags: needinfo?(amarchesini)
Attachment #8652822 - Flags: review?(bugs)
Attachment #8652822 - Attachment description: warning.path → warning.patch
Attachment #8652822 - Attachment is patch: true
Attached patch warning.patch (obsolete) — Splinter Review
Attachment #8652822 - Attachment is obsolete: true
Attachment #8652822 - Flags: review?(bugs)
Attachment #8652823 - Flags: review?(bugs)
Comment on attachment 8652823 [details] [diff] [review]
warning.patch

Please don't change the JS API just because of some warning.
Attachment #8652823 - Flags: review?(bugs) → review-
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.
Flags: needinfo?(bugs)
Since we don't care about the return value anyway, just not having the warning should be ok.
Flags: needinfo?(bugs)
Attached patch warning.patch (obsolete) — Splinter Review
Attachment #8652823 - Attachment is obsolete: true
Attachment #8652861 - Flags: review?(bugs)
Attachment #8652861 - Flags: review?(bugs) → review+
Assignee: nobody → amarchesini
Attached patch warning.patchSplinter Review
Attachment #8652861 - Attachment is obsolete: true
No try-push needed because this code removes some NS_WARN_IF lines.
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/fea87cbeaa6b
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla43
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.