Status

()

defect
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: jya, Assigned: jya)

Tracking

unspecified
mozilla52
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox52 fixed)

Details

Attachments

(3 attachments)

Assignee

Description

3 years ago
HAs been unused for over a year now...
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Assignee

Updated

3 years ago
Blocks: 1316211

Comment 3

3 years ago
mozreview-review
Comment on attachment 8808889 [details]
Bug 1316205: P1. Remove unused WaveReader.

https://reviewboard.mozilla.org/r/91606/#review91528
Attachment #8808889 - Flags: review?(kaku) → review+

Comment 4

3 years ago
mozreview-review
Comment on attachment 8808890 [details]
Bug 1316205: P2. Remove wave duplicated code.

https://reviewboard.mozilla.org/r/91608/#review91524

::: dom/media/DecoderTraits.cpp:210
(Diff revision 1)
>        // We can only reach this position if a particular codec was requested,
>        // ogg is supported and working: the codec must be invalid.
>        return CANPLAY_NO;
>      }
>    }
> -  if (IsWaveType(aType.GetMIMEType())) {
> +  if (IsWaveSupportedType(aType.GetMIMEType())) {

Personally, I think a wrapper function, likes the other formats do, helps to read this nested if-statement.

```
static bool 
IsWaveTypeAndEnabled(const nsACString& aType)
{
  return IsWaveSupportedType(aType);
}
```
Attachment #8808890 - Flags: review?(kaku) → review+
Assignee

Comment 5

3 years ago
mozreview-review-reply
Comment on attachment 8808890 [details]
Bug 1316205: P2. Remove wave duplicated code.

https://reviewboard.mozilla.org/r/91608/#review91524

> Personally, I think a wrapper function, likes the other formats do, helps to read this nested if-statement.
> 
> ```
> static bool 
> IsWaveTypeAndEnabled(const nsACString& aType)
> {
>   return IsWaveSupportedType(aType);
> }
> ```

Those "wrappers" are only defined due to 2 members being public and the wish to minimize code duplication. the majority don't use a wrapper.
As it is now, it's using the same style as the line just above with IsOggSupportedType
Comment hidden (mozreview-request)

Comment 7

3 years ago
mozreview-review
Comment on attachment 8809232 [details]
Bug 1316205: P3. Remove WaveDecoder::IsEnable().

https://reviewboard.mozilla.org/r/91820/#review91796
Attachment #8809232 - Flags: review?(gsquelart) → review+

Comment 8

3 years ago
Pushed by jyavenard@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/c61363a3c271
P1. Remove unused WaveReader. r=kaku
https://hg.mozilla.org/integration/autoland/rev/133477f27fe6
P2. Remove wave duplicated code. r=kaku
https://hg.mozilla.org/integration/autoland/rev/60bf1293eabf
P3. Remove WaveDecoder::IsEnable(). r=gerald
You need to log in before you can comment on or make changes to this bug.