Closed
Bug 1184292
Opened 9 years ago
Closed 9 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)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
mozilla43
People
(Reporter: erahm, Assigned: baku)
References
(Blocks 1 open bug)
Details
Attachments
(1 file, 3 obsolete files)
(deleted),
patch
|
Details | Diff | Splinter Review |
+++ 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
Reporter | ||
Comment 1•9 years ago
|
||
This is the #4 most verbose warning during testing.
Reporter | ||
Comment 2•9 years ago
|
||
: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)
Assignee | ||
Comment 3•9 years ago
|
||
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)
Assignee | ||
Updated•9 years ago
|
Attachment #8652822 -
Attachment description: warning.path → warning.patch
Attachment #8652822 -
Attachment is patch: true
Assignee | ||
Comment 4•9 years ago
|
||
Attachment #8652822 -
Attachment is obsolete: true
Attachment #8652822 -
Flags: review?(bugs)
Attachment #8652823 -
Flags: review?(bugs)
Comment 5•9 years ago
|
||
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-
Assignee | ||
Comment 6•9 years ago
|
||
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)
Comment 7•9 years ago
|
||
Since we don't care about the return value anyway, just not having the warning should be ok.
Flags: needinfo?(bugs)
Assignee | ||
Comment 8•9 years ago
|
||
Attachment #8652823 -
Attachment is obsolete: true
Attachment #8652861 -
Flags: review?(bugs)
Updated•9 years ago
|
Attachment #8652861 -
Flags: review?(bugs) → review+
Assignee | ||
Updated•9 years ago
|
Assignee: nobody → amarchesini
Assignee | ||
Comment 9•9 years ago
|
||
Attachment #8652861 -
Attachment is obsolete: true
Assignee | ||
Comment 10•9 years ago
|
||
No try-push needed because this code removes some NS_WARN_IF lines.
Keywords: checkin-needed
Comment 11•9 years ago
|
||
Keywords: checkin-needed
Comment 12•9 years ago
|
||
Status: NEW → RESOLVED
Closed: 9 years ago
status-firefox43:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla43
Updated•6 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•