Skip to content

Commit

Permalink
Reland "[webauthn] Port fingerprint enrollment assets to lottie"
Browse files Browse the repository at this point in the history
This is a reland of 98450e6

The original change uncovered a bug by changing the timing of requests.
If <cr-lottie> was detached while the animation request was in-flight,
the worker would be terminated but the onreadystatus change callback
would still fire and try to send a message to the terminated worker. Fix
this by aborting the request when detaching.

The reland also fixes a similar racy issue when changing the animation
URL. If the URL changes before the first one had a chance to load, then
whichever request comes last would be the one shown. This fixes that
issue by cancelling the first request before starting the new one.

Added tests for both issues.

Bug: 1104105

Original change's description:
> [webauthn] Port fingerprint enrollment assets to lottie
>
> Replace the fingerprint enrollment APNGs on the webauthn fingerprint
> enrollment page by lottie versions. This shaves ~200kb and adds a
> transparent background to the fingerprint animation that partially fixes
> issue 1041877.
>
> Also, add "hidden" and "singleLoop" attributes to lottie to match the
> APNG img tag behaviour; and update the wrapper to allow dynamically
> loading another animation into the same element.
>
> Finally, this updates the settings UI worker CSP to allow the lottie
> worker.
>
> Fixed: 1082312
> Change-Id: I0c92663ec6e1bfd76c03f995b6e96d210f26173f
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2116557
> Commit-Queue: Nina Satragno <nsatragno@chromium.org>
> Reviewed-by: Malay Keshav <malaykeshav@chromium.org>
> Reviewed-by: dpapad <dpapad@chromium.org>
> Reviewed-by: Mitsuru Oshima <oshima@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#786933}

Change-Id: I6686e11295d7c0154659ae5c97cdf1cb773344f5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2290847
Reviewed-by: dpapad <dpapad@chromium.org>
Reviewed-by: Mitsuru Oshima <oshima@chromium.org>
Reviewed-by: Malay Keshav <malaykeshav@chromium.org>
Commit-Queue: Nina Satragno <nsatragno@chromium.org>
Cr-Commit-Position: refs/heads/master@{#788790}
  • Loading branch information
nsatragno authored and Commit Bot committed Jul 15, 2020
1 parent b7afea8 commit 221cc44
Show file tree
Hide file tree
Showing 18 changed files with 357 additions and 141 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@ const test::UIPath kFingerprintPositionPage = {"fingerprint-setup",
const test::UIPath kProgressPage = {"fingerprint-setup",
"startFingerprintEnroll"};
const test::UIPath kFingerprintArc = {"fingerprint-setup", "arc"};
const test::UIPath kCheckmarkAnimation = {"fingerprint-setup", "arc",
"checkmarkAnimation"};
const test::UIPath kScanningAnimation = {"fingerprint-setup", "arc",
"scanningAnimation"};
const test::UIPath kDoneButton = {"fingerprint-setup", "done"};
const test::UIPath kDoItLaterButton = {"fingerprint-setup", "setupLater"};
const test::UIPath kNextButtonOnStart = {"fingerprint-setup", "next"};
Expand All @@ -40,6 +40,9 @@ const test::UIPath kAddAnotherFingerButton = {"fingerprint-setup",
"addAnotherFinger"};

constexpr char kTestFingerprintDataString[] = "testFinger";
constexpr char kAnimationUrlAttribute[] = "animationUrl";
constexpr char kCheckmarkAnimationUrl[] =
"chrome://theme/IDR_FINGERPRINT_COMPLETE_TICK";

int kMaxAllowedFingerprints = 3;

Expand Down Expand Up @@ -104,7 +107,9 @@ class FingerprintSetupTest : public OobeBaseTest {
test::OobeJS().ExpectVisiblePath(kFingerprintArc);
test::OobeJS().CreateVisibilityWaiter(true, kDoneButton)->Wait();
test::OobeJS().ExpectHiddenPath(kSkipButtonOnProgress);
test::OobeJS().ExpectVisiblePath(kCheckmarkAnimation);
test::OobeJS().ExpectVisiblePath(kScanningAnimation);
test::OobeJS().ExpectAttributeEQ(kAnimationUrlAttribute, kScanningAnimation,
std::string(kCheckmarkAnimationUrl));
test::OobeJS().ExpectVisiblePath(kAddAnotherFingerButton);
}

Expand Down
2 changes: 2 additions & 0 deletions chrome/browser/ui/webui/settings/settings_ui.cc
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,8 @@ SettingsUI::SettingsUI(content::WebUI* web_ui)
Profile* profile = Profile::FromWebUI(web_ui);
content::WebUIDataSource* html_source =
content::WebUIDataSource::Create(chrome::kChromeUISettingsHost);
html_source->OverrideContentSecurityPolicy(
network::mojom::CSPDirectiveName::WorkerSrc, "worker-src blob: 'self';");

AddSettingsPageUIHandler(std::make_unique<AppearanceHandler>(web_ui));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -596,6 +596,7 @@ CrElementsLottieTest.prototype = {

/** @override */
extraLibraries: CrElementsBrowserTest.prototype.extraLibraries.concat([
'../mock_controller.js',
'../test_util.js',
'cr_lottie_tests.js',
]),
Expand Down
Loading

0 comments on commit 221cc44

Please sign in to comment.