FontAccess: Sticky User Activation is required to enumerate

This change adds the requirement for sticky user activation for the
enumeration API to return a result. This will require sites to wait for
a user activation on session-restore or whatever causes the page to load
without user activation before accessing the API.

Also, the enumeration API now consumes the transient activation.

Bug: 1043306
Change-Id: I77e18795eaa22cc801145243b276c6fd29308d8c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2414920
Commit-Queue: Olivier Yiptong <oyiptong@chromium.org>
Reviewed-by: Balazs Engedy <engedy@chromium.org>
Reviewed-by: Martin Barbella <mbarbella@chromium.org>
Reviewed-by: Joshua Bell <jsbell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#809194}

content/browser/font_access/font_access_manager_impl.cc

@@ -8,9 +8,10 @@
 #include "base/task/post_task.h"
 #include "content/browser/font_access/font_enumeration_cache.h"
 #include "content/browser/permissions/permission_controller_impl.h"
+#include "content/browser/renderer_host/frame_tree_node.h"
+#include "content/browser/renderer_host/render_frame_host_impl.h"
 #include "content/public/browser/browser_task_traits.h"
 #include "content/public/browser/browser_thread.h"
-#include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/render_process_host.h"
 #include "third_party/blink/public/common/features.h"
 
@@ -43,7 +44,15 @@ void FontAccessManagerImpl::EnumerateLocalFonts(
 #if defined(OS_WIN) || defined(OS_LINUX) || defined(OS_CHROMEOS) || \
     defined(OS_MAC)
   const BindingContext& context = receivers_.current_context();
-  RenderFrameHost* rfh = RenderFrameHost::FromID(context.frame_id);
+  RenderFrameHostImpl* rfh = RenderFrameHostImpl::FromID(context.frame_id);
+
+  // Sticky User Activation is required for the API to function at all.
+  if (!rfh->frame_tree_node()->HasStickyUserActivation()) {
+    std::move(callback).Run(
+        blink::mojom::FontEnumerationStatus::kNeedsUserActivation,
+        base::ReadOnlySharedMemoryRegion());
+    return;
+  }
 
   auto status = PermissionControllerImpl::FromBrowserContext(
                     rfh->GetProcess()->GetBrowserContext())
@@ -56,12 +65,17 @@ void FontAccessManagerImpl::EnumerateLocalFonts(
     return;
   }
 
+  // Transient User Activation only required before showing permission prompt.
+  // This action will consume it.
   if (!rfh->HasTransientUserActivation()) {
     std::move(callback).Run(
-        blink::mojom::FontEnumerationStatus::kPermissionDenied,
+        blink::mojom::FontEnumerationStatus::kNeedsUserActivation,
         base::ReadOnlySharedMemoryRegion());
     return;
   }
+  rfh->frame_tree_node()->UpdateUserActivationState(
+      blink::mojom::UserActivationUpdateType::kConsumeTransientActivation,
+      blink::mojom::UserActivationNotificationType::kNone);
 
   PermissionControllerImpl::FromBrowserContext(
       rfh->GetProcess()->GetBrowserContext())

content/browser/font_access/font_access_manager_impl_unittest.cc

@@ -10,6 +10,7 @@
 #include "build/build_config.h"
 #include "content/browser/font_access/font_enumeration_cache.h"
 #include "content/browser/permissions/permission_controller_impl.h"
+#include "content/browser/renderer_host/frame_tree_node.h"
 #include "content/browser/renderer_host/render_frame_host_impl.h"
 #include "content/public/test/mock_permission_manager.h"
 #include "content/public/test/test_browser_context.h"
@@ -148,11 +149,21 @@ class FontAccessManagerImplTest : public RenderViewHostImplTestHarness {
         }));
   }
 
+  void SimulateStickyUserActivation() {
+    SimulateUserActivation();
+    // We'll consume the activation here because the sticky activation bit is
+    // set even if the transient bit is no longer set.
+    static_cast<RenderFrameHostImpl*>(main_rfh())
+        ->UpdateUserActivationState(
+            blink::mojom::UserActivationUpdateType::kConsumeTransientActivation,
+            blink::mojom::UserActivationNotificationType::kTest);
+  }
+
   void SimulateUserActivation() {
     static_cast<RenderFrameHostImpl*>(main_rfh())
         ->UpdateUserActivationState(
             blink::mojom::UserActivationUpdateType::kNotifyActivation,
-            blink::mojom::UserActivationNotificationType::kInteraction);
+            blink::mojom::UserActivationNotificationType::kTest);
   }
 
  protected:
@@ -188,9 +199,53 @@ void ValidateFontEnumerationBasic(FontEnumerationStatus status,
 
 }  // namespace
 
+TEST_F(FontAccessManagerImplTest, FailsIfNoStickyUserActivation) {
+  ASSERT_TRUE(manager_remote_.is_bound() && manager_remote_.is_connected());
+  AutoGrantPermission();
+
+  base::RunLoop run_loop;
+  manager_remote_->EnumerateLocalFonts(
+      base::BindLambdaForTesting([&](FontEnumerationStatus status,
+                                     base::ReadOnlySharedMemoryRegion region) {
+        EXPECT_EQ(status, FontEnumerationStatus::kNeedsUserActivation)
+            << "Sticky User Activation Required.";
+        run_loop.Quit();
+      }));
+  run_loop.Run();
+}
+
+TEST_F(FontAccessManagerImplTest, EnumerationConsumesUserActivation) {
+  ASSERT_TRUE(manager_remote_.is_bound() && manager_remote_.is_connected());
+  AskGrantPermission();
+  SimulateUserActivation();
+
+  base::RunLoop run_loop;
+  manager_remote_->EnumerateLocalFonts(
+      base::BindLambdaForTesting([&](FontEnumerationStatus status,
+                                     base::ReadOnlySharedMemoryRegion region) {
+        EXPECT_EQ(status, FontEnumerationStatus::kOk)
+            << "Font Enumeration was successful.";
+        run_loop.Quit();
+      }));
+  run_loop.Run();
+
+  AskGrantPermission();
+
+  base::RunLoop run_loop2;
+  manager_remote_->EnumerateLocalFonts(
+      base::BindLambdaForTesting([&](FontEnumerationStatus status,
+                                     base::ReadOnlySharedMemoryRegion region) {
+        EXPECT_EQ(status, FontEnumerationStatus::kNeedsUserActivation)
+            << "User Activation Required.";
+        run_loop2.Quit();
+      }));
+  run_loop2.Run();
+}
+
 TEST_F(FontAccessManagerImplTest, PreviouslyGrantedValidateEnumerationBasic) {
   ASSERT_TRUE(manager_remote_.is_bound() && manager_remote_.is_connected());
   AutoGrantPermission();
+  SimulateStickyUserActivation();
 
   base::RunLoop run_loop;
   manager_remote_->EnumerateLocalFonts(
@@ -220,16 +275,17 @@ TEST_F(FontAccessManagerImplTest, UserActivationRequiredBeforeGrant) {
   run_loop.Run();
 }
 
-TEST_F(FontAccessManagerImplTest, EnumerationPermissionDeniedIfNoActivation) {
+TEST_F(FontAccessManagerImplTest, EnumerationFailsIfNoActivation) {
   ASSERT_TRUE(manager_remote_.is_bound() && manager_remote_.is_connected());
   AskGrantPermission();
+  SimulateStickyUserActivation();
 
   base::RunLoop run_loop;
   manager_remote_->EnumerateLocalFonts(
       base::BindLambdaForTesting([&](FontEnumerationStatus status,
                                      base::ReadOnlySharedMemoryRegion region) {
-        EXPECT_EQ(status, FontEnumerationStatus::kPermissionDenied)
-            << "Permission was denied.";
+        EXPECT_EQ(status, FontEnumerationStatus::kNeedsUserActivation)
+            << "User Activation is needed.";
         run_loop.Quit();
       }));
   run_loop.Run();
@@ -254,6 +310,7 @@ TEST_F(FontAccessManagerImplTest, PermissionDeniedOnAskErrors) {
 TEST_F(FontAccessManagerImplTest, PermissionPreviouslyDeniedErrors) {
   ASSERT_TRUE(manager_remote_.is_bound() && manager_remote_.is_connected());
   AutoDenyPermission();
+  SimulateUserActivation();
 
   base::RunLoop run_loop;
   manager_remote_->EnumerateLocalFonts(

third_party/blink/public/mojom/font_access/font_access.mojom

@@ -13,6 +13,8 @@ enum FontEnumerationStatus {
   kUnimplemented,
   // The service failed due to an unexpected error.
   kUnexpectedError,
+  // Needs user activation to proceed.
+  kNeedsUserActivation,
   // The site doesn't have permission for the requested operation.
   kPermissionDenied,
 };

third_party/blink/renderer/modules/font_access/font_iterator.cc

@@ -71,15 +71,17 @@ void FontIterator::DidGetEnumerationResponse(
     FontEnumerationStatus status,
     base::ReadOnlySharedMemoryRegion region) {
   switch (status) {
+    case FontEnumerationStatus::kOk:
+      break;
     case FontEnumerationStatus::kUnimplemented:
       pending_resolver_->Reject(MakeGarbageCollected<DOMException>(
           DOMExceptionCode::kNotSupportedError,
           "Not yet supported on this platform."));
       pending_resolver_.Clear();
       return;
-    case FontEnumerationStatus::kUnexpectedError:
+    case FontEnumerationStatus::kNeedsUserActivation:
       pending_resolver_->Reject(MakeGarbageCollected<DOMException>(
-          DOMExceptionCode::kUnknownError, "An unexpected error occured."));
+          DOMExceptionCode::kSecurityError, "User activation is required."));
       pending_resolver_.Clear();
       return;
     case FontEnumerationStatus::kPermissionDenied:
@@ -88,8 +90,12 @@ void FontIterator::DidGetEnumerationResponse(
           DOMExceptionCode::kNotAllowedError, "Permission not granted."));
       pending_resolver_.Clear();
       return;
+    case FontEnumerationStatus::kUnexpectedError:
     default:
-      break;
+      pending_resolver_->Reject(MakeGarbageCollected<DOMException>(
+          DOMExceptionCode::kUnknownError, "An unexpected error occured."));
+      pending_resolver_.Clear();
+      return;
   }
   permission_status_ = PermissionStatus::GRANTED;
 

third_party/blink/web_tests/external/wpt/font-access/resources/window-tests-enumeration.js

@@ -3,11 +3,11 @@
 promise_test(async t => {
   const iterator = navigator.fonts.query();
 
-  await promise_rejects_dom(t, 'NotAllowedError', (async () => {
+  await promise_rejects_dom(t, 'SecurityError', (async () => {
                               for await (const f of iterator) {
                               }
                             })());
-}, 'iteration fails if there is no user activation');
+}, 'iteration fails if there is no sticky user activation');
 
 font_access_test(async t => {
   const iterator = navigator.fonts.query();