Closed
Bug 1398659
Opened 7 years ago
Closed 7 years ago
Some ChannelMediaResource/MediaCacheStream code cleanup
Categories
(Core :: Audio/Video: Playback, enhancement, P3)
Core
Audio/Video: Playback
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 | ||
Updated•7 years ago
|
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Updated•7 years ago
|
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 13•7 years ago
|
||
mozreview-review |
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 14•7 years ago
|
||
mozreview-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 15•7 years ago
|
||
mozreview-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 16•7 years ago
|
||
mozreview-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 17•7 years ago
|
||
mozreview-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 18•7 years ago
|
||
mozreview-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 19•7 years ago
|
||
mozreview-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 20•7 years ago
|
||
mozreview-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 21•7 years ago
|
||
mozreview-review |
Comment on attachment 8906461 [details] Bug 1398659. P9 - remove unused member. https://reviewboard.mozilla.org/r/178202/#review183128
Attachment #8906461 -
Flags: review?(gsquelart) → review+
Comment 22•7 years ago
|
||
mozreview-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 23•7 years ago
|
||
mozreview-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 24•7 years ago
|
||
mozreview-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+
Assignee | ||
Comment 25•7 years ago
|
||
Thanks for the reviews!
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 33•7 years ago
|
||
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
Comment 34•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/3563eaa019d3 https://hg.mozilla.org/mozilla-central/rev/3928ea56c1d3 https://hg.mozilla.org/mozilla-central/rev/2a71c9276c31 https://hg.mozilla.org/mozilla-central/rev/bd93f2e8986b https://hg.mozilla.org/mozilla-central/rev/aae0f0941029 https://hg.mozilla.org/mozilla-central/rev/020a6054e228 https://hg.mozilla.org/mozilla-central/rev/44e400e8dc12 https://hg.mozilla.org/mozilla-central/rev/9692fc0d7937 https://hg.mozilla.org/mozilla-central/rev/efd3ef5ae6f9 https://hg.mozilla.org/mozilla-central/rev/665409f64b70 https://hg.mozilla.org/mozilla-central/rev/fadb12f8f604 https://hg.mozilla.org/mozilla-central/rev/130a5e787ba6
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox57:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
You need to log in
before you can comment on or make changes to this bug.
Description
•