Closed
Bug 1395015
Opened 3 years ago
Closed 3 years ago
Remove HLSResource
Categories
(Core :: Audio/Video: Playback, enhancement)
Core
Audio/Video: Playback
Tracking
()
RESOLVED
FIXED
mozilla57
Tracking | Status | |
---|---|---|
firefox57 | --- | fixed |
People
(Reporter: jwwang, Assigned: JamesCheng)
References
Details
Attachments
(1 file)
HLSResource can be removed by merging HLSDecoder and HLSResource.
Reporter | ||
Comment 1•3 years ago
|
||
Hi James, Please take this bug.
Blocks: 1373160
Flags: needinfo?(jacheng)
Assignee | ||
Updated•3 years ago
|
Assignee: nobody → jacheng
Flags: needinfo?(jacheng)
Comment hidden (mozreview-request) |
Assignee | ||
Updated•3 years ago
|
Attachment #8902535 -
Flags: review?(jwwang)
Comment hidden (mozreview-request) |
Reporter | ||
Comment 4•3 years ago
|
||
mozreview-review |
Comment on attachment 8902535 [details] Bug 1395015 - Remove HLSResource. https://reviewboard.mozilla.org/r/174134/#review179396 ::: dom/media/hls/HLSDecoder.cpp:64 (Diff revision 2) > +void > +HLSResourceCallbacksSupport::OnDataArrived() > +{ > + MOZ_ASSERT(NS_IsMainThread()); > + if (mDecoder) { > + mDecoder->onDataAvailable(); Call mDecoder->NotifyDataArrived() directly so you can remove HLSDecoder::onDataAvailable(). ::: dom/media/hls/HLSDecoder.cpp:73 (Diff revision 2) > void > -HLSDecoder::Shutdown() > +HLSResourceCallbacksSupport::OnError(int aErrorCode) > { > MOZ_ASSERT(NS_IsMainThread()); > - if (mResource) { > - mResource->Detach(); > + if (mDecoder) { > + mDecoder->onError(aErrorCode); Ditto. ::: dom/media/hls/HLSDecoder.cpp (Diff revision 2) > > void > HLSDecoder::AddSizeOfResources(ResourceSizes* aSizes) > { > MOZ_ASSERT(NS_IsMainThread()); > - if (mResource) { Add a todo item that we will count the size of JAVA wrappers in the future. ::: dom/media/hls/HLSDecoder.cpp:171 (Diff revision 2) > HLSDecoder::GetCurrentPrincipal() > { > MOZ_ASSERT(NS_IsMainThread()); > - return mResource ? mResource->GetCurrentPrincipal() : nullptr; > + nsCOMPtr<nsIPrincipal> principal; > + nsIScriptSecurityManager* secMan = nsContentUtils::GetSecurityManager(); > + if (!secMan || !mChannel) Always use parenthesis. ::: dom/media/hls/HLSDecoder.cpp:215 (Diff revision 2) > + > +HLSDecoder::~HLSDecoder() > +{ > + HLS_DEBUG("HLSDecoder", "~HLSDecoder()"); > + if (mCallbackSupport) { > + mCallbackSupport->Detach(); Detach() should be done in Shutdown() to prevent callbacks from firing after Shutdown().
Comment hidden (mozreview-request) |
Reporter | ||
Comment 6•3 years ago
|
||
mozreview-review |
Comment on attachment 8902535 [details] Bug 1395015 - Remove HLSResource. https://reviewboard.mozilla.org/r/174134/#review179402 ::: dom/media/hls/HLSDecoder.cpp:87 (Diff revisions 1 - 3) > - HLS_DEBUG("HLSDecoder", "onDataAvailable"); > - MediaDecoder::NotifyDataArrived(); > -} > - > -void > HLSDecoder::onError(int aErrorCode) Remove this function. ::: dom/media/hls/HLSDecoder.cpp:218 (Diff revisions 1 - 3) > - HLS_DEBUG("HLSDecoder", "~HLSDecoder()"); > + HLS_DEBUG("HLSDecoder", "Shutdown"); > if (mCallbackSupport) { > mCallbackSupport->Detach(); > mCallbackSupport = nullptr; > } > +} Be sure to call baseclass::Shutdown(). ::: dom/media/hls/HLSDecoder.cpp:222 (Diff revisions 1 - 3) > } > +} > +HLSDecoder::~HLSDecoder() > +{ > + HLS_DEBUG("HLSDecoder", "~HLSDecoder()"); > if (mHLSResourceWrapper) { These lines can be moved to Shutdown().
Attachment #8902535 -
Flags: review?(jwwang) → review-
Comment hidden (mozreview-request) |
Reporter | ||
Comment 8•3 years ago
|
||
mozreview-review |
Comment on attachment 8902535 [details] Bug 1395015 - Remove HLSResource. https://reviewboard.mozilla.org/r/174134/#review179410
Attachment #8902535 -
Flags: review?(jwwang) → review+
Pushed by jacheng@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/6fc3ff8c6424 Remove HLSResource. r=jwwang
Comment 10•3 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/6fc3ff8c6424
Status: NEW → RESOLVED
Closed: 3 years ago
status-firefox57:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
You need to log in
before you can comment on or make changes to this bug.
Description
•