Closed Bug 1395015 Opened 2 years ago Closed 2 years ago

Remove HLSResource

Categories

(Core :: Audio/Video: Playback, enhancement)

enhancement
Not set

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.
Hi James,
Please take this bug.
Blocks: 1373160
Flags: needinfo?(jacheng)
Assignee: nobody → jacheng
Flags: needinfo?(jacheng)
Attachment #8902535 - Flags: review?(jwwang)
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 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 on attachment 8902535 [details]
Bug 1395015 - Remove HLSResource.

https://reviewboard.mozilla.org/r/174134/#review179410
Attachment #8902535 - Flags: review?(jwwang) → review+
https://hg.mozilla.org/mozilla-central/rev/6fc3ff8c6424
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
You need to log in before you can comment on or make changes to this bug.