mirror of
https://github.com/block/buzz.git
synced 2026-11-14 13:21:02 +00:00
perf(mobile): skip avatar URL parsing for non-HTTP URLs (#9448)
Every avatar build checks its URL: `parseAnimatedAvatarUrl` scans for the `#buzz-anim=` fragment and parses both halves, and `usesInsetAgentArtwork` also runs `Uri.tryParse` and a regex. For an ordinary URL that's a few microseconds. Some profile pictures are `data:image/png;base64,...` URLs, though, and those cost about 0.7 ms per check for 100 KB on a Mac, because the fragment search and `Uri` normalization read the whole string. Channel screens, the native navigation bar payload and the DM header portrait run these checks on every build, so a channel with such members pays for them frame after frame. Both functions can only answer yes for an HTTP(S) URL, so they now first look at the first six characters and return early unless the URL starts with `http:` or `https:` (in any case). A data URL is turned away without reading the rest of it. The authority is still checked by `Uri` as before, which also accepts backslashes there (`https:\\host`). ### Validation Added test pinning the answers this check must keep: an `https://` preset poster is inset artwork in any case and with backslash authorities (`https:\\`, `https:/\`, `https:\/`), while `http://`, a leading space, a data URL, an empty string and null are not; an animated avatar URL still is. Existing animated avatar tests (including a data-URL poster with a `#buzz-anim=` fragment) pass; full mobile test suite and `flutter analyze` pass. On an iPhone 12 mini signed in to Block's Builderlab (instrumented profile build), opening a channel whose members include two data-URL avatars (100 KB and 150 KB): over the first 7 seconds, the CPU profile spent 69 ms in these checks before this change and none with it (0 of 647 samples). ### Regressions None expected. Every URL the new check turns away already got the same answer before: `_isHttpUrl` and the preset check both need scheme `http` or `https`, which `Uri` only reports for a string that starts with `http:` or `https:` (it lowercases the scheme, so case doesn't matter, and a leading space already parsed to no scheme). Reviewers compared the old and new functions on several thousand generated URLs (case, whitespace, IDN, punycode, backslash forms) and found no differences. --------- Signed-off-by: Tom Brow <tomb@squareup.com> Co-authored-by: Tom Brow <tomb@squareup.com> Co-authored-by: studio <dc2fc481343672df31a689840c09299972e704dac592e0da6f18486646a7ee3e@buzz.block.builderlab.xyz>
This commit is contained in:
@@ -31,7 +31,7 @@ String buildAnimatedAvatarUrl(String posterUrl, String animationUrl) =>
|
||||
/// Returns `null` when the poster or animation URL is missing, malformed, or
|
||||
/// does not use HTTP(S).
|
||||
AnimatedAvatarDescriptor? parseAnimatedAvatarUrl(String? url) {
|
||||
if (url == null || url.isEmpty) return null;
|
||||
if (url == null || !_startsWithHttp(url)) return null;
|
||||
|
||||
final separatorIndex = url.indexOf(_animatedAvatarSeparator);
|
||||
if (separatorIndex <= 0) return null;
|
||||
@@ -58,6 +58,15 @@ AnimatedAvatarDescriptor? parseAnimatedAvatarUrl(String? url) {
|
||||
);
|
||||
}
|
||||
|
||||
/// Whether [url] has an HTTP(S) scheme, checked without reading the rest of
|
||||
/// it. Avatars are checked on every build, and a `data:` profile picture of
|
||||
/// 100 KB costs about a millisecond to scan and parse. The authority is left to
|
||||
/// `Uri`, which also accepts backslashes there.
|
||||
bool _startsWithHttp(String url) {
|
||||
final head = url.substring(0, url.length < 6 ? url.length : 6).toLowerCase();
|
||||
return head.startsWith('http:') || head.startsWith('https:');
|
||||
}
|
||||
|
||||
bool _isHttpUrl(String value) {
|
||||
final uri = Uri.tryParse(value);
|
||||
return uri != null &&
|
||||
@@ -68,8 +77,9 @@ bool _isHttpUrl(String value) {
|
||||
/// Character presets can be saved as plain poster URLs without buzz-anim.
|
||||
/// Keep photographs and ordinary emoji avatars at their existing size.
|
||||
bool usesInsetAgentArtwork(String? url) {
|
||||
if (url == null || !_startsWithHttp(url)) return false;
|
||||
if (parseAnimatedAvatarUrl(url) != null) return true;
|
||||
final uri = url == null ? null : Uri.tryParse(url);
|
||||
final uri = Uri.tryParse(url);
|
||||
return uri != null &&
|
||||
uri.scheme == 'https' &&
|
||||
uri.host == 'dwwgwmfqqjotj.cloudfront.net' &&
|
||||
|
||||
@@ -40,4 +40,25 @@ void main() {
|
||||
isNull,
|
||||
);
|
||||
});
|
||||
|
||||
test('only HTTP(S) URLs, in any case, can be preset artwork', () {
|
||||
const preset = 'dwwgwmfqqjotj.cloudfront.net/avatars/a/poster/b/c.png';
|
||||
expect(usesInsetAgentArtwork('https://$preset'), isTrue);
|
||||
expect(usesInsetAgentArtwork('HTTPS://$preset'), isTrue);
|
||||
expect(usesInsetAgentArtwork(r'https:\\' + preset), isTrue);
|
||||
expect(usesInsetAgentArtwork(r'https:/\' + preset), isTrue);
|
||||
expect(usesInsetAgentArtwork(r'https:\/' + preset), isTrue);
|
||||
expect(usesInsetAgentArtwork('http://$preset'), isFalse);
|
||||
expect(usesInsetAgentArtwork(' https://$preset'), isFalse);
|
||||
expect(
|
||||
usesInsetAgentArtwork('data:image/png;base64,${'A' * 1000}'),
|
||||
isFalse,
|
||||
);
|
||||
expect(usesInsetAgentArtwork(''), isFalse);
|
||||
expect(usesInsetAgentArtwork(null), isFalse);
|
||||
expect(
|
||||
usesInsetAgentArtwork(buildAnimatedAvatarUrl(posterUrl, animationUrl)),
|
||||
isTrue,
|
||||
);
|
||||
});
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user