Skip to content

Commit

Permalink
[Remote Inspection] Refactor some element selection heuristics
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=273914
rdar://127633767

Reviewed by Aditya Keerthi.

Refactor some logic for finding elements in Web Inspector using Develop > Start Element Selection,
and when right clicking on an element in the tree view and selecting `Toggle Visibility`.

* Source/WebCore/loader/DocumentLoader.h:
(WebCore::DocumentLoader::visibilityAdjustmentSelectors const):
(WebCore::DocumentLoader::setVisibilityAdjustmentSelectors):
* Source/WebCore/page/ElementTargetingController.cpp:
(WebCore::ElementTargetingController::extractTargets):

Adjust this to avoid surfacing nearby elements that are already children of a targeted element,
since they're redundant.

(WebCore::ElementTargetingController::adjustVisibility):

Make this take a list of `TargetedElementAdjustment` structs, each of which contains a pair of IDs
that represents the targeted element, as well as selectors corresponding to that element.

(WebCore::ElementTargetingController::adjustVisibilityInRepeatedlyTargetedRegions):

Hoist code to initialize `m_visibilityAdjustmentSelectors` into the call site here, and also add
logic to schedule a timer to periodically check the list of selectors ~once per second, 10 seconds
after page load.

(WebCore::ElementTargetingController::applyVisibilityAdjustmentFromSelectors):

Rename `m_remainingVisibilityAdjustmentSelectors` to just `m_visibilityAdjustmentSelectors`, and
change how it works. Instead of removing matched elements from this set and clearing the whole set
after a 30 seconds, keep the members of this set intact over the course of page load. Accumulate
more entries in this list when toggling element visibility using `adjustVisibility` below.

(WebCore::ElementTargetingController::reset):
(WebCore::ElementTargetingController::resetVisibilityAdjustments):
(WebCore::ElementTargetingController::selectorBasedVisibilityAdjustmentTimerFired):
* Source/WebCore/page/ElementTargetingController.h:
* Source/WebCore/page/ElementTargetingTypes.h:

Add and deploy some type aliases to make these nested templated types easier to read.

* Source/WebKit/Scripts/webkit/messages.py:
(headers_for_type):
* Source/WebKit/Shared/WebCoreArgumentCoders.serialization.in:
* Source/WebKit/Shared/WebsitePoliciesData.h:
* Source/WebKit/Shared/WebsitePoliciesData.serialization.in:
* Source/WebKit/UIProcess/API/Cocoa/WKWebpagePreferences.mm:
(-[WKWebpagePreferences _setVisibilityAdjustmentSelectorsIncludingShadowHosts:]):

Deploy the type aliases above in more places.

* Source/WebKit/UIProcess/WebPageProxy.cpp:
(WebKit::WebPageProxy::resetVisibilityAdjustmentsForTargetedElements):
(WebKit::WebPageProxy::adjustVisibilityForTargetedElements):
(WebKit::extractIdentifiers): Deleted.
* Source/WebKit/WebProcess/WebPage/WebPage.cpp:
(WebKit::WebPage::adjustVisibilityForTargetedElements):
(WebKit::WebPage::resetVisibilityAdjustmentsForTargetedElements):
* Source/WebKit/WebProcess/WebPage/WebPage.h:
* Source/WebKit/WebProcess/WebPage/WebPage.messages.in:
* Tools/TestWebKitAPI/Tests/WebKitCocoa/ElementTargetingTests.mm:
(TestWebKitAPI::TEST(ElementTargeting, AdjustVisibilityAfterRecreatingElement)):
* Tools/TestWebKitAPI/Tests/WebKitCocoa/element-targeting-7.html:

Add an API test to exercise the change.

Canonical link: https://commits.webkit.org/278562@main
  • Loading branch information
whsieh committed May 9, 2024
1 parent dff86da commit df5bab1
Show file tree
Hide file tree
Showing 15 changed files with 185 additions and 73 deletions.
7 changes: 4 additions & 3 deletions Source/WebCore/loader/DocumentLoader.h
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
#include "DeviceOrientationOrMotionPermissionState.h"
#include "DocumentLoadTiming.h"
#include "DocumentWriter.h"
#include "ElementTargetingTypes.h"
#include "FrameDestructionObserver.h"
#include "LinkIcon.h"
#include "NavigationAction.h"
Expand Down Expand Up @@ -346,8 +347,8 @@ class DocumentLoader
bool allowsActiveContentRuleListActionsForURL(const String& contentRuleListIdentifier, const URL&) const;
WEBCORE_EXPORT void setActiveContentRuleListActionPatterns(const HashMap<String, Vector<String>>&);

const Vector<Vector<HashSet<String>>>& visibilityAdjustmentSelectors() const { return m_visibilityAdjustmentSelectors; }
void setVisibilityAdjustmentSelectors(Vector<Vector<HashSet<String>>>&& selectors) { m_visibilityAdjustmentSelectors = WTFMove(selectors); }
const Vector<TargetedElementSelectors>& visibilityAdjustmentSelectors() const { return m_visibilityAdjustmentSelectors; }
void setVisibilityAdjustmentSelectors(Vector<TargetedElementSelectors>&& selectors) { m_visibilityAdjustmentSelectors = WTFMove(selectors); }

#if ENABLE(DEVICE_ORIENTATION)
DeviceOrientationOrMotionPermissionState deviceOrientationAndMotionAccessState() const { return m_deviceOrientationAndMotionAccessState; }
Expand Down Expand Up @@ -706,7 +707,7 @@ class DocumentLoader
MemoryCompactRobinHoodHashMap<String, Vector<UserContentURLPattern>> m_activeContentRuleListActionPatterns;
ContentExtensionEnablement m_contentExtensionEnablement { ContentExtensionDefaultEnablement::Enabled, { } };

Vector<Vector<HashSet<String>>> m_visibilityAdjustmentSelectors;
Vector<TargetedElementSelectors> m_visibilityAdjustmentSelectors;

ScriptExecutionContextIdentifier m_resultingClientId;

Expand Down
139 changes: 95 additions & 44 deletions Source/WebCore/page/ElementTargetingController.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -67,9 +67,13 @@ namespace WebCore {
static constexpr auto maximumNumberOfClasses = 5;
static constexpr auto marginForTrackingAdjustmentRects = 5;
static constexpr auto minimumDistanceToConsiderEdgesEquidistant = 2;
static constexpr auto minimumWidthForNearbyTarget = 2;
static constexpr auto minimumHeightForNearbyTarget = 2;
static constexpr auto minimumLengthForSearchableText = 25;
static constexpr auto maximumLengthForSearchableText = 100;
static constexpr auto selectorBasedVisibilityAdjustmentTimeLimit = 30_s;
static constexpr auto selectorBasedVisibilityAdjustmentThrottlingTimeLimit = 10_s;
static constexpr auto selectorBasedVisibilityAdjustmentInterval = 1_s;
static constexpr auto maximumNumberOfAdditionalAdjustments = 20;
static constexpr auto adjustmentClientRectCleanUpDelay = 15_s;
static constexpr auto minimumAreaRatioForElementToCoverViewport = 0.95;
static constexpr auto minimumAreaForInterpolation = 200000;
Expand Down Expand Up @@ -111,6 +115,7 @@ using ElementSelectorCache = HashMap<Ref<Element>, std::optional<String>>;
ElementTargetingController::ElementTargetingController(Page& page)
: m_page { page }
, m_recentAdjustmentClientRectsCleanUpTimer { *this, &ElementTargetingController::cleanUpAdjustmentClientRects, adjustmentClientRectCleanUpDelay }
, m_selectorBasedVisibilityAdjustmentTimer { *this, &ElementTargetingController::selectorBasedVisibilityAdjustmentTimerFired }
{
}

Expand Down Expand Up @@ -969,10 +974,10 @@ Vector<TargetedElementInfo> ElementTargetingController::extractTargets(Vector<Re
return results;

auto nearbyTargets = [&] {
HashSet<Ref<Element>> targets;
HashSet<Ref<Element>> results;
CheckedPtr bodyRenderer = bodyElement->renderer();
if (!bodyRenderer)
return targets;
return results;

for (auto& renderer : descendantsOfType<RenderElement>(*bodyRenderer)) {
if (!renderer.isOutOfFlowPositioned())
Expand All @@ -982,7 +987,14 @@ Vector<TargetedElementInfo> ElementTargetingController::extractTargets(Vector<Re
if (!element)
continue;

if (targets.contains(*element))
bool elementIsAlreadyTargeted = targets.containsIf([&element](auto& target) {
return target->containsIncludingShadowDOM(element.get());
});

if (elementIsAlreadyTargeted)
continue;

if (results.contains(*element))
continue;

if (nodes.containsIf([&](auto& node) { return node.ptr() == element; }))
Expand All @@ -992,15 +1004,21 @@ Vector<TargetedElementInfo> ElementTargetingController::extractTargets(Vector<Re
continue;

auto boundingBox = element->boundingBoxInRootViewCoordinates();
if (boundingBox.width() <= minimumWidthForNearbyTarget)
continue;

if (boundingBox.height() <= minimumHeightForNearbyTarget)
continue;

if (!additionalRegionForNearbyElements.contains(boundingBox))
continue;

if (computeViewportAreaRatio(boundingBox) > nearbyTargetAreaRatio)
continue;

targets.add(element.releaseNonNull());
results.add(element.releaseNonNull());
}
return targets;
return results;
}();

for (auto& element : nearbyTargets) {
Expand Down Expand Up @@ -1048,7 +1066,7 @@ static inline VisibilityAdjustmentResult adjustVisibilityIfNeeded(Element& eleme
return { adjustedElement.ptr(), adjustment == VisibilityAdjustment::Subtree };
}

bool ElementTargetingController::adjustVisibility(const Vector<std::pair<ElementIdentifier, ScriptExecutionContextIdentifier>>& identifiers)
bool ElementTargetingController::adjustVisibility(Vector<TargetedElementAdjustment>&& adjustments)
{
RefPtr page = m_page.get();
if (!page)
Expand All @@ -1068,7 +1086,8 @@ bool ElementTargetingController::adjustVisibility(const Vector<std::pair<Element
return false;

Region newAdjustmentRegion;
for (auto [elementID, documentID] : identifiers) {
for (auto& [identifiers, selectors] : adjustments) {
auto [elementID, documentID] = identifiers;
if (auto rect = m_recentAdjustmentClientRects.get(elementID); !rect.isEmpty())
newAdjustmentRegion.unite(rect);
}
Expand All @@ -1077,10 +1096,21 @@ bool ElementTargetingController::adjustVisibility(const Vector<std::pair<Element
m_adjustmentClientRegion.unite(newAdjustmentRegion);

Vector<Ref<Element>> elements;
elements.reserveInitialCapacity(identifiers.size());
for (auto [elementID, documentID] : identifiers) {
if (RefPtr element = Element::fromIdentifier(elementID); element && element->document().identifier() == documentID)
elements.append(element.releaseNonNull());
elements.reserveInitialCapacity(adjustments.size());
for (auto& [identifiers, selectors] : adjustments) {
auto [elementID, documentID] = identifiers;
RefPtr element = Element::fromIdentifier(elementID);
if (!element)
continue;

if (element->document().identifier() != documentID)
continue;

elements.append(element.releaseNonNull());
if (m_additionalAdjustmentCount < maximumNumberOfAdditionalAdjustments) {
m_visibilityAdjustmentSelectors.append({ elementID, WTFMove(selectors) });
m_additionalAdjustmentCount++;
}
}

bool changed = false;
Expand Down Expand Up @@ -1176,7 +1206,20 @@ void ElementTargetingController::adjustVisibilityInRepeatedlyTargetedRegions(Doc
adjustRegionAfterViewportSizeChange(m_repeatedAdjustmentClientRegion, previousViewportSize, m_viewportSizeForVisibilityAdjustment);
}

applyVisibilityAdjustmentFromSelectors(document);
if (RefPtr loader = document.loader(); loader && !m_didCollectInitialAdjustments) {
m_visibilityAdjustmentSelectors.appendVector(loader->visibilityAdjustmentSelectors().map([](auto& selectors) -> std::pair<ElementIdentifier, TargetedElementSelectors> {
return { { }, selectors };
}));
m_startTimeForSelectorBasedVisibilityAdjustment = ApproximateTime::now();
m_didCollectInitialAdjustments = true;
}

if (!m_visibilityAdjustmentSelectors.isEmpty()) {
if (ApproximateTime::now() - m_startTimeForSelectorBasedVisibilityAdjustment <= selectorBasedVisibilityAdjustmentThrottlingTimeLimit)
applyVisibilityAdjustmentFromSelectors(document);
else if (!m_selectorBasedVisibilityAdjustmentTimer.isActive())
m_selectorBasedVisibilityAdjustmentTimer.startOneShot(selectorBasedVisibilityAdjustmentInterval);
}

if (m_repeatedAdjustmentClientRegion.isEmpty())
return;
Expand Down Expand Up @@ -1225,26 +1268,11 @@ void ElementTargetingController::adjustVisibilityInRepeatedlyTargetedRegions(Doc

void ElementTargetingController::applyVisibilityAdjustmentFromSelectors(Document& document)
{
RefPtr page = m_page.get();
if (!page)
if (m_visibilityAdjustmentSelectors.isEmpty())
return;

RefPtr loader = document.loader();
if (!loader)
return;

auto currentTime = ApproximateTime::now();
if (!m_remainingVisibilityAdjustmentSelectors) {
m_remainingVisibilityAdjustmentSelectors = loader->visibilityAdjustmentSelectors();
m_startTimeForSelectorBasedVisibilityAdjustment = currentTime;
}

if (currentTime - m_startTimeForSelectorBasedVisibilityAdjustment >= selectorBasedVisibilityAdjustmentTimeLimit) {
m_remainingVisibilityAdjustmentSelectors->clear();
return;
}

if (m_remainingVisibilityAdjustmentSelectors->isEmpty())
RefPtr page = m_page.get();
if (!page)
return;

auto resolveSelectorToQuery = [](const String& selectorIncludingPseudo) -> std::pair<String, VisibilityAdjustment> {
Expand All @@ -1269,11 +1297,10 @@ void ElementTargetingController::applyVisibilityAdjustmentFromSelectors(Document
auto viewportArea = m_viewportSizeForVisibilityAdjustment.area();
Region adjustmentRegion;
Vector<String> matchingSelectors;
for (auto& selectorsForElementIncludingShadowHosts : *m_remainingVisibilityAdjustmentSelectors) {
for (auto& [identifier, selectorsForElementIncludingShadowHosts] : m_visibilityAdjustmentSelectors) {
if (selectorsForElementIncludingShadowHosts.isEmpty())
continue;

bool foundLastTarget = false;
Ref<ContainerNode> containerToQuery = document;
size_t indexOfSelectorToQuery = 0;
for (auto& selectorsToQuery : selectorsForElementIncludingShadowHosts) {
Expand Down Expand Up @@ -1321,8 +1348,9 @@ void ElementTargetingController::applyVisibilityAdjustmentFromSelectors(Document

if (auto clientRect = inflatedClientRectForAdjustmentRegionTracking(*element, viewportArea))
adjustmentRegion.unite(*clientRect);

matchingSelectors.append(selectorIncludingPseudo);
}
matchingSelectors.append(selectorIncludingPseudo);
}

currentTarget = WTFMove(element);
Expand All @@ -1336,7 +1364,6 @@ void ElementTargetingController::applyVisibilityAdjustmentFromSelectors(Document

if (isLastTarget) {
// We resolved the final targeted element.
foundLastTarget = true;
break;
}

Expand All @@ -1347,18 +1374,11 @@ void ElementTargetingController::applyVisibilityAdjustmentFromSelectors(Document
// Continue the search underneath the next shadow root.
containerToQuery = nextShadowRoot.releaseNonNull();
}

if (foundLastTarget)
selectorsForElementIncludingShadowHosts.clear();
}

if (!adjustmentRegion.isEmpty())
m_adjustmentClientRegion.unite(adjustmentRegion);

m_remainingVisibilityAdjustmentSelectors->removeAllMatching([](auto& selectors) {
return selectors.isEmpty();
});

if (matchingSelectors.isEmpty())
return;

Expand All @@ -1372,13 +1392,16 @@ void ElementTargetingController::reset()
m_repeatedAdjustmentClientRegion = { };
m_viewportSizeForVisibilityAdjustment = { };
m_adjustedElements = { };
m_remainingVisibilityAdjustmentSelectors = { };
m_visibilityAdjustmentSelectors = { };
m_didCollectInitialAdjustments = false;
m_additionalAdjustmentCount = 0;
m_selectorBasedVisibilityAdjustmentTimer.stop();
m_startTimeForSelectorBasedVisibilityAdjustment = { };
m_recentAdjustmentClientRectsCleanUpTimer.stop();
cleanUpAdjustmentClientRects();
}

bool ElementTargetingController::resetVisibilityAdjustments(const Vector<std::pair<ElementIdentifier, ScriptExecutionContextIdentifier>>& identifiers)
bool ElementTargetingController::resetVisibilityAdjustments(const Vector<TargetedElementIdentifiers>& identifiers)
{
RefPtr page = m_page.get();
if (!page)
Expand Down Expand Up @@ -1419,6 +1442,17 @@ bool ElementTargetingController::resetVisibilityAdjustments(const Vector<std::pa
}
}

if (RefPtr loader = document->loader(); loader && !identifiers.isEmpty()) {
m_visibilityAdjustmentSelectors = loader->visibilityAdjustmentSelectors().map([](auto& selectors) -> std::pair<ElementIdentifier, TargetedElementSelectors> {
return { { }, selectors };
});
} else {
// There are no initial adjustments after resetting.
m_visibilityAdjustmentSelectors = { };
}
m_additionalAdjustmentCount = 0;
m_didCollectInitialAdjustments = true;

if (elementsToReset.isEmpty())
return false;

Expand Down Expand Up @@ -1523,4 +1557,21 @@ void ElementTargetingController::dispatchVisibilityAdjustmentStateDidChange()
});
}

void ElementTargetingController::selectorBasedVisibilityAdjustmentTimerFired()
{
RefPtr page = m_page.get();
if (!page)
return;

RefPtr mainFrame = dynamicDowncast<LocalFrame>(page->mainFrame());
if (!mainFrame)
return;

RefPtr document = mainFrame->document();
if (!document)
return;

applyVisibilityAdjustmentFromSelectors(*document);
}

} // namespace WebCore
11 changes: 8 additions & 3 deletions Source/WebCore/page/ElementTargetingController.h
Original file line number Diff line number Diff line change
Expand Up @@ -54,19 +54,21 @@ class ElementTargetingController final : public CanMakeCheckedPtr<ElementTargeti

WEBCORE_EXPORT Vector<TargetedElementInfo> findTargets(TargetedElementRequest&&);

WEBCORE_EXPORT bool adjustVisibility(const Vector<std::pair<ElementIdentifier, ScriptExecutionContextIdentifier>>&);
WEBCORE_EXPORT bool adjustVisibility(Vector<TargetedElementAdjustment>&&);
void adjustVisibilityInRepeatedlyTargetedRegions(Document&);

void reset();

WEBCORE_EXPORT uint64_t numberOfVisibilityAdjustmentRects() const;
WEBCORE_EXPORT bool resetVisibilityAdjustments(const Vector<std::pair<ElementIdentifier, ScriptExecutionContextIdentifier>>&);
WEBCORE_EXPORT bool resetVisibilityAdjustments(const Vector<TargetedElementIdentifiers>&);

private:
void cleanUpAdjustmentClientRects();

void applyVisibilityAdjustmentFromSelectors(Document&);

void dispatchVisibilityAdjustmentStateDidChange();
void selectorBasedVisibilityAdjustmentTimerFired();

std::pair<Vector<Ref<Node>>, RefPtr<Element>> findNodes(FloatPoint location, bool shouldIgnorePointerEventsNone);
std::pair<Vector<Ref<Node>>, RefPtr<Element>> findNodes(const String& searchText);
Expand All @@ -77,11 +79,14 @@ class ElementTargetingController final : public CanMakeCheckedPtr<ElementTargeti
DeferrableOneShotTimer m_recentAdjustmentClientRectsCleanUpTimer;
HashMap<ElementIdentifier, IntRect> m_recentAdjustmentClientRects;
ApproximateTime m_startTimeForSelectorBasedVisibilityAdjustment;
std::optional<Vector<Vector<HashSet<String>>>> m_remainingVisibilityAdjustmentSelectors;
Timer m_selectorBasedVisibilityAdjustmentTimer;
Vector<std::pair<ElementIdentifier, TargetedElementSelectors>> m_visibilityAdjustmentSelectors;
Region m_adjustmentClientRegion;
Region m_repeatedAdjustmentClientRegion;
WeakHashSet<Element, WeakPtrImplWithEventTargetData> m_adjustedElements;
FloatSize m_viewportSizeForVisibilityAdjustment;
unsigned m_additionalAdjustmentCount { 0 };
bool m_didCollectInitialAdjustments { false };
};

} // namespace WebCore
8 changes: 8 additions & 0 deletions Source/WebCore/page/ElementTargetingTypes.h
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,14 @@

namespace WebCore {

using TargetedElementSelectors = Vector<HashSet<String>>;
using TargetedElementIdentifiers = std::pair<ElementIdentifier, ScriptExecutionContextIdentifier>;

struct TargetedElementAdjustment {
TargetedElementIdentifiers identifiers;
TargetedElementSelectors selectors;
};

struct TargetedElementRequest {
std::variant<FloatPoint, String> data;
bool canIncludeNearbyElements { true };
Expand Down
1 change: 1 addition & 0 deletions Source/WebKit/Scripts/webkit/messages.py
Original file line number Diff line number Diff line change
Expand Up @@ -969,6 +969,7 @@ def headers_for_type(type):
'WebCore::SupportedPluginIdentifier': ['<WebCore/PluginData.h>'],
'WebCore::SWServerConnectionIdentifier': ['<WebCore/ServiceWorkerTypes.h>'],
'WebCore::SystemPreviewInfo': ['<WebCore/FrameLoaderTypes.h>'],
'WebCore::TargetedElementAdjustment': ['<WebCore/ElementTargetingTypes.h>'],
'WebCore::TargetedElementInfo': ['<WebCore/ElementTargetingTypes.h>'],
'WebCore::TargetedElementRequest': ['<WebCore/ElementTargetingTypes.h>'],
'WebCore::TextCheckingRequestData': ['<WebCore/TextChecking.h>'],
Expand Down
6 changes: 6 additions & 0 deletions Source/WebKit/Shared/WebCoreArgumentCoders.serialization.in
Original file line number Diff line number Diff line change
Expand Up @@ -884,6 +884,12 @@ struct WebCore::ShareData {

enum class WebCore::ShareDataOriginator : bool

header: <WebCore/ElementTargetingTypes.h>
[CustomHeader] struct WebCore::TargetedElementAdjustment {
std::pair<WebCore::ElementIdentifier, WebCore::ScriptExecutionContextIdentifier> identifiers
Vector<HashSet<String>> selectors
};

header: <WebCore/ElementTargetingTypes.h>
[CustomHeader] struct WebCore::TargetedElementRequest {
std::variant<WebCore::FloatPoint, String> data
Expand Down
2 changes: 1 addition & 1 deletion Source/WebKit/Shared/WebsitePoliciesData.h
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ struct WebsitePoliciesData {

HashMap<String, Vector<String>> activeContentRuleListActionPatterns;
Vector<WebCore::CustomHeaderFields> customHeaderFields;
Vector<Vector<HashSet<String>>> visibilityAdjustmentSelectors;
Vector<WebCore::TargetedElementSelectors> visibilityAdjustmentSelectors;
String customUserAgent;
String customUserAgentAsSiteSpecificQuirks;
String customNavigatorPlatform;
Expand Down
Loading

0 comments on commit df5bab1

Please sign in to comment.