1416663 - Factor Gecko specific code out of VideoFrameContainer

Status: RESOLVED FIXED

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

Description

[Chris Pearce [:cpearce (Not reading bugmail)]]

There's a bunch of code that uses HTMLMediaElement in VideoFrameContainer. Most of this is layout specific. If we can factor that out, say back into HTMLMediaElement which knows all about how Gecko does layout, then it's easier to import VideoFrameContainer into the gecko-media Rust crate.

Comment 1

[Chris Pearce [:cpearce (Not reading bugmail)]]

Attachment: Bug 1416663 - Move Gecko/HTMLMediaElement specific stuff out of VideoFrameContainer.

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

Comment 2

[Chris Pearce [:cpearce (Not reading bugmail)]]

Comment on attachment 8927742 [details]
Bug 1416663 - Move Gecko/HTMLMediaElement specific stuff out of VideoFrameContainer.

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

Comment 3

[Code Review Bot [:reviewbot]]

Comment on attachment 8927742 [details]
Bug 1416663 - Move Gecko/HTMLMediaElement specific stuff out of VideoFrameContainer.

https://reviewboard.mozilla.org/r/199012/#review204034


C/C++ static analysis found 1 defect in this patch.

You can run this analysis locally with: `./mach static-analysis check path/to/file.cpp`


::: dom/html/HTMLMediaElement.cpp:6412
(Diff revision 1)
>    return mDecoderPrincipalChangeObservers.RemoveElement(aObserver);
>  }
>  
> +void
> +HTMLMediaElement::Invalidate(bool aImageSizeChanged,
> +                             Maybe<nsIntSize> aNewIntrinsicSize,

Error: Type 'maybe<nsintsize>' (aka 'maybe<intsizetyped<mozilla::gfx::unknownunits> >') must not be used as parameter [clang-tidy: mozilla-non-memmovable-template-arg]

Comment 4

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

Comment on attachment 8927742 [details]
Bug 1416663 - Move Gecko/HTMLMediaElement specific stuff out of VideoFrameContainer.

https://reviewboard.mozilla.org/r/199012/#review204032

::: dom/media/MediaDecoderOwner.h:184
(Diff revision 2)
>    }
>  
> +  // Called by the frame container to notify the layout engine that the
> +  // size of the image has changed, or the video needs to be be repainted
> +  // for some other reason.
> +  virtual void Invalidate(bool aImageSizeChanged,

Please provide default implementations for gecko-only functions as above.

Comment 5

[Code Review Bot [:reviewbot]]

Comment on attachment 8927742 [details]
Bug 1416663 - Move Gecko/HTMLMediaElement specific stuff out of VideoFrameContainer.

https://reviewboard.mozilla.org/r/199012/#review204040


C/C++ static analysis found 1 defect in this patch.

You can run this analysis locally with: `./mach static-analysis check path/to/file.cpp`


::: dom/html/HTMLMediaElement.cpp:6412
(Diff revision 2)
>    return mDecoderPrincipalChangeObservers.RemoveElement(aObserver);
>  }
>  
> +void
> +HTMLMediaElement::Invalidate(bool aImageSizeChanged,
> +                             Maybe<nsIntSize> aNewIntrinsicSize,

Error: Type 'maybe<nsintsize>' (aka 'maybe<intsizetyped<mozilla::gfx::unknownunits> >') must not be used as parameter [clang-tidy: mozilla-non-memmovable-template-arg]

Comment 6

[Chris Pearce [:cpearce (Not reading bugmail)]]

Comment on attachment 8927742 [details]
Bug 1416663 - Move Gecko/HTMLMediaElement specific stuff out of VideoFrameContainer.

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

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/442c830d2109
Move Gecko/HTMLMediaElement specific stuff out of VideoFrameContainer. r=jwwang

Comment 8

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/442c830d2109