[PEPC] Track instances per page by visibility
We'll keep track of PEPC instances based on their rendered states.
Registration and unregistration will be checked when elements are
inserted to/removed from the page and updated at the end of lifecycle
phases.
https://docs.google.com/document/d/1c8JZAi9t-_ZPbYXIfUzgJmsfMRLDTxgHg15GglmGB4Q/edit?resourcekey=0-rrRb6K_42FfuDC8r9mlO9A&tab=t.0
Bug: 407938235
Change-Id: Ibb725d3d408e312c0fcb60cef724cd6754d62bc6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6512933
Commit-Queue: Thomas Nguyen <tungnh@chromium.org>
Reviewed-by: Mason Freed <masonf@chromium.org>
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1461285}
diff --git a/third_party/blink/renderer/core/html/html_permission_element.cc b/third_party/blink/renderer/core/html/html_permission_element.cc
index 1429ac6..1fb09b8 100644
--- a/third_party/blink/renderer/core/html/html_permission_element.cc
+++ b/third_party/blink/renderer/core/html/html_permission_element.cc
@@ -417,21 +417,6 @@
DCHECK(RuntimeEnabledFeatures::PermissionElementEnabled(
document.GetExecutionContext()));
SetHasCustomStyleCallbacks();
- intersection_observer_ = IntersectionObserver::Create(
- GetDocument(),
- WTF::BindRepeating(&HTMLPermissionElement::OnIntersectionChanged,
- WrapWeakPersistent(this)),
- LocalFrameUkmAggregator::kPermissionElementIntersectionObserver,
- IntersectionObserver::Params{
- .thresholds = {kIntersectionThreshold},
- .semantics = IntersectionObserver::kFractionOfTarget,
- .behavior = IntersectionObserver::kDeliverDuringPostLifecycleSteps,
- .delay = base::Milliseconds(100),
- .track_visibility = true,
- .expose_occluder_id = true,
- });
-
- intersection_observer_->observe(this);
EnsureUserAgentShadowRoot();
UseCounter::Count(document, WebFeature::kHTMLPermissionElement);
}
@@ -479,16 +464,39 @@
Node::InsertionNotificationRequest HTMLPermissionElement::InsertedInto(
ContainerNode& insertion_point) {
HTMLElement::InsertedInto(insertion_point);
- MaybeRegisterPageEmbeddedPermissionControl();
+ if (!permission_descriptors_.empty()) {
+ CachedPermissionStatus::From(GetDocument().domWindow())
+ ->RegisterClient(this, permission_descriptors_);
+ }
return kInsertionDone;
}
void HTMLPermissionElement::AttachLayoutTree(AttachContext& context) {
Element::AttachLayoutTree(context);
+ if (fallback_mode_) {
+ return;
+ }
DisableClickingTemporarily(DisableReason::kRecentlyAttachedToLayoutTree,
kDefaultDisableTimeout);
CHECK(GetDocument().View());
GetDocument().View()->RegisterForLifecycleNotifications(this);
+ if (!intersection_observer_) {
+ intersection_observer_ = IntersectionObserver::Create(
+ GetDocument(),
+ WTF::BindRepeating(&HTMLPermissionElement::OnIntersectionChanged,
+ WrapWeakPersistent(this)),
+ LocalFrameUkmAggregator::kPermissionElementIntersectionObserver,
+ IntersectionObserver::Params{
+ .thresholds = {kIntersectionThreshold},
+ .semantics = IntersectionObserver::kFractionOfTarget,
+ .behavior = IntersectionObserver::kDeliverDuringPostLifecycleSteps,
+ .delay = base::Milliseconds(100),
+ .track_visibility = true,
+ .expose_occluder_id = true,
+ });
+
+ intersection_observer_->observe(this);
+ }
}
void HTMLPermissionElement::DetachLayoutTree(bool performing_reattach) {
@@ -507,15 +515,11 @@
disable_reason_expire_timer_.Stop();
}
intersection_rect_ = std::nullopt;
- if (embedded_permission_control_receiver_.is_bound()) {
- embedded_permission_control_receiver_.reset();
- }
-
- is_registered_in_browser_process_ = false;
if (LocalDOMWindow* window = GetDocument().domWindow()) {
CachedPermissionStatus::From(window)->UnregisterClient(
this, permission_descriptors_);
}
+ EnsureUnregisterPageEmbeddedPermissionControl();
}
void HTMLPermissionElement::Focus(const FocusParams& params) {
@@ -572,6 +576,14 @@
DisableReason::kIntersectionVisibilityOccludedOrDistorted);
}
+bool HTMLPermissionElement::IsRenderered() const {
+ LayoutObject* layout_object = GetLayoutObject();
+ if (!layout_object) {
+ return false;
+ }
+ return layout_object->StyleRef().Visibility() == EVisibility::kVisible;
+}
+
// static
Vector<PermissionDescriptorPtr>
HTMLPermissionElement::ParsePermissionDescriptorsForTesting(
@@ -697,16 +709,27 @@
}
}
- CachedPermissionStatus::From(GetDocument().domWindow())
- ->RegisterClient(this, permission_descriptors_);
+ if (!IsRenderered()) {
+ return false;
+ }
+
mojo::PendingRemote<EmbeddedPermissionControlClient> client;
embedded_permission_control_receiver_.Bind(
client.InitWithNewPipeAndPassReceiver(), GetTaskRunner());
+ CHECK(embedded_permission_control_receiver_.is_bound());
GetPermissionService()->RegisterPageEmbeddedPermissionControl(
mojo::Clone(permission_descriptors_), std::move(client));
return true;
}
+void HTMLPermissionElement::EnsureUnregisterPageEmbeddedPermissionControl() {
+ if (embedded_permission_control_receiver_.is_bound()) {
+ embedded_permission_control_receiver_.reset();
+ }
+
+ is_registered_in_browser_process_ = false;
+}
+
void HTMLPermissionElement::LangAttributeChanged() {
UpdateText();
HTMLElement::LangAttributeChanged();
@@ -1164,6 +1187,12 @@
return false;
}
+ // Do not check click-disabling reasons if the PEPC validation feature is
+ // disabled. This should only occur in testing scenarios.
+ if (RuntimeEnabledFeatures::BypassPepcSecurityForTestingEnabled()) {
+ return true;
+ }
+
if (!is_registered_in_browser_process()) {
AddConsoleError(
WTF::StrCat({"The permission element '", GetType(),
@@ -1175,12 +1204,6 @@
return false;
}
- // Do not check click-disabling reasons if the PEPC validation feature is
- // disabled. This should only occur in testing scenarios.
- if (RuntimeEnabledFeatures::BypassPepcSecurityForTestingEnabled()) {
- return true;
- }
-
// Remove expired reasons. If the remaining map is not empty, clicking is
// disabled. Record and log all the remaining reasons in the map in this case.
base::TimeTicks now = base::TimeTicks::Now();
@@ -1645,6 +1668,12 @@
kDefaultDisableTimeout);
}
intersection_rect_ = intersection_rect;
+
+ if (IsRenderered()) {
+ MaybeRegisterPageEmbeddedPermissionControl();
+ } else {
+ EnsureUnregisterPageEmbeddedPermissionControl();
+ }
}
gfx::Rect HTMLPermissionElement::ComputeIntersectionRectWithViewport(
@@ -1678,8 +1707,9 @@
void HTMLPermissionElement::EnableFallbackMode() {
CHECK(!fallback_mode_);
fallback_mode_ = true;
- intersection_observer_->unobserve(this);
-
+ if (intersection_observer_) {
+ intersection_observer_->unobserve(this);
+ }
// Adding this slot element will make all children of the permission element
// render, the permission element's built-in elements are removed at the same
// time.
diff --git a/third_party/blink/renderer/core/html/html_permission_element.h b/third_party/blink/renderer/core/html/html_permission_element.h
index 3b03e04e..c51c3ba 100644
--- a/third_party/blink/renderer/core/html/html_permission_element.h
+++ b/third_party/blink/renderer/core/html/html_permission_element.h
@@ -86,6 +86,7 @@
bool HasInvalidStyle() const;
bool IsOccluded() const;
+ bool IsRenderered() const;
bool granted() const { return PermissionsGranted(); }
// Given an input type, return permissions list. This method is for testing
@@ -136,6 +137,8 @@
FontSizeCanDisableElement);
FRIEND_TEST_ALL_PREFIXES(HTMLPermissionElementSimTest,
MovePEPCToAnotherDocument);
+ FRIEND_TEST_ALL_PREFIXES(HTMLPermissionElementSimTest,
+ RegisterAfterBeingVisible);
FRIEND_TEST_ALL_PREFIXES(HTMLPermissionElementLayoutChangeTest,
InvalidatePEPCAfterMove);
FRIEND_TEST_ALL_PREFIXES(HTMLPermissionElementLayoutChangeTest,
@@ -309,6 +312,9 @@
// process.
bool MaybeRegisterPageEmbeddedPermissionControl();
+ // Ensure we reset the PEPC IPC endpoint.
+ void EnsureUnregisterPageEmbeddedPermissionControl();
+
// blink::Element implements
void AttributeChanged(const AttributeModificationParams& params) override;
void DidAddUserAgentShadowRoot(ShadowRoot&) override;
diff --git a/third_party/blink/renderer/core/html/html_permission_element_test.cc b/third_party/blink/renderer/core/html/html_permission_element_test.cc
index d8af608f..3513ea3b 100644
--- a/third_party/blink/renderer/core/html/html_permission_element_test.cc
+++ b/third_party/blink/renderer/core/html/html_permission_element_test.cc
@@ -450,6 +450,7 @@
}
GetDocument().body()->AppendChild(permission_element);
GetDocument().UpdateStyleAndLayout(DocumentUpdateReason::kTest);
+ GetDocument().View()->UpdateAllLifecyclePhasesForTest();
return permission_element;
}
@@ -559,6 +560,7 @@
AtomicString("width: auto; height: auto"));
GetDocument().body()->AppendChild(permission_element);
GetDocument().UpdateStyleAndLayout(DocumentUpdateReason::kTest);
+ GetDocument().View()->UpdateAllLifecyclePhasesForTest();
EXPECT_EQ(
data.expected_text,
permission_element->permission_text_span_for_testing()->innerText());
@@ -598,6 +600,7 @@
permission_service()->NotifyPermissionStatusChange(
PermissionName::GEOLOCATION, MojoPermissionStatus::ASK);
GetDocument().UpdateStyleAndLayout(DocumentUpdateReason::kTest);
+ GetDocument().View()->UpdateAllLifecyclePhasesForTest();
EXPECT_EQ(
data.expected_text_ask,
permission_element->permission_text_span_for_testing()->innerText());
@@ -605,6 +608,7 @@
permission_service()->NotifyPermissionStatusChange(
PermissionName::GEOLOCATION, MojoPermissionStatus::GRANTED);
GetDocument().UpdateStyleAndLayout(DocumentUpdateReason::kTest);
+ GetDocument().View()->UpdateAllLifecyclePhasesForTest();
EXPECT_EQ(
data.expected_text_allowed,
permission_element->permission_text_span_for_testing()->innerText());
@@ -661,6 +665,7 @@
permission_element->setAttribute(html_names::kPreciselocationAttr,
AtomicString(""));
}
+ UpdateAllLifecyclePhasesForTest();
RegistrationWaiter(permission_element).Wait();
EXPECT_EQ(
data.expected_text,
@@ -989,6 +994,7 @@
}
document.body()->AppendChild(permission_element);
document.UpdateStyleAndLayout(DocumentUpdateReason::kTest);
+ GetDocument().View()->UpdateAllLifecyclePhasesForTest();
return permission_element;
}
@@ -1081,7 +1087,6 @@
static_cast<frame_test_helpers::TestWebFrameClient*>(
first_child_frame->Client())
->ConsoleMessages();
- EXPECT_EQ(first_console_messages.size(), 2u);
EXPECT_TRUE(first_console_messages.front().Contains(
"is not allowed in the current context due to PermissionsPolicy"));
first_console_messages.clear();
@@ -1214,6 +1219,36 @@
}
}
+TEST_F(HTMLPermissionElementSimTest, RegisterAfterBeingVisible) {
+ SimRequest main_resource("https://example.test/", "text/html");
+ LoadURL("https://example.test/");
+ main_resource.Complete(R"HTML(
+ <body>
+ <permission
+ style='display:block; visibility:hidden'
+ id='camera'></permission>
+ </body>
+ )HTML");
+
+ Compositor().BeginFrame();
+ auto* permission_element = To<HTMLPermissionElement>(
+ GetDocument().QuerySelector(AtomicString("permission")));
+ GetDocument().View()->UpdateAllLifecyclePhasesForTest();
+ EXPECT_FALSE(permission_element->is_registered_in_browser_process());
+ permission_element->setAttribute(html_names::kTypeAttr,
+ AtomicString("camera"));
+ GetDocument().View()->UpdateAllLifecyclePhasesForTest();
+ EXPECT_FALSE(permission_element->is_registered_in_browser_process());
+ permission_element->setAttribute(html_names::kStyleAttr,
+ AtomicString("visibility:visible;"));
+ GetDocument().View()->UpdateAllLifecyclePhasesForTest();
+ RegistrationWaiter(permission_element).Wait();
+ permission_element->setAttribute(html_names::kStyleAttr,
+ AtomicString("display: none"));
+ GetDocument().View()->UpdateAllLifecyclePhasesForTest();
+ EXPECT_FALSE(permission_element->is_registered_in_browser_process());
+}
+
class HTMLPermissionElementDispatchValidationEventTest
: public HTMLPermissionElementSimTest {
public:
@@ -1234,6 +1269,7 @@
/*should_defer*/ true);
document.body()->AppendChild(permission_element);
document.UpdateStyleAndLayout(DocumentUpdateReason::kTest);
+ GetDocument().View()->UpdateAllLifecyclePhasesForTest();
DeferredChecker checker(permission_element, &MainFrame());
checker.CheckConsoleMessageAtIndex(0u, kValidationStatusChangeEvent);
EXPECT_FALSE(permission_element->isValid());
@@ -1245,6 +1281,8 @@
RegistrationWaiter(permission_element).Wait();
permission_service()->set_should_defer_registered_callback(
/*should_defer*/ false);
+ checker.CheckConsoleMessageAtIndex(1u, kValidationStatusChangeEvent);
+ ConsoleMessages().clear();
return permission_element;
}
@@ -1255,9 +1293,8 @@
// Test receiving event after registration
TEST_F(HTMLPermissionElementDispatchValidationEventTest, Registration) {
auto* permission_element = CreateElementAndWaitForRegistration();
- DeferredChecker checker(permission_element, &MainFrame());
- checker.CheckConsoleMessageAtIndex(1u, kValidationStatusChangeEvent);
- EXPECT_TRUE(permission_element->isValid());
+ EXPECT_TRUE(
+ base::test::RunUntil([&]() { return permission_element->isValid(); }));
}
// Test receiving event after several times disabling (temporarily or
@@ -1284,10 +1321,9 @@
for (const auto& data : kTestData) {
auto* permission_element = CreateElementAndWaitForRegistration();
DeferredChecker checker(permission_element, &MainFrame());
- checker.CheckConsoleMessageAtIndex(1u, kValidationStatusChangeEvent);
EXPECT_TRUE(permission_element->isValid());
permission_element->DisableClickingIndefinitely(data.reason);
- checker.CheckConsoleMessageAtIndex(2u, kValidationStatusChangeEvent);
+ checker.CheckConsoleMessageAtIndex(0u, kValidationStatusChangeEvent);
EXPECT_FALSE(permission_element->isValid());
EXPECT_EQ(permission_element->invalidReason(),
data.expected_invalid_reason);
@@ -1305,7 +1341,7 @@
EXPECT_FALSE(permission_element->isValid());
EXPECT_EQ(permission_element->invalidReason(),
data.expected_invalid_reason);
- checker.CheckConsoleMessageAtIndex(3u, kValidationStatusChangeEvent);
+ checker.CheckConsoleMessageAtIndex(1u, kValidationStatusChangeEvent);
EXPECT_TRUE(permission_element->isValid());
// Calling |EnableClickingAfterDelay| for a reason that is currently *not*
// disabling clicking does not do anything.
@@ -1313,11 +1349,11 @@
checker.CheckNoNewMessagesAfterDelay(kSmallTimeout);
permission_element->DisableClickingTemporarily(data.reason, kSmallTimeout);
- checker.CheckConsoleMessageAtIndex(4u, kValidationStatusChangeEvent);
+ checker.CheckConsoleMessageAtIndex(2u, kValidationStatusChangeEvent);
EXPECT_FALSE(permission_element->isValid());
EXPECT_EQ(permission_element->invalidReason(),
data.expected_invalid_reason);
- checker.CheckConsoleMessageAtIndex(5u, kValidationStatusChangeEvent);
+ checker.CheckConsoleMessageAtIndex(3u, kValidationStatusChangeEvent);
EXPECT_TRUE(permission_element->isValid());
GetDocument().body()->RemoveChild(permission_element);
@@ -1332,19 +1368,18 @@
ChangeReasonRestartTimer) {
auto* permission_element = CreateElementAndWaitForRegistration();
DeferredChecker checker(permission_element, &MainFrame());
- checker.CheckConsoleMessageAtIndex(1u, kValidationStatusChangeEvent);
EXPECT_TRUE(permission_element->isValid());
permission_element->DisableClickingTemporarily(
HTMLPermissionElement::DisableReason::kRecentlyAttachedToLayoutTree,
kSmallTimeout);
- checker.CheckConsoleMessageAtIndex(2u, kValidationStatusChangeEvent);
+ checker.CheckConsoleMessageAtIndex(0u, kValidationStatusChangeEvent);
EXPECT_FALSE(permission_element->isValid());
EXPECT_EQ(permission_element->invalidReason(), "recently_attached");
permission_element->DisableClickingTemporarily(
HTMLPermissionElement::DisableReason::kInvalidStyle, kDefaultTimeout);
// Reason change to the "longest alive" reason, in this case is
// `kInvalidStyle`
- checker.CheckConsoleMessageAtIndex(3u, kValidationStatusChangeEvent);
+ checker.CheckConsoleMessageAtIndex(1u, kValidationStatusChangeEvent);
EXPECT_FALSE(permission_element->isValid());
EXPECT_EQ(permission_element->invalidReason(), "style_invalid");
permission_element->DisableClickingTemporarily(
@@ -1354,10 +1389,10 @@
EXPECT_EQ(permission_element->invalidReason(), "style_invalid");
permission_element->EnableClickingAfterDelay(
HTMLPermissionElement::DisableReason::kInvalidStyle, kSmallTimeout);
- checker.CheckConsoleMessageAtIndex(4u, kValidationStatusChangeEvent);
+ checker.CheckConsoleMessageAtIndex(2u, kValidationStatusChangeEvent);
EXPECT_FALSE(permission_element->isValid());
EXPECT_EQ(permission_element->invalidReason(), "recently_attached");
- checker.CheckConsoleMessageAtIndex(5u, kValidationStatusChangeEvent);
+ checker.CheckConsoleMessageAtIndex(3u, kValidationStatusChangeEvent);
EXPECT_TRUE(permission_element->isValid());
}
@@ -1367,13 +1402,12 @@
DisableEnableClickingDifferentReasons) {
auto* permission_element = CreateElementAndWaitForRegistration();
DeferredChecker checker(permission_element, &MainFrame());
- checker.CheckConsoleMessageAtIndex(1u, kValidationStatusChangeEvent);
EXPECT_TRUE(permission_element->isValid());
permission_element->DisableClickingTemporarily(
HTMLPermissionElement::DisableReason::
kIntersectionVisibilityOutOfViewPortOrClipped,
kDefaultTimeout);
- checker.CheckConsoleMessageAtIndex(2u, kValidationStatusChangeEvent);
+ checker.CheckConsoleMessageAtIndex(0u, kValidationStatusChangeEvent);
EXPECT_FALSE(permission_element->isValid());
EXPECT_EQ(permission_element->invalidReason(),
"intersection_out_of_viewport_or_clipped");
@@ -1383,7 +1417,7 @@
HTMLPermissionElement::DisableReason::kInvalidStyle);
// `invalidReason` change from temporary `intersection` to indefinitely
// `style`
- checker.CheckConsoleMessageAtIndex(3u, kValidationStatusChangeEvent);
+ checker.CheckConsoleMessageAtIndex(1u, kValidationStatusChangeEvent);
EXPECT_FALSE(permission_element->isValid());
EXPECT_EQ(permission_element->invalidReason(), "style_invalid");
checker.CheckNoNewMessagesAfterDelay(kDefaultTimeout);
@@ -1399,11 +1433,11 @@
permission_element->EnableClicking(
HTMLPermissionElement::DisableReason::kInvalidStyle);
// `invalidReason` change from `style` to temporary `intersection`
- checker.CheckConsoleMessageAtIndex(4u, kValidationStatusChangeEvent);
+ checker.CheckConsoleMessageAtIndex(2u, kValidationStatusChangeEvent);
EXPECT_FALSE(permission_element->isValid());
EXPECT_EQ(permission_element->invalidReason(),
"intersection_out_of_viewport_or_clipped");
- checker.CheckConsoleMessageAtIndex(5u, kValidationStatusChangeEvent);
+ checker.CheckConsoleMessageAtIndex(3u, kValidationStatusChangeEvent);
EXPECT_TRUE(permission_element->isValid());
}
@@ -1489,7 +1523,6 @@
static_cast<frame_test_helpers::TestWebFrameClient*>(
first_child_frame->Client())
->ConsoleMessages();
- EXPECT_EQ(first_console_messages.size(), 2u);
EXPECT_TRUE(first_console_messages.front().Contains(
"is not allowed without the CSP 'frame-ancestors' directive present."));
first_console_messages.clear();
