[webkit-changes] [WebKit/WebKit] 58a545: [Remote Inspection] Frequent targeted element requ...

[Wenson Hsieh]

  Branch: refs/heads/main
  Home:   https://github.com/WebKit/WebKit
  Commit: 58a545029fd142c73099d59e6e3a601a6c034dc4
      https://github.com/WebKit/WebKit/commit/58a545029fd142c73099d59e6e3a601a6c034dc4
  Author: Wenson Hsieh <wenson_hsieh at apple.com>
  Date:   2024-04-03 (Wed, 03 Apr 2024)

  Changed paths:
    M Source/WebCore/page/ElementTargetingController.cpp
    M Source/WebCore/page/ElementTargetingController.h
    M Tools/TestWebKitAPI/Tests/WebKitCocoa/ElementTargetingTests.mm
    M Tools/TestWebKitAPI/Tests/WebKitCocoa/element-targeting-2.html

  Log Message:
  -----------
  [Remote Inspection] Frequent targeted element requests can break automatic visibility adjustment
https://bugs.webkit.org/show_bug.cgi?id=272068

Reviewed by Tim Horton.

Currently, `m_pendingAdjustmentClientRects` cleared out and recomputed in
`ElementTargetingController` when the WebKit client requests targeted element information; this
cache is then consulted when the client goes to adjust visibility for these previously requested
elements, to update visibility adjustment coverage regions (in client coordinates). By clearing out
`m_pendingAdjustmentClientRects`, we avoid unbounded growth in this map, as the user targets
elements around the page.

However, this strategy has a subtle flaw — it's possible for the client to perform another targeted
element request in between the first targeting request, and visibility adjustment; in this case, if
the request finds different hit-tested elements (e.g. in the case where the targeted element has
already been unparented), we'll end up with no cached client rects at adjustent time, since the
second request would've cleared out the cached rects.

Fix this race by not clearing out this client rect cache during each targeting request; instead, we
prevent unbounded growth by scheduling a timer to perform cache cleanup, once no more targeting
requests are received. This works well in practice, since the cache doesn't get very large to begin
with (in real use cases), so there's no need to aggressively clear out the map.

* Source/WebCore/page/ElementTargetingController.cpp:
(WebCore::ElementTargetingController::findTargets):

Restart the client rect cleanup timer whenever we receive another request to target elements. After
a somewhat-arbitrary 15 seconds of inactivity, purge the client rect cache.

(WebCore::ElementTargetingController::adjustVisibility):
(WebCore::ElementTargetingController::reset):
(WebCore::ElementTargetingController::cleanUpAdjustmentClientRects):
* Source/WebCore/page/ElementTargetingController.h:

Add a timer that cleans up `m_recentAdjustmentClientRects` when fired. Additionally, rename what is
currently `m_pendingAdjustmentClientRects` to `m_recentAdjustmentClientRects`, now that this map no
longer tracks only the last set of targeted elements that are pending visibility adjustment.

* Tools/TestWebKitAPI/Tests/WebKitCocoa/ElementTargetingTests.mm:
(TestWebKitAPI::TEST(ElementTargeting, NearbyOutOfFlowElements)):
(TestWebKitAPI::TEST(ElementTargeting, AdjustVisibilityForUnparentedElement)):
* Tools/TestWebKitAPI/Tests/WebKitCocoa/element-targeting-2.html:

Augment an existing API test to cover this corner case.

Canonical link: https://commits.webkit.org/277038@main



To unsubscribe from these emails, change your notification settings at https://github.com/WebKit/WebKit/settings/notifications