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

Issue 22796004: Simplify WTF::base64Decode() after r154538 (Closed)

Created:
7 years, 4 months ago by do-not-use
Modified:
7 years, 4 months ago
CC:
blink-reviews, loislo+blink_chromium.org, eae+blinkwatch, yurys+blink_chromium.org, abarth-chromium, dglazkov+blink, adamk+blink_chromium.org, jeez, lgombos
Visibility:
Public.

Description

Simplify WTF::base64Decode() after r154538 r154538 increased the complexity of WTF::base64Decode() by introducing a new Base64PaddingValidationPolicy parameter. This patch simplifies the code a bit while achieving the same functionality and matching the corresponding WebKit implementation: http://trac.webkit.org/changeset/153904 Strictly validating padding does not make much sense unless we also validate strictly invalid characters. Therefore, this patch replaces the Base64PaddingValidationPolicy enum with an additional Base64FailOnInvalidCharacterOrExcessPadding value in the Base64DecodePolicy enum. The code is also improved to validate the padding at the end of the decoding loop and also removes a redundant check for the (dataLength % 4 == 1) case. BUG=261200 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=155999

Patch Set 1 #

Patch Set 2 : Restore the enum's original name #

Unified diffs Side-by-side diffs Delta from patch set Stats (+28 lines, -40 lines) Patch
M LayoutTests/fast/dom/Window/atob-btoa.html View 1 chunk +1 line, -0 lines 0 comments Download
M LayoutTests/fast/dom/Window/atob-btoa-expected.txt View 1 chunk +1 line, -0 lines 0 comments Download
M Source/core/page/DOMWindowBase64.cpp View 1 chunk +1 line, -1 line 0 comments Download
M Source/wtf/text/Base64.h View 1 3 chunks +7 lines, -13 lines 0 comments Download
M Source/wtf/text/Base64.cpp View 1 3 chunks +18 lines, -26 lines 0 comments Download

Messages

Total messages: 4 (0 generated)
do-not-use
7 years, 4 months ago (2013-08-13 08:04:59 UTC) #1
eseidel
lgtm
7 years, 4 months ago (2013-08-13 08:11:54 UTC) #2
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/ch.dumez@sisa.samsung.com/22796004/5001
7 years, 4 months ago (2013-08-13 08:13:42 UTC) #3
commit-bot: I haz the power
7 years, 4 months ago (2013-08-13 10:01:39 UTC) #4
Message was sent while issue was closed.
Change committed as 155999

Powered by Google App Engine
This is Rietveld 408576698