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

Issue 75243004: Replace character vectors with StringBuilders in the WebVTT tokenizer (Closed)

Created:
7 years, 1 month ago by fs
Modified:
7 years, 1 month ago
CC:
blink-reviews, nessy, philipj_slow, gasubic, dglazkov+blink, adamk+blink_chromium.org, vcarbune.chromium
Base URL:
https://chromium.googlesource.com/chromium/blink.git@master
Visibility:
Public.

Description

Replace character vectors with StringBuilders in the WebVTT tokenizer With a goal of simplifying the state management of the WebVTTTokenizer/WebVTTToken pair, turn all character vectors into StringBuilders. Since the tokenizer in general deals with strings, this makes for a better fit in general (and at least two of the three eliminated FIXMEs should be able to attest to that). Also remove the local variable tagName in WebVTTTreeBuilder::constructTreeFromToken, which appear unused. BUG=319391 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=162307

Patch Set 1 #

Patch Set 2 : Rebase after symbolnames changed #

Unified diffs Side-by-side diffs Delta from patch set Stats (+35 lines, -44 lines) Patch
M Source/core/html/track/vtt/VTTParser.cpp View 1 4 chunks +7 lines, -9 lines 0 comments Download
M Source/core/html/track/vtt/VTTToken.h View 1 7 chunks +12 lines, -11 lines 0 comments Download
M Source/core/html/track/vtt/VTTTokenizer.h View 1 2 chunks +2 lines, -1 line 0 comments Download
M Source/core/html/track/vtt/VTTTokenizer.cpp View 1 4 chunks +14 lines, -23 lines 0 comments Download

Messages

Total messages: 6 (0 generated)
fs
7 years, 1 month ago (2013-11-18 15:10:31 UTC) #1
fs
7 years, 1 month ago (2013-11-19 09:26:19 UTC) #2
jochen (gone - plz use gerrit)
can you please run tryjobs?
7 years, 1 month ago (2013-11-19 13:21:17 UTC) #3
jochen (gone - plz use gerrit)
lgtm
7 years, 1 month ago (2013-11-19 16:04:24 UTC) #4
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/fs@opera.com/75243004/40001
7 years, 1 month ago (2013-11-19 16:52:17 UTC) #5
commit-bot: I haz the power
7 years, 1 month ago (2013-11-19 17:38:11 UTC) #6
Message was sent while issue was closed.
Change committed as 162307

Powered by Google App Engine
This is Rietveld 408576698