1395015 - Remove HLSResource

Status: RESOLVED FIXED

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

Description

[JW Wang [:jwwang] [:jw_wang]]

HLSResource can be removed by merging HLSDecoder and HLSResource.

Comment 1

[JW Wang [:jwwang] [:jw_wang]]

Hi James,
Please take this bug.

Comment 2

[James Cheng[:JamesCheng]]

Attachment: Bug 1395015 - Remove HLSResource.

Move the dependent method into HLSDecoder.
Remove the class entirely.

Review commit: https://reviewboard.mozilla.org/r/174134/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/174134/

Comment 3

[James Cheng[:JamesCheng]]

Comment on attachment 8902535 [details]
Bug 1395015 - Remove HLSResource.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/174134/diff/1-2/

Comment 4

[JW Wang [:jwwang] [:jw_wang]]

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 5

[James Cheng[:JamesCheng]]

Comment on attachment 8902535 [details]
Bug 1395015 - Remove HLSResource.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/174134/diff/2-3/

Comment 6

[JW Wang [:jwwang] [:jw_wang]]

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().

Comment 7

[James Cheng[:JamesCheng]]

Comment on attachment 8902535 [details]
Bug 1395015 - Remove HLSResource.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/174134/diff/3-4/

Comment 8

[JW Wang [:jwwang] [:jw_wang]]

Comment on attachment 8902535 [details]
Bug 1395015 - Remove HLSResource.

https://reviewboard.mozilla.org/r/174134/#review179410

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6fc3ff8c6424
Remove HLSResource. r=jwwang

Comment 10

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/6fc3ff8c6424