Skip to content

Commit

Permalink
Replace use of Skia's Base64 Encoding/Decoding logic with a copy of t…
Browse files Browse the repository at this point in the history
…he equivalent code (flutter#46543)

Skia would like to remove SkBase64.h from its public API. This ports the
same functionality into Flutter's codebase with tests.

The implementation was copied from
[Skia](https://github.com/google/skia/blob/387853af198f03b4634239fb92cac5d49fe37100/src/utils/SkBase64.cpp)
then modified to match Flutter's style and have readable tests.

In a follow-up PR, I would like to add a function to pre-flight the
calculation needed to figure out how many
bytes are needed to be allocated, to avoid the clunky double API
call (see the TODOs).

I chose to put the code in `//shell/common` at the suggestion of Flutter
devs, but it needs to be in its own source_set because it is used in a
few other places (for now) and we want to avoid dependency cycles.

## Pre-launch Checklist

- [x] I read the [Contributor Guide] and followed the process outlined
there for submitting PRs.
- [x] I read the [Tree Hygiene] wiki page, which explains my
responsibilities.
- [x] I read and followed the [Flutter Style Guide] and the [C++,
Objective-C, Java style guides].
- [ ] I listed at least one issue that this PR fixes in the description
above.
- [x] I added new tests to check the change I am making or feature I am
adding, or the PR is [test-exempt]. See [testing the engine] for
instructions on writing and running engine tests.
- [x] I updated/added relevant documentation (doc comments with `///`).
- [x] I signed the [CLA].
- [x] All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel
on [Discord].

<!-- Links -->
[Contributor Guide]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#overview
[Tree Hygiene]: https://github.com/flutter/flutter/wiki/Tree-hygiene
[test-exempt]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#tests
[Flutter Style Guide]:
https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo
[C++, Objective-C, Java style guides]:
https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
[testing the engine]:
https://github.com/flutter/flutter/wiki/Testing-the-engine
[CLA]: https://cla.developers.google.com/
[flutter/tests]: https://github.com/flutter/tests
[breaking change policy]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#handling-breaking-changes
[Discord]: https://github.com/flutter/flutter/wiki/Chat
  • Loading branch information
kjlubick authored Oct 5, 2023
1 parent edb214f commit ad83202
Show file tree
Hide file tree
Showing 12 changed files with 372 additions and 17 deletions.
1 change: 1 addition & 0 deletions ci/licenses_golden/excluded_files
Original file line number Diff line number Diff line change
Expand Up @@ -232,6 +232,7 @@
../../../flutter/runtime/no_dart_plugin_registrant_unittests.cc
../../../flutter/runtime/type_conversions_unittests.cc
../../../flutter/shell/common/animator_unittests.cc
../../../flutter/shell/common/base64_unittests.cc
../../../flutter/shell/common/context_options_unittests.cc
../../../flutter/shell/common/dl_op_spy_unittests.cc
../../../flutter/shell/common/engine_unittests.cc
Expand Down
4 changes: 4 additions & 0 deletions ci/licenses_golden/licenses_flutter
Original file line number Diff line number Diff line change
Expand Up @@ -2246,6 +2246,8 @@ ORIGIN: ../../../flutter/runtime/test_font_data.cc + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/runtime/test_font_data.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/shell/common/animator.cc + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/shell/common/animator.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/shell/common/base64.cc + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/shell/common/base64.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/shell/common/context_options.cc + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/shell/common/context_options.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/shell/common/dart_native_benchmarks.cc + ../../../flutter/LICENSE
Expand Down Expand Up @@ -5008,6 +5010,8 @@ FILE: ../../../flutter/runtime/test_font_data.cc
FILE: ../../../flutter/runtime/test_font_data.h
FILE: ../../../flutter/shell/common/animator.cc
FILE: ../../../flutter/shell/common/animator.h
FILE: ../../../flutter/shell/common/base64.cc
FILE: ../../../flutter/shell/common/base64.h
FILE: ../../../flutter/shell/common/context_options.cc
FILE: ../../../flutter/shell/common/context_options.h
FILE: ../../../flutter/shell/common/dart_native_benchmarks.cc
Expand Down
1 change: 1 addition & 0 deletions common/graphics/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ source_set("graphics") {
"//flutter/assets",
"//flutter/display_list",
"//flutter/fml",
"//flutter/shell/common:base64",
"//flutter/shell/version:version",
"//third_party/boringssl",
"//third_party/rapidjson",
Expand Down
16 changes: 8 additions & 8 deletions common/graphics/persistent_cache.cc
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,11 @@
#include "flutter/fml/mapping.h"
#include "flutter/fml/paths.h"
#include "flutter/fml/trace_event.h"
#include "flutter/shell/common/base64.h"
#include "flutter/shell/version/version.h"
#include "openssl/sha.h"
#include "rapidjson/document.h"
#include "third_party/skia/include/gpu/GrDirectContext.h"
#include "third_party/skia/include/utils/SkBase64.h"

namespace flutter {

Expand Down Expand Up @@ -169,21 +169,21 @@ sk_sp<SkData> ParseBase32(const std::string& input) {
}

sk_sp<SkData> ParseBase64(const std::string& input) {
SkBase64::Error error;
Base64::Error error;

size_t output_len;
error = SkBase64::Decode(input.c_str(), input.length(), nullptr, &output_len);
if (error != SkBase64::Error::kNoError) {
FML_LOG(ERROR) << "Base64 decode error: " << error;
error = Base64::Decode(input.c_str(), input.length(), nullptr, &output_len);
if (error != Base64::Error::kNone) {
FML_LOG(ERROR) << "Base64 decode error: " << (int)error;
FML_LOG(ERROR) << "Base64 can't decode: " << input;
return nullptr;
}

sk_sp<SkData> data = SkData::MakeUninitialized(output_len);
void* output = data->writable_data();
error = SkBase64::Decode(input.c_str(), input.length(), output, &output_len);
if (error != SkBase64::Error::kNoError) {
FML_LOG(ERROR) << "Base64 decode error: " << error;
error = Base64::Decode(input.c_str(), input.length(), output, &output_len);
if (error != Base64::Error::kNone) {
FML_LOG(ERROR) << "Base64 decode error: " << (int)error;
FML_LOG(ERROR) << "Base64 can't decode: " << input;
return nullptr;
}
Expand Down
1 change: 1 addition & 0 deletions flow/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,7 @@ if (enable_unittests) {
"//flutter/common/graphics",
"//flutter/display_list/testing:display_list_testing",
"//flutter/fml",
"//flutter/shell/common:base64",
"//flutter/testing:skia",
"//flutter/testing:testing_lib",
"//third_party/dart/runtime:libdart_jit", # for tracing
Expand Down
8 changes: 5 additions & 3 deletions flow/layers/performance_overlay_layer_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#include "flutter/flow/flow_test_utils.h"
#include "flutter/flow/raster_cache.h"
#include "flutter/flow/testing/layer_test.h"
#include "flutter/shell/common/base64.h"
#include "flutter/testing/mock_canvas.h"
#include "third_party/skia/include/core/SkData.h"
#include "third_party/skia/include/core/SkImage.h"
Expand All @@ -18,7 +19,6 @@
#include "third_party/skia/include/core/SkSurface.h"
#include "third_party/skia/include/core/SkTextBlob.h"
#include "third_party/skia/include/encode/SkPngEncoder.h"
#include "third_party/skia/include/utils/SkBase64.h"

namespace flutter {
namespace testing {
Expand Down Expand Up @@ -111,11 +111,13 @@ static void TestPerformanceOverlayLayerGold(int refresh_rate) {
wstream.write(snapshot_data->data(), snapshot_data->size());
wstream.flush();

// TODO(kjlubick) We shouldn't need to call Encode once to pre-flight the
// encode length. It should be ceil(4/3 * sksl.value->size()).
size_t b64_size =
SkBase64::Encode(snapshot_data->data(), snapshot_data->size(), nullptr);
Base64::Encode(snapshot_data->data(), snapshot_data->size(), nullptr);
sk_sp<SkData> b64_data = SkData::MakeUninitialized(b64_size + 1);
char* b64_char = static_cast<char*>(b64_data->writable_data());
SkBase64::Encode(snapshot_data->data(), snapshot_data->size(), b64_char);
Base64::Encode(snapshot_data->data(), snapshot_data->size(), b64_char);
b64_char[b64_size] = 0; // make it null terminated for printing

EXPECT_TRUE(golden_data_matches)
Expand Down
12 changes: 12 additions & 0 deletions shell/common/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,7 @@ source_set("common") {
"//flutter/fml",
"//flutter/lib/ui",
"//flutter/runtime",
"//flutter/shell/common:base64",
"//flutter/shell/profiling",
"//third_party/dart/runtime:dart_api",
"//third_party/skia",
Expand All @@ -158,6 +159,15 @@ source_set("common") {
}
}

# These are in their own source_set to avoid a dependency cycle with //common/graphics
source_set("base64") {
sources = [
"base64.cc",
"base64.h",
]
deps = [ "//flutter/fml" ]
}

template("shell_host_executable") {
executable(target_name) {
testonly = true
Expand Down Expand Up @@ -293,6 +303,7 @@ if (enable_unittests) {

sources = [
"animator_unittests.cc",
"base64_unittests.cc",
"context_options_unittests.cc",
"dl_op_spy_unittests.cc",
"engine_unittests.cc",
Expand All @@ -312,6 +323,7 @@ if (enable_unittests) {
":shell_unittests_fixtures",
"//flutter/assets",
"//flutter/common/graphics",
"//flutter/shell/common:base64",
"//flutter/shell/profiling:profiling_unittests",
"//flutter/shell/version",
"//flutter/testing:fixture_test",
Expand Down
157 changes: 157 additions & 0 deletions shell/common/base64.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,157 @@
// Copyright 2013 The Flutter Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include "flutter/shell/common/base64.h"

#include "flutter/fml/logging.h"

#include <cstdint>

#define DecodePad -2
#define EncodePad 64

static const char kDefaultEncode[] =
"ABCDEFGHIJKLMNOPQRSTUVWXYZ"
"abcdefghijklmnopqrstuvwxyz"
"0123456789+/=";

static const signed char kDecodeData[] = {
62, -1, -1, -1, 63, 52, 53, 54, 55, 56, 57, 58, 59, 60, 61, -1,
-1, -1, DecodePad, -1, -1, -1, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9,
10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25,
-1, -1, -1, -1, -1, -1, 26, 27, 28, 29, 30, 31, 32, 33, 34, 35,
36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47, 48, 49, 50, 51};

namespace flutter {

Base64::Error Base64::Decode(const void* srcv,
size_t srcLength,
void* dstv,
size_t* dstLength) {
const unsigned char* src = static_cast<const unsigned char*>(srcv);
unsigned char* dst = static_cast<unsigned char*>(dstv);

int i = 0;
bool padTwo = false;
bool padThree = false;
char unsigned const* const end = src + srcLength;
while (src < end) {
unsigned char bytes[4] = {0, 0, 0, 0};
int byte = 0;
do {
unsigned char srcByte = *src++;
if (srcByte == 0) {
*dstLength = i;
return Error::kNone;
}
if (srcByte <= ' ') {
continue; // treat as white space
}
if (srcByte < '+' || srcByte > 'z') {
return Error::kBadChar;
}
signed char decoded = kDecodeData[srcByte - '+'];
bytes[byte] = decoded;
if (decoded != DecodePad) {
if (decoded < 0) {
return Error::kBadChar;
}
byte++;
if (*src) {
continue;
}
if (byte == 0) {
*dstLength = i;
return Error::kNone;
}
if (byte == 4) {
break;
}
}
// As an optimization, if we find an equals sign
// we assume all future bytes to read are the
// appropriate number of padding equals signs.
if (byte < 2) {
return Error::kBadPadding;
}
padThree = true;
if (byte == 2) {
padTwo = true;
}
break;
} while (byte < 4);
int two = 0;
int three = 0;
if (dst) {
int one = (uint8_t)(bytes[0] << 2);
two = bytes[1];
one |= two >> 4;
two = (uint8_t)((two << 4) & 0xFF);
three = bytes[2];
two |= three >> 2;
three = (uint8_t)((three << 6) & 0xFF);
three |= bytes[3];
FML_DCHECK(one < 256 && two < 256 && three < 256);
dst[i] = (unsigned char)one;
}
i++;
if (padTwo) {
break;
}
if (dst) {
dst[i] = (unsigned char)two;
}
i++;
if (padThree) {
break;
}
if (dst) {
dst[i] = (unsigned char)three;
}
i++;
}
*dstLength = i;
return Error::kNone;
}

size_t Base64::Encode(const void* srcv, size_t length, void* dstv) {
const unsigned char* src = static_cast<const unsigned char*>(srcv);
unsigned char* dst = static_cast<unsigned char*>(dstv);

const char* encode = kDefaultEncode;
if (dst) {
size_t remainder = length % 3;
char unsigned const* const end = &src[length - remainder];
while (src < end) {
unsigned a = *src++;
unsigned b = *src++;
unsigned c = *src++;
int d = c & 0x3F;
c = (c >> 6 | b << 2) & 0x3F;
b = (b >> 4 | a << 4) & 0x3F;
a = a >> 2;
*dst++ = encode[a];
*dst++ = encode[b];
*dst++ = encode[c];
*dst++ = encode[d];
}
if (remainder > 0) {
int k1 = 0;
int k2 = EncodePad;
int a = (uint8_t)*src++;
if (remainder == 2) {
int b = *src++;
k1 = b >> 4;
k2 = (b << 2) & 0x3F;
}
*dst++ = encode[a >> 2];
*dst++ = encode[(k1 | a << 4) & 0x3F];
*dst++ = encode[k2];
*dst++ = encode[EncodePad];
}
}
return (length + 2) / 3 * 4;
}

} // namespace flutter
53 changes: 53 additions & 0 deletions shell/common/base64.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
// Copyright 2013 The Flutter Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#ifndef FLUTTER_SHELL_COMMON_BASE64_H_
#define FLUTTER_SHELL_COMMON_BASE64_H_

#include <cstddef>

namespace flutter {

struct Base64 {
public:
enum class Error {
kNone,
kBadPadding,
kBadChar,
};

/**
Base64 encodes src into dst.
Normally this is called once with 'dst' nullptr to get the required size,
then again with an allocated 'dst' pointer to do the actual encoding.
@param dst nullptr or a pointer to a buffer large enough to receive the
result
@return the required length of dst for encoding.
*/
static size_t Encode(const void* src, size_t length, void* dst);

/**
Base64 decodes src into dst.
Normally this is called once with 'dst' nullptr to get the required size,
then again with an allocated 'dst' pointer to do the actual encoding.
@param dst nullptr or a pointer to a buffer large enough to receive the
result
@param dstLength assigned the length dst is required to be. Must not be
nullptr.
*/
[[nodiscard]] static Error Decode(const void* src,
size_t srcLength,
void* dst,
size_t* dstLength);
};

} // namespace flutter

#endif // FLUTTER_SHELL_COMMON_BASE64_H_
Loading

0 comments on commit ad83202

Please sign in to comment.