Use Span<> to replace low level pointer arithmetic in MediaCacheStream::Read()

RESOLVED FIXED in Firefox 59

Status

()

Core
Audio/Video: Playback
P3
normal
RESOLVED FIXED
12 days ago
5 hours ago

People

(Reporter: jwwang, Assigned: jwwang)

Tracking

(Blocks: 1 bug)

58 Branch
mozilla59
Points:
---

Firefox Tracking Flags

(firefox58 wontfix, firefox59 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

Comment hidden (empty)
(Assignee)

Updated

12 days ago
Assignee: nobody → jwwang
Blocks: 1355409
Priority: -- → P3
Comment hidden (mozreview-request)
(Assignee)

Updated

9 days ago
Attachment #8927692 - Flags: review?(bechen)

Comment 2

9 days ago
mozreview-review
Comment on attachment 8927692 [details]
Bug 1416085 - use Span<> to replace low level pointer arithmetic in Read().

https://reviewboard.mozilla.org/r/198970/#review204028
Attachment #8927692 - Flags: review?(bechen) → review+
(Assignee)

Updated

9 days ago
Attachment #8927692 - Flags: review?(gsquelart)

Comment 3

8 days ago
mozreview-review
Comment on attachment 8927692 [details]
Bug 1416085 - use Span<> to replace low level pointer arithmetic in Read().

https://reviewboard.mozilla.org/r/198970/#review204256

::: dom/media/MediaCache.cpp:2568
(Diff revision 1)
> +    }
>  
> -      // See if the data is available in the partial cache block of any
> -      // stream reading this resource. We need to do this in case there is
> -      // another stream with this resource that has all the data to the end of
> -      // the stream but the data doesn't end on a block boundary.
> +    // See if we can use the data in the partial block of any stream reading
> +    // this resource. Note we use the partial block only when it is completed,
> +    // that is reaching EOS.
> +    bool foundDataInPartialBLock = false;

'foundDataInPartialBLock' -> 'foundDataInPartialBlock' ('L' in 'Block' should not be capitalized)

::: dom/media/MediaCache.cpp:2587
(Diff revision 1)
> -        // Break for we've reached EOS and have nothing more to read.
> -        break;
> +      break;
> -      }
> +    }
>  
> -      if (mStreamOffset != streamOffset) {
> +    if (mStreamOffset != streamOffset) {
> -        // Updat mStreamOffset before we drop the lock. We need to run
> +      // Updat mStreamOffset before we drop the lock. We need to run

'Updat' -> 'Update' (mis-fix of a recent nit!)
Attachment #8927692 - Flags: review?(gsquelart) → review+
(Assignee)

Comment 4

8 days ago
Thanks for the reviews!
Comment hidden (mozreview-request)

Comment 6

8 days ago
Pushed by jwwang@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/05f87748526f
use Span<> to replace low level pointer arithmetic in Read(). r=bechen,gerald

Comment 7

8 days ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/05f87748526f
Status: NEW → RESOLVED
Last Resolved: 8 days ago
status-firefox59: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla59

Updated

5 hours ago
status-firefox58: affected → wontfix
You need to log in before you can comment on or make changes to this bug.