Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(252)

Issue 956323002: Tweak the TextTrackCue "cue index" management (Closed)

Created:
5 years, 10 months ago by fs
Modified:
5 years, 10 months ago
Reviewers:
philipj_slow
CC:
blink-reviews, nessy, gasubic, eric.carlson_apple.com, blink-reviews-html_chromium.org, dglazkov+blink, vcarbune.chromium
Target Ref:
refs/heads/master
Project:
blink
Visibility:
Public.

Description

Tweak the TextTrackCue "cue index" management Change the invalidation function to instead of updating the cached cue indices, update an index to the first invalid index stored in the list itself. Use this "first invalid index" to check if a cached index is valid or not, and if it isn't then validate all the list's cues cached indices starting at the stored index. This means invalidation is faster than before (constant time), and valid is worst case the same, but with chance of amortization. BUG=321654 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=190948

Patch Set 1 #

Total comments: 8

Patch Set 2 : Invalidate cached cue index on removal from list. #

Patch Set 3 : nits #

Patch Set 4 : Nits(2). #

Patch Set 5 : Rebase. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+43 lines, -32 lines) Patch
M Source/core/html/track/CueTimeline.cpp View 1 chunk +1 line, -1 line 0 comments Download
M Source/core/html/track/TextTrackCue.h View 1 2 chunks +4 lines, -2 lines 0 comments Download
M Source/core/html/track/TextTrackCue.cpp View 1 2 2 chunks +12 lines, -9 lines 0 comments Download
M Source/core/html/track/TextTrackCueList.h View 1 2 3 4 2 chunks +4 lines, -2 lines 0 comments Download
M Source/core/html/track/TextTrackCueList.cpp View 1 2 3 4 6 chunks +22 lines, -18 lines 0 comments Download

Messages

Total messages: 12 (3 generated)
fs
5 years, 10 months ago (2015-02-26 15:21:26 UTC) #2
philipj_slow
LGTM with some nits https://codereview.chromium.org/956323002/diff/1/Source/core/html/track/TextTrackCue.cpp File Source/core/html/track/TextTrackCue.cpp (right): https://codereview.chromium.org/956323002/diff/1/Source/core/html/track/TextTrackCue.cpp#newcode125 Source/core/html/track/TextTrackCue.cpp:125: // a(n enabled) track (and ...
5 years, 10 months ago (2015-02-26 15:43:12 UTC) #3
fs
Plugged a potential hole, and now we may have another invariant on our hands... =) ...
5 years, 10 months ago (2015-02-26 16:09:34 UTC) #4
fs
And now I conflicted with myself.. bummer.
5 years, 10 months ago (2015-02-26 16:10:11 UTC) #5
fs
Did you have a strong opinion on unsigned vs. size_t?
5 years, 10 months ago (2015-02-26 17:02:40 UTC) #6
philipj_slow
No I guess I don't. I see unsigned everywhere when I go looking for it, ...
5 years, 10 months ago (2015-02-26 18:16:55 UTC) #7
fs
On 2015/02/26 18:16:55, philipj_UTC7 wrote: > No I guess I don't. I see unsigned everywhere ...
5 years, 10 months ago (2015-02-26 18:19:25 UTC) #8
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/956323002/20002
5 years, 10 months ago (2015-02-26 18:20:31 UTC) #11
commit-bot: I haz the power
5 years, 10 months ago (2015-02-26 19:21:32 UTC) #12
Message was sent while issue was closed.
Committed patchset #5 (id:20002) as
https://src.chromium.org/viewvc/blink?view=rev&revision=190948

Powered by Google App Engine
This is Rietveld 408576698