1733109 - Fix some static analysis issues in media code

Status: RESOLVED FIXED

(Core :: Audio/Video, task)

Comment 1

[Paul Adenot (:padenot)]

Attachment: Bug 1733109 - use 'using' instead of 'typedef' in MediaDecoder.cpp. r?bryce

Comment 2

[Paul Adenot (:padenot)]

Attachment: Bug 1733109 - 'auto owner' can be declared as 'auto *owner' in MediaDecoder.cpp. r?bryce

Depends on D126927

Comment 3

[Paul Adenot (:padenot)]

Attachment: Bug 1733109 - do not use 'else' after 'return' in MediaDecoder.cpp. r?bryce

Depends on D126928

Comment 4

[Paul Adenot (:padenot)]

Attachment: Bug 1733109 - the parameter 'aSecondaryVideoContainer' is copied for each invocation but only used as a const reference; consider making it a const reference in MediaDecoder.cpp r?bryce

Depends on D126929

Comment 5

[Paul Adenot (:padenot)]

Attachment: Bug 1733109 - passing result of std::move() as a const reference argument; no move will actually happen in MediaDecoder.cpp. r?bryce

Depends on D126930

Comment 6

[Paul Adenot (:padenot)]

Attachment: Bug 1733109 - do not use 'else' after 'return' in MediaDecoder.cpp. r?bryce

Depends on D126931

Comment 7

[Paul Adenot (:padenot)]

Attachment: Bug 1733109 - narrowing conversion from 'size_t' in MediaDecoder.cpp. r?bryce

INT64_MAX is more than enough for this, no worries to have.

Depends on D126932

Comment 8

[Paul Adenot (:padenot)]

Attachment: Bug 1733109 - function 'mozilla::MediaDecoder::OnSecondaryVideoContainerInstalled' has a definition with different parameter names in MediaDecoder.h. r?bryce

Depends on D126933

Comment 9

[Paul Adenot (:padenot)]

Attachment: Bug 1733109 - constructor does not initialize these fields: mOwnerBrowsingContextId in HTMLMEdiaElement.cpp. r?alwu

Depends on D126934

Comment 10

[Paul Adenot (:padenot)]

Attachment: Bug 1733109 - narrowing conversion from 'double' to 'float' in HTMLMEdiaElement.cpp. r?alwu

Depends on D126935

Comment 11

[Paul Adenot (:padenot)]

Attachment: Bug 1733109 - narrowing conversion from 'uint32_t' (aka 'unsigned int') to signed type 'int32_t' (aka 'int') is implementation-defined in HTMLMEdiaElement.cpp. r?alwu,#webidl

This is Chrome-only.

Depends on D126936

Comment 12

[Paul Adenot (:padenot)]

Attachment: Bug 1733109 - redundant return statement at the end of a function with a void return type in HTMLMediaElement.cpp. r?alwu

Depends on D126937

Comment 13

[Paul Adenot (:padenot)]

Attachment: Bug 1733109 - narrowing conversion from 'unsigned long' to signed type 'int32_t' (aka 'int') is implementation-defined HTMLMediaElement.cpp. r?pehrsons

Somewhat controversial, let me know what you think.

Depends on D126938

Comment 14

[Paul Adenot (:padenot)]

Attachment: Bug 1733109 - move constructors should be marked noexcept HTMLMediaElement.cpp. r?alwu

We don't use exceptions anyways, but having no warnings allows warning us when
warnings appear, so let's fix this.

Depends on D126939

Comment 15

[Paul Adenot (:padenot)]

Attachment: Bug 1733109 - 'auto decoder' can be declared as 'auto *decoder' in HTMLMediaElement.cpp. r?alwu

Depends on D126940

Comment 16

[Paul Adenot (:padenot)]

Attachment: Bug 1733109 - 'auto decoder' can be declared as 'auto *decoder' in HTMLMediaElement.cpp. r?alwu

Depends on D126941

Comment 17

[Paul Adenot (:padenot)]

Attachment: Bug 1733109 - local copy 'handle' of the variable 'aPrincipalHandle' is never modified; consider avoiding the copy in HTMLMediaElement.cpp. r?alwu

Depends on D126942

Comment 18

[Paul Adenot (:padenot)]

Attachment: Bug 1733109 - do not use 'else' after 'return' in HTMLMediaElement.cpp. r?alwu

Depends on D126943

Comment 19

[Paul Adenot (:padenot)]

Attachment: Bug 1733109 - 'auto cdmProxy' can be declared as 'auto *cdmProxy' in HTMLMediaElement.cpp. r?alwu

Depends on D126944

Comment 20

[Paul Adenot (:padenot)]

Attachment: Bug 1733109 - narrowing conversion from 'double' to 'float' in HTMLMediaElement.cpp. r?alwu

Depends on D126945

Comment 21

[Paul Adenot (:padenot)]

Attachment: Bug 1733109 - function 'mozilla::dom::HTMLMediaElement::MozGetMetadata' has a definition with different parameter names in HTMLMediaElement.cpp. r?alwu

Depends on D126946

Comment 22

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/88c9dbc265f6
use 'using' instead of 'typedef' in MediaDecoder.cpp. r=bryce
https://hg.mozilla.org/integration/autoland/rev/6b28a5600fee
'auto owner' can be declared as 'auto *owner' in MediaDecoder.cpp. r=bryce
https://hg.mozilla.org/integration/autoland/rev/42b69e6ec427
do not use 'else' after 'return' in MediaDecoder.cpp. r=bryce
https://hg.mozilla.org/integration/autoland/rev/15582a2f41e5
the parameter 'aSecondaryVideoContainer' is copied for each invocation but only used as a const reference; consider making it a const reference in MediaDecoder.cpp r=bryce
https://hg.mozilla.org/integration/autoland/rev/132e76589b5f
passing result of std::move() as a const reference argument; no move will actually happen in MediaDecoder.cpp. r=bryce
https://hg.mozilla.org/integration/autoland/rev/1507b273da3e
do not use 'else' after 'return' in MediaDecoder.cpp. r=bryce
https://hg.mozilla.org/integration/autoland/rev/7a15381e527d
narrowing conversion from 'size_t' in MediaDecoder.cpp. r=bryce
https://hg.mozilla.org/integration/autoland/rev/79212757b61c
function 'mozilla::MediaDecoder::OnSecondaryVideoContainerInstalled' has a definition with different parameter names in MediaDecoder.h. r=bryce
https://hg.mozilla.org/integration/autoland/rev/18b017cd25a7
constructor does not initialize these fields: mOwnerBrowsingContextId in HTMLMEdiaElement.cpp. r=alwu
https://hg.mozilla.org/integration/autoland/rev/54ba215e9a51
narrowing conversion from 'double' to 'float' in HTMLMEdiaElement.cpp. r=alwu
https://hg.mozilla.org/integration/autoland/rev/741870f29565
narrowing conversion from 'uint32_t' (aka 'unsigned int') to signed type 'int32_t' (aka 'int') is implementation-defined in HTMLMEdiaElement.cpp. r=alwu,emilio
https://hg.mozilla.org/integration/autoland/rev/e0e245487bbc
redundant return statement at the end of a function with a void return type in HTMLMediaElement.cpp. r=alwu
https://hg.mozilla.org/integration/autoland/rev/b79004b0ba85
narrowing conversion from 'unsigned long' to signed type 'int32_t' (aka 'int') is implementation-defined HTMLMediaElement.cpp. r=pehrsons
https://hg.mozilla.org/integration/autoland/rev/9f55994889a3
move constructors should be marked noexcept HTMLMediaElement.cpp. r=alwu
https://hg.mozilla.org/integration/autoland/rev/4e7e2b64ad12
'auto decoder' can be declared as 'auto *decoder' in HTMLMediaElement.cpp. r=alwu
https://hg.mozilla.org/integration/autoland/rev/48edd55318a8
'auto decoder' can be declared as 'auto *decoder' in HTMLMediaElement.cpp. r=alwu
https://hg.mozilla.org/integration/autoland/rev/2195c875c46d
local copy 'handle' of the variable 'aPrincipalHandle' is never modified; consider avoiding the copy in HTMLMediaElement.cpp. r=alwu
https://hg.mozilla.org/integration/autoland/rev/f19ade0d6039
do not use 'else' after 'return' in HTMLMediaElement.cpp. r=alwu
https://hg.mozilla.org/integration/autoland/rev/6f13eb57901f
'auto cdmProxy' can be declared as 'auto *cdmProxy' in HTMLMediaElement.cpp. r=alwu
https://hg.mozilla.org/integration/autoland/rev/77bb7f9f9026
narrowing conversion from 'double' to 'float' in HTMLMediaElement.cpp. r=alwu
https://hg.mozilla.org/integration/autoland/rev/f15b78b88c6a
function 'mozilla::dom::HTMLMediaElement::MozGetMetadata' has a definition with different parameter names in HTMLMediaElement.cpp. r=alwu

Comment 23

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/88c9dbc265f6
https://hg.mozilla.org/mozilla-central/rev/6b28a5600fee
https://hg.mozilla.org/mozilla-central/rev/42b69e6ec427
https://hg.mozilla.org/mozilla-central/rev/15582a2f41e5
https://hg.mozilla.org/mozilla-central/rev/132e76589b5f
https://hg.mozilla.org/mozilla-central/rev/1507b273da3e
https://hg.mozilla.org/mozilla-central/rev/7a15381e527d
https://hg.mozilla.org/mozilla-central/rev/79212757b61c
https://hg.mozilla.org/mozilla-central/rev/18b017cd25a7
https://hg.mozilla.org/mozilla-central/rev/54ba215e9a51
https://hg.mozilla.org/mozilla-central/rev/741870f29565
https://hg.mozilla.org/mozilla-central/rev/e0e245487bbc
https://hg.mozilla.org/mozilla-central/rev/b79004b0ba85
https://hg.mozilla.org/mozilla-central/rev/9f55994889a3
https://hg.mozilla.org/mozilla-central/rev/4e7e2b64ad12
https://hg.mozilla.org/mozilla-central/rev/48edd55318a8
https://hg.mozilla.org/mozilla-central/rev/2195c875c46d
https://hg.mozilla.org/mozilla-central/rev/f19ade0d6039
https://hg.mozilla.org/mozilla-central/rev/6f13eb57901f
https://hg.mozilla.org/mozilla-central/rev/77bb7f9f9026
https://hg.mozilla.org/mozilla-central/rev/f15b78b88c6a