Modified: branches/safari-612.1.13-branch/Source/WebCore/ChangeLog (277048 => 277049)
--- branches/safari-612.1.13-branch/Source/WebCore/ChangeLog 2021-05-05 23:01:42 UTC (rev 277048)
+++ branches/safari-612.1.13-branch/Source/WebCore/ChangeLog 2021-05-05 23:01:46 UTC (rev 277049)
@@ -1,5 +1,77 @@
2021-05-05 Ruben Turcios <[email protected]>
+ Cherry-pick r277044. rdar://problem/77579966
+
+ Sampled Page Top Color: don't snapshot if the hit test location is an image or has an animation
+ https://bugs.webkit.org/show_bug.cgi?id=225338
+
+ Reviewed by Tim Horton.
+
+ Source/WebCore:
+
+ Tests: SampledPageTopColor.HitTestHTMLImage
+ SampledPageTopColor.HitTestCSSBackgroundImage
+ SampledPageTopColor.HitTestCSSAnimation
+
+ * dom/Document.h:
+ * dom/Document.cpp:
+ (WebCore::isValidPageSampleLocation): Added.
+ (WebCore::samplePageColor): Added.
+ (WebCore::Document::determineSampledPageTopColor):
+ (WebCore::Document::isHitTestLocationThirdPartyFrame): Deleted.
+ Refactor `isHitTestLocationThirdPartyFrame` (and the `pixelColor` lambda) into `static`
+ functions that are right above `Document::determineSampledPageTopColor` for clarity and to
+ allow for more flexibility. In order to check if the hit test node is an image (including
+ having a CSS `background-image`) or has a CSS animation (or CSS transition), it's necessary
+ to continue to hit test beyond one node as the image and/or node with the CSS animation may
+ be an ancestor (or unrelated `position: absolute` node) to the `HitTestResult::innerNode`.
+ ```
+ <div style="background-image: url(...)">
+ <button>Lorum ipsum ... </button>
+ </div>
+ ```
+
+ Tools:
+
+ * TestWebKitAPI/Tests/WebKitCocoa/SampledPageTopColor.mm:
+ (TEST.SampledPageTopColor.HitTestHTMLImage):
+ (TEST.SampledPageTopColor.HitTestCSSBackgroundImage):
+ (TEST.SampledPageTopColor.HitTestCSSAnimation):
+
+
+ git-svn-id: https://svn.webkit.org/repository/webkit/trunk@277044 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+ 2021-05-05 Devin Rousso <[email protected]>
+
+ Sampled Page Top Color: don't snapshot if the hit test location is an image or has an animation
+ https://bugs.webkit.org/show_bug.cgi?id=225338
+
+ Reviewed by Tim Horton.
+
+ Tests: SampledPageTopColor.HitTestHTMLImage
+ SampledPageTopColor.HitTestCSSBackgroundImage
+ SampledPageTopColor.HitTestCSSAnimation
+
+ * dom/Document.h:
+ * dom/Document.cpp:
+ (WebCore::isValidPageSampleLocation): Added.
+ (WebCore::samplePageColor): Added.
+ (WebCore::Document::determineSampledPageTopColor):
+ (WebCore::Document::isHitTestLocationThirdPartyFrame): Deleted.
+ Refactor `isHitTestLocationThirdPartyFrame` (and the `pixelColor` lambda) into `static`
+ functions that are right above `Document::determineSampledPageTopColor` for clarity and to
+ allow for more flexibility. In order to check if the hit test node is an image (including
+ having a CSS `background-image`) or has a CSS animation (or CSS transition), it's necessary
+ to continue to hit test beyond one node as the image and/or node with the CSS animation may
+ be an ancestor (or unrelated `position: absolute` node) to the `HitTestResult::innerNode`.
+ ```
+ <div style="background-image: url(...)">
+ <button>Lorum ipsum ... </button>
+ </div>
+ ```
+
+2021-05-05 Ruben Turcios <[email protected]>
+
Cherry-pick r277030. rdar://problem/77573075
Sampled Page Top Color: take additional snapshots further down the page to see if the sampled top color is more than just a tiny strip
Modified: branches/safari-612.1.13-branch/Source/WebCore/dom/Document.cpp (277048 => 277049)
--- branches/safari-612.1.13-branch/Source/WebCore/dom/Document.cpp 2021-05-05 23:01:42 UTC (rev 277048)
+++ branches/safari-612.1.13-branch/Source/WebCore/dom/Document.cpp 2021-05-05 23:01:46 UTC (rev 277049)
@@ -177,9 +177,11 @@
#include "Range.h"
#include "RealtimeMediaSourceCenter.h"
#include "RenderChildIterator.h"
+#include "RenderImage.h"
#include "RenderInline.h"
#include "RenderLayerCompositor.h"
#include "RenderLineBreak.h"
+#include "RenderStyle.h"
#include "RenderTreeUpdater.h"
#include "RenderView.h"
#include "RenderWidget.h"
@@ -3870,6 +3872,53 @@
#endif // ENABLE(RUBBER_BANDING)
}
+static bool isValidPageSampleLocation(Document& document, const IntPoint& location)
+{
+ // FIXME: <https://webkit.org/b/225167> (Sampled Page Top Color: hook into painting logic instead of taking snapshots)
+
+ constexpr OptionSet<HitTestRequest::RequestType> hitTestRequestTypes { HitTestRequest::ReadOnly, HitTestRequest::DisallowUserAgentShadowContent, HitTestRequest::CollectMultipleElements, HitTestRequest::IncludeAllElementsUnderPoint };
+ HitTestResult hitTestResult(location);
+ document.hitTest(hitTestRequestTypes, hitTestResult);
+
+ for (auto& hitTestNode : hitTestResult.listBasedTestResult()) {
+ auto& node = hitTestNode.get();
+
+ auto* renderer = node.renderer();
+ if (!renderer)
+ return false;
+
+ // Skip images (both `<img>` and CSS `background-image`) as they're likely not a solid color.
+ if (is<RenderImage>(renderer) || renderer->style().hasBackgroundImage())
+ return false;
+
+ // Skip nodes with animations as the sample may get an odd color if the animation is in-progress.
+ if (renderer->style().hasTransitions() || renderer->style().hasAnimations())
+ return false;
+
+ // Skip 3rd-party `<iframe>` as the content likely won't match the rest of the page.
+ if (is<HTMLIFrameElement>(node) && !areRegistrableDomainsEqual(downcast<HTMLIFrameElement>(node).location(), document.url()))
+ return false;
+ }
+
+ return true;
+}
+
+static Optional<Lab<float>> samplePageColor(Document& document, IntPoint&& location)
+{
+ // FIXME: <https://webkit.org/b/225167> (Sampled Page Top Color: hook into painting logic instead of taking snapshots)
+
+ if (!isValidPageSampleLocation(document, location))
+ return WTF::nullopt;
+
+ ASSERT(document.view());
+ auto snapshot = snapshotFrameRect(document.view()->frame(), IntRect(location, IntSize(1, 1)), SnapshotOptionsExcludeSelectionHighlighting | SnapshotOptionsPaintEverythingExcludingSelection);
+ if (!snapshot)
+ return WTF::nullopt;
+
+ auto snapshotData = snapshot->toBGRAData();
+ return convertColor<Lab<float>>(SRGBA<uint8_t> { snapshotData[2], snapshotData[1], snapshotData[0], snapshotData[3] });
+}
+
static double colorDifference(Lab<float>& lhs, Lab<float>& rhs)
{
return sqrt(pow(rhs.lightness - lhs.lightness, 2) + pow(rhs.a - lhs.a, 2) + pow(rhs.b - lhs.b, 2));
@@ -3914,21 +3963,6 @@
page->chrome().client().didSamplePageTopColor();
});
- // FIXME: <https://webkit.org/b/225167> (Sampled Page Top Color: hook into painting logic instead of taking snapshots)
- auto pixelColor = [&] (IntPoint&& location) -> Optional<Lab<float>> {
- IntSize size(1, 1);
-
- if (isHitTestLocationThirdPartyFrame(LayoutPoint(location)))
- return WTF::nullopt;
-
- auto snapshot = snapshotFrameRect(frameView->frame(), IntRect(location, size), SnapshotOptionsExcludeSelectionHighlighting | SnapshotOptionsPaintEverythingExcludingSelection);
- if (!snapshot)
- return WTF::nullopt;
-
- auto snapshotData = snapshot->toBGRAData();
- return convertColor<Lab<float>>(SRGBA<uint8_t> { snapshotData[2], snapshotData[1], snapshotData[0], snapshotData[3] });
- };
-
// Decrease the width by one pixel so that the last snapshot is within bounds and not off-by-one.
auto frameWidth = frameView->contentsWidth() - 1;
@@ -3948,7 +3982,7 @@
};
for (size_t i = 0; i < numSnapshots; ++i) {
- auto snapshot = pixelColor(IntPoint(frameWidth * i / (numSnapshots - 1), 0));
+ auto snapshot = samplePageColor(*this, IntPoint(frameWidth * i / (numSnapshots - 1), 0));
if (!snapshot) {
if (shouldStopAfterFindingNonMatchingColor(i))
return;
@@ -3996,7 +4030,7 @@
auto minHeight = settings().sampledPageTopColorMinHeight() - 1;
if (minHeight > 0) {
if (nonMatchingColorIndex) {
- if (auto leftMiddleSnapshot = pixelColor(IntPoint(0, minHeight))) {
+ if (auto leftMiddleSnapshot = samplePageColor(*this, IntPoint(0, minHeight))) {
if (colorDifference(*leftMiddleSnapshot, snapshots[0]) > maxDifference)
return;
}
@@ -4003,7 +4037,7 @@
}
if (nonMatchingColorIndex != numSnapshots - 1) {
- if (auto rightMiddleSnapshot = pixelColor(IntPoint(frameWidth, minHeight))) {
+ if (auto rightMiddleSnapshot = samplePageColor(*this, IntPoint(frameWidth, minHeight))) {
if (colorDifference(*rightMiddleSnapshot, snapshots[numSnapshots - 1]) > maxDifference)
return;
}
@@ -8753,22 +8787,6 @@
return resultLayer;
}
-bool Document::isHitTestLocationThirdPartyFrame(const HitTestLocation& location)
-{
- constexpr OptionSet<HitTestRequest::RequestType> hitTestRequestTypes { HitTestRequest::ReadOnly, HitTestRequest::DisallowUserAgentShadowContent };
- HitTestResult hitTestResult(location);
- hitTest(hitTestRequestTypes, hitTestResult);
-
- auto hitTestNode = makeRefPtr(hitTestResult.innerNode());
- if (!hitTestNode)
- return false;
-
- if (!is<HTMLIFrameElement>(hitTestNode))
- return false;
-
- return areRegistrableDomainsEqual(downcast<HTMLIFrameElement>(*hitTestNode).location(), m_url);
-}
-
ElementIdentifier Document::identifierForElement(Element& element)
{
ASSERT(&element.document() == this);
Modified: branches/safari-612.1.13-branch/Source/WebCore/dom/Document.h (277048 => 277049)
--- branches/safari-612.1.13-branch/Source/WebCore/dom/Document.h 2021-05-05 23:01:42 UTC (rev 277048)
+++ branches/safari-612.1.13-branch/Source/WebCore/dom/Document.h 2021-05-05 23:01:46 UTC (rev 277049)
@@ -1567,7 +1567,6 @@
WEBCORE_EXPORT bool hitTest(const HitTestRequest&, HitTestResult&);
bool hitTest(const HitTestRequest&, const HitTestLocation&, HitTestResult&);
- bool isHitTestLocationThirdPartyFrame(const HitTestLocation&);
#if ASSERT_ENABLED
bool inHitTesting() const { return m_inHitTesting; }
#endif
Modified: branches/safari-612.1.13-branch/Tools/TestWebKitAPI/Tests/WebKitCocoa/SampledPageTopColor.mm (277048 => 277049)
--- branches/safari-612.1.13-branch/Tools/TestWebKitAPI/Tests/WebKitCocoa/SampledPageTopColor.mm 2021-05-05 23:01:42 UTC (rev 277048)
+++ branches/safari-612.1.13-branch/Tools/TestWebKitAPI/Tests/WebKitCocoa/SampledPageTopColor.mm 2021-05-05 23:01:46 UTC (rev 277049)
@@ -284,6 +284,33 @@
EXPECT_NULL([webView _sampledPageTopColor]);
}
+TEST(SampledPageTopColor, HitTestHTMLImage)
+{
+ auto webView = createWebViewWithSampledPageTopColorMaxDifference(5);
+ EXPECT_NULL([webView _sampledPageTopColor]);
+
+ [webView synchronouslyLoadHTMLStringAndWaitUntilAllImmediateChildFramesPaint:@"<body style='margin: 0'><img src='' style='width: 100%; height: 100%'>Test"];
+ EXPECT_NULL([webView _sampledPageTopColor]);
+}
+
+TEST(SampledPageTopColor, HitTestCSSBackgroundImage)
+{
+ auto webView = createWebViewWithSampledPageTopColorMaxDifference(5);
+ EXPECT_NULL([webView _sampledPageTopColor]);
+
+ [webView synchronouslyLoadHTMLStringAndWaitUntilAllImmediateChildFramesPaint:@"<body style='margin: 0;'><div style='width: 100%; height: 100%; background-image: url(\'enormous.svg\')'>Test"];
+ EXPECT_NULL([webView _sampledPageTopColor]);
+}
+
+TEST(SampledPageTopColor, HitTestCSSAnimation)
+{
+ auto webView = createWebViewWithSampledPageTopColorMaxDifference(5);
+ EXPECT_NULL([webView _sampledPageTopColor]);
+
+ [webView synchronouslyLoadHTMLStringAndWaitUntilAllImmediateChildFramesPaint:@"<style>@keyframes fadeIn { from { opacity: 0; } }</style><body style='animation: fadeIn 1s infinite alternate'>Test"];
+ EXPECT_NULL([webView _sampledPageTopColor]);
+}
+
// FIXME: <https://webkit.org/b/225167> (Sampled Page Top Color: hook into painting logic instead of taking snapshots)
TEST(SampledPageTopColor, DISABLED_DisplayP3)
{
