Closed
Bug 1343458
Opened 7 years ago
Closed 7 years ago
Label runnables in dom/media/VideoFrameContainer.cpp
Categories
(Core :: Audio/Video: Playback, enhancement, P3)
Core
Audio/Video: Playback
Tracking
()
RESOLVED
FIXED
mozilla54
Tracking | Status | |
---|---|---|
firefox54 | --- | fixed |
People
(Reporter: jwwang, Assigned: jwwang)
References
Details
(Whiteboard: [QDL][TDC-MVP][MEDIA])
Attachments
(1 file)
No description provided.
Assignee | ||
Updated•7 years ago
|
Comment hidden (mozreview-request) |
Comment 2•7 years ago
|
||
mozreview-review |
Comment on attachment 8842709 [details] Bug 1343458 - Label runnables in dom/media/VideoFrameContainer.cpp. https://reviewboard.mozilla.org/r/116478/#review118584 As far as I can tell, what you did here is assign these runnables to a tabgroup, you didn't actually label the runnables, unless I missed something? So either r+ with the changes I suggest to ensure the runnables are labled, or an explanation and/or the commit message adjusted to ensure it reflects what your patch does. ::: dom/media/VideoFrameContainer.cpp:177 (Diff revision 1) > newImages.LastElement().mFrameID); > } > > SetCurrentFramesLocked(mLastPlayedVideoFrame.GetIntrinsicSize(), images); > nsCOMPtr<nsIRunnable> event = > new VideoFrameContainerInvalidateRunnable(this); VideoFrameContainerInvalidateRunnable inherits from mozilla::Runnable, which implements nsINamed, so you should be able to call SetNamed() in its constructor to set a distinctive name. ::: dom/media/VideoFrameContainer.cpp:249 (Diff revision 1) > RefPtr<VideoFrameContainer> self = this; > PrincipalHandle principalHandle = mPendingPrincipalHandle; > mLastPrincipalHandle = mPendingPrincipalHandle; > mPendingPrincipalHandle = PRINCIPAL_HANDLE_NONE; > mFrameIDForPendingPrincipalHandle = 0; > - NS_DispatchToMainThread(NS_NewRunnableFunction([self, principalHandle]() { > + mMainThread->Dispatch(NS_NewRunnableFunction([self, principalHandle]() { It seems we should be using the variant of NS_NewRunnableFunction that takes a name argument going forward? https://dxr.mozilla.org/mozilla-central/rev/d29f84406483c721a13cf9a52936ecced0c5c98a/xpcom/threads/nsThreadUtils.h#437
Attachment #8842709 -
Flags: review?(cpearce) → review+
Assignee | ||
Comment 3•7 years ago
|
||
mozreview-review-reply |
Comment on attachment 8842709 [details] Bug 1343458 - Label runnables in dom/media/VideoFrameContainer.cpp. https://reviewboard.mozilla.org/r/116478/#review118584 By labeling a runnable, it means to associate a special attribue with the runnable so its execution can be prioritized and delayed. This is done by dispatching a runnable through some utility class. See https://wiki.mozilla.org/Quantum/DOM#Q1:_What_is_the_impact_of_leaving_a_runnable_unlabeled.3F for more details. Giving a runnable a name however is another kind of labeling which doesn't change the execution order of runnables and is for debugging purpose only.
Comment hidden (mozreview-request) |
Pushed by jwwang@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/6874dcb3e095 Label runnables in dom/media/VideoFrameContainer.cpp. r=cpearce
Comment 6•7 years ago
|
||
Thanks for explaining.
Comment 7•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/6874dcb3e095
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox54:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
Updated•7 years ago
|
Whiteboard: [QDL][TDC-MVP][MEDIA]
You need to log in
before you can comment on or make changes to this bug.
Description
•