Closed Bug 1398659 Opened 7 years ago Closed 7 years ago

Some ChannelMediaResource/MediaCacheStream code cleanup

Categories

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

enhancement

Tracking

()

RESOLVED FIXED
mozilla57
Tracking Status
firefox57 --- fixed

People

(Reporter: jwwang, Assigned: jwwang)

References

Details

Attachments

(12 files)

59 bytes, text/x-review-board-request
mozbugz
: review+
Details
59 bytes, text/x-review-board-request
mozbugz
: review+
Details
59 bytes, text/x-review-board-request
mozbugz
: review+
Details
59 bytes, text/x-review-board-request
mozbugz
: review+
Details
59 bytes, text/x-review-board-request
mozbugz
: review+
Details
59 bytes, text/x-review-board-request
mozbugz
: review+
Details
59 bytes, text/x-review-board-request
mozbugz
: review+
Details
59 bytes, text/x-review-board-request
mozbugz
: review+
Details
59 bytes, text/x-review-board-request
mozbugz
: review+
Details
59 bytes, text/x-review-board-request
mozbugz
: review+
Details
59 bytes, text/x-review-board-request
mozbugz
: review+
Details
59 bytes, text/x-review-board-request
mozbugz
: review+
Details
      No description provided.
Assignee: nobody → jwwang
Blocks: 1382978
Priority: -- → P3
Attachment #8906453 - Flags: review?(gsquelart)
Attachment #8906454 - Flags: review?(gsquelart)
Attachment #8906455 - Flags: review?(gsquelart)
Attachment #8906456 - Flags: review?(gsquelart)
Attachment #8906457 - Flags: review?(gsquelart)
Attachment #8906458 - Flags: review?(gsquelart)
Attachment #8906459 - Flags: review?(gsquelart)
Attachment #8906460 - Flags: review?(gsquelart)
Attachment #8906461 - Flags: review?(gsquelart)
Attachment #8906462 - Flags: review?(gsquelart)
Attachment #8906463 - Flags: review?(gsquelart)
Attachment #8906464 - Flags: review?(gsquelart)
Comment on attachment 8906453 [details]
Bug 1398659. P1 - tighten up the assertions in InitAsClone().

https://reviewboard.mozilla.org/r/178186/#review183104
Attachment #8906453 - Flags: review?(gsquelart) → review+
Comment on attachment 8906454 [details]
Bug 1398659. P2 - remove the if statement since new() is infallible.

https://reviewboard.mozilla.org/r/178188/#review183106
Attachment #8906454 - Flags: review?(gsquelart) → review+
Comment on attachment 8906455 [details]
Bug 1398659. P3 - ChannelMediaResource::Open() is a no-op for a cloned resource.

https://reviewboard.mozilla.org/r/178190/#review183110

Indeed.
Attachment #8906455 - Flags: review?(gsquelart) → review+
Comment on attachment 8906456 [details]
Bug 1398659. P4 - remove unused code and add some assertions.

https://reviewboard.mozilla.org/r/178192/#review183114
Attachment #8906456 - Flags: review?(gsquelart) → review+
Comment on attachment 8906457 [details]
Bug 1398659. P5 - let ChannelMediaResource::Open() set mListener without calling OpenChannel().

https://reviewboard.mozilla.org/r/178194/#review183120
Attachment #8906457 - Flags: review?(gsquelart) → review+
Comment on attachment 8906458 [details]
Bug 1398659. P6 - remove the nsIStreamListener** parameter from OpenChannel().

https://reviewboard.mozilla.org/r/178196/#review183122

::: commit-message-dada9:3
(Diff revision 1)
> +Bug 1398659. P6 - remove the nsIStreamListener** parameter from OpenChannel().
> +
> +The only caller is CacheClientSeek() which always pass nullptr to it.

'pass' -> 'passes'
Attachment #8906458 - Flags: review?(gsquelart) → review+
Comment on attachment 8906459 [details]
Bug 1398659. P7 - tighten up the assertions in OpenChannel().

https://reviewboard.mozilla.org/r/178198/#review183124
Attachment #8906459 - Flags: review?(gsquelart) → review+
Comment on attachment 8906460 [details]
Bug 1398659. P8 - remove the call to mCacheStream.NotifyDataLength() from OpenChannel().

https://reviewboard.mozilla.org/r/178200/#review183126
Attachment #8906460 - Flags: review?(gsquelart) → review+
Comment on attachment 8906462 [details]
Bug 1398659. P10 - remove ChannelMediaResource::mIgnoreClose.

https://reviewboard.mozilla.org/r/178204/#review183130

::: commit-message-12a82:4
(Diff revision 1)
> +Bug 1398659. P10 - remove ChannelMediaResource::mIgnoreClose.
> +
> +mIgnoreClose is always set in conjunction with a call to CloseChannel().
> +Sincce mListener->Revoke() will prevent future OnStopRequest() calls from coming,

'Sincce' -> 'Since'
Attachment #8906462 - Flags: review?(gsquelart) → review+
Comment on attachment 8906463 [details]
Bug 1398659. P11 - replace mCacheStream.GetLength() with GetLength().

https://reviewboard.mozilla.org/r/178206/#review183132
Attachment #8906463 - Flags: review?(gsquelart) → review+
Comment on attachment 8906464 [details]
Bug 1398659. P12 - remove unused MediaCacheStream::mHasHadUpdate.

https://reviewboard.mozilla.org/r/178208/#review183134
Attachment #8906464 - Flags: review?(gsquelart) → review+
Thanks for the reviews!
Pushed by jwwang@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/3563eaa019d3
P1 - tighten up the assertions in InitAsClone(). r=gerald
https://hg.mozilla.org/integration/autoland/rev/3928ea56c1d3
P2 - remove the if statement since new() is infallible. r=gerald
https://hg.mozilla.org/integration/autoland/rev/2a71c9276c31
P3 - ChannelMediaResource::Open() is a no-op for a cloned resource. r=gerald
https://hg.mozilla.org/integration/autoland/rev/bd93f2e8986b
P4 - remove unused code and add some assertions. r=gerald
https://hg.mozilla.org/integration/autoland/rev/aae0f0941029
P5 - let ChannelMediaResource::Open() set mListener without calling OpenChannel(). r=gerald
https://hg.mozilla.org/integration/autoland/rev/020a6054e228
P6 - remove the nsIStreamListener** parameter from OpenChannel(). r=gerald
https://hg.mozilla.org/integration/autoland/rev/44e400e8dc12
P7 - tighten up the assertions in OpenChannel(). r=gerald
https://hg.mozilla.org/integration/autoland/rev/9692fc0d7937
P8 - remove the call to mCacheStream.NotifyDataLength() from OpenChannel(). r=gerald
https://hg.mozilla.org/integration/autoland/rev/efd3ef5ae6f9
P9 - remove unused member. r=gerald
https://hg.mozilla.org/integration/autoland/rev/665409f64b70
P10 - remove ChannelMediaResource::mIgnoreClose. r=gerald
https://hg.mozilla.org/integration/autoland/rev/fadb12f8f604
P11 - replace mCacheStream.GetLength() with GetLength(). r=gerald
https://hg.mozilla.org/integration/autoland/rev/130a5e787ba6
P12 - remove unused MediaCacheStream::mHasHadUpdate. r=gerald
Blocks: 1428688
No longer blocks: 1428688
You need to log in before you can comment on or make changes to this bug.