From 0bffbd477e771849bfb26c2312b1c4ab3dda3107 Mon Sep 17 00:00:00 2001 From: Yi Gu Date: Mon, 25 Sep 2023 18:00:45 +0000 Subject: [PATCH] [FedCM] Check top frame's visibility instead of iframe's Currently we only proceed with FedCM API if the render frame is visible. This makes sense when FedCM is used in the top frame. However, it's common for top frame to embed a cross-origin iframe, possibly invisible, to invoke FedCM API. This patch checks the top frame's visibility instead of the iframe's to make sure FedCM API can be used as long as the top frame is visible. (cherry picked from commit 2deafcec08683f6de9287c7c1f1da5e582b8da23) Change-Id: I299f1d3fb6dfb50b45cb5323efc6a97c95d45132 Bug: 1481308 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4860279 Reviewed-by: Zachary Tan Commit-Queue: Yi Gu Cr-Original-Commit-Position: refs/heads/main@{#1195567} Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4892208 Auto-Submit: Yi Gu Commit-Queue: Zachary Tan Cr-Commit-Position: refs/branch-heads/5993@{#785} Cr-Branched-From: 511350718e646be62331ae9d7213d10ec320d514-refs/heads/main@{#1192594} --- .../browser/webid/federated_auth_request_impl.cc | 14 +++++++------- .../credential-management/fedcm-iframe.https.html | 13 ++++++++++++- 2 files changed, 19 insertions(+), 8 deletions(-) diff --git a/content/browser/webid/federated_auth_request_impl.cc b/content/browser/webid/federated_auth_request_impl.cc index 2a8a19580f6abb..b2717870cd40ec 100644 --- a/content/browser/webid/federated_auth_request_impl.cc +++ b/content/browser/webid/federated_auth_request_impl.cc @@ -399,6 +399,11 @@ absl::optional GetIframeOriginForDisplay( return iframe_for_display; } +bool IsFrameVisible(RenderFrameHost* frame) { + return frame && frame->IsActive() && + frame->GetVisibilityState() == content::PageVisibilityState::kVisible; +} + } // namespace FederatedAuthRequestImpl::FetchData::FetchData() = default; @@ -1189,9 +1194,7 @@ void FederatedAuthRequestImpl::MaybeShowAccountsDialog() { // if the RenderFrameHost is hidden because the user does not seem interested // in the contents of the current page. if (!fetch_data_.for_idp_signin) { - bool is_visible = (render_frame_host().IsActive() && - render_frame_host().GetVisibilityState() == - content::PageVisibilityState::kVisible); + bool is_visible = IsFrameVisible(render_frame_host().GetMainFrame()); fedcm_metrics_->RecordWebContentsVisibilityUponReadyToShowDialog( is_visible); @@ -1377,10 +1380,7 @@ void FederatedAuthRequestImpl::HandleAccountsFetchFailure( return; } - bool is_visible = (render_frame_host().IsActive() && - render_frame_host().GetVisibilityState() == - content::PageVisibilityState::kVisible); - if (!is_visible) { + if (!IsFrameVisible(render_frame_host().GetMainFrame())) { CompleteRequestWithError(FederatedAuthRequestResult::kErrorRpPageNotVisible, TokenStatus::kRpPageNotVisible, /*should_delay_callback=*/true); diff --git a/third_party/blink/web_tests/external/wpt/credential-management/fedcm-iframe.https.html b/third_party/blink/web_tests/external/wpt/credential-management/fedcm-iframe.https.html index 964ebf4c44df4e..dc0c17dea695e6 100644 --- a/third_party/blink/web_tests/external/wpt/credential-management/fedcm-iframe.https.html +++ b/third_party/blink/web_tests/external/wpt/credential-management/fedcm-iframe.https.html @@ -16,13 +16,16 @@ const remoteBaseURL = host.HTTPS_REMOTE_ORIGIN + basePath; const localhostBaseURL = "http://localhost:" + host.HTTP_PORT + basePath; -async function createIframeAndWaitForMessage(test, iframeUrl, setPermissionPolicy) { +async function createIframeAndWaitForMessage(test, iframeUrl, setPermissionPolicy, style = "") { const messageWatcher = new EventWatcher(test, window, "message"); var iframe = document.createElement("iframe"); iframe.src = iframeUrl; if (setPermissionPolicy) { iframe.allow = "identity-credentials-get"; } + if (style !== "") { + iframe.style = style; + } document.body.appendChild(iframe); const message = await messageWatcher.wait_for("message"); return message.data; @@ -44,6 +47,14 @@ assert_equals(message.token, "token"); }, "FedCM enabled in 2 level deep nested iframe. FedCM should be enabled regardless of iframe nesting depth"); +fedcm_test(async t => { + const message = await createIframeAndWaitForMessage( + t, remoteBaseURL + "support/fedcm-iframe.html", + /*setPermissionPolicy=*/true, /*style=*/"display:none;"); + assert_equals(message.result, "Pass"); + assert_equals(message.token, "token"); +}, "FedCM enabled in invisible iframe. FedCM should be enabled as long as the top frame is visible"); + fedcm_test(async t => { const message = await createIframeAndWaitForMessage( t, remoteBaseURL + "support/fedcm-iframe-level2.html",