Modified: trunk/Source/WebCore/ChangeLog (226193 => 226194)
--- trunk/Source/WebCore/ChangeLog 2017-12-20 19:29:08 UTC (rev 226193)
+++ trunk/Source/WebCore/ChangeLog 2017-12-20 19:59:35 UTC (rev 226194)
@@ -1,3 +1,22 @@
+2017-12-20 Daniel Bates <daba...@apple.com>
+
+ MarkerSubrange.SubdivideGrammarAndSelectionOverlap{Frontmost, FrontmostWithLongestEffectiveRange} are failing
+ https://bugs.webkit.org/show_bug.cgi?id=181014
+
+ Reviewed by Simon Fraser.
+
+ Fixes an issue in the subdivision algorithm where the returned subranges may not be paint order
+ or reverse paint order when using the default overlap strategy (OverlapStrategy::None) and
+ either OverlapStrategy::Frontmost or OverlapStrategy::FrontmostWithLongestEffectiveRange, respectively.
+
+ Currently we compute the overlapping subranges up to some point p_i on the line by sweeping from the
+ start of the line through all the unclosed subranges. The unclosed subranges are sorted along the line.
+ That is, they are not sorted by paint order or reverse paint order. Therefore we must take care to
+ ensure that we return the computed overlapping subranges with respect to paint order/reverse paint order.
+
+ * rendering/MarkerSubrange.cpp:
+ (WebCore::subdivide):
+
2017-12-20 Youenn Fablet <you...@apple.com>
LayoutTest imported/w3c/web-platform-tests/service-workers/cache-storage/serviceworker/cache-match.https.html is a flaky failure
Modified: trunk/Source/WebCore/rendering/MarkerSubrange.cpp (226193 => 226194)
--- trunk/Source/WebCore/rendering/MarkerSubrange.cpp 2017-12-20 19:29:08 UTC (rev 226193)
+++ trunk/Source/WebCore/rendering/MarkerSubrange.cpp 2017-12-20 19:59:35 UTC (rev 226194)
@@ -70,7 +70,7 @@
if (overlapStrategy == OverlapStrategy::Frontmost || overlapStrategy == OverlapStrategy::FrontmostWithLongestEffectiveRange) {
std::optional<unsigned> frontmost;
for (unsigned j = 0; j < i; ++j) {
- if (!processedSubranges.contains(offsets[j].subrange))
+ if (!processedSubranges.contains(offsets[j].subrange) && (!frontmost || offsets[j].subrange->type > offsets[*frontmost].subrange->type))
frontmost = j;
}
if (frontmost) {
@@ -84,6 +84,7 @@
result.append({ offsetSoFar, offsets[i].value, offsets[frontmost.value()].subrange->type, offsets[frontmost.value()].subrange->marker });
}
} else {
+ // The appended subranges may not be in paint order. We will fix this up at the end of this function.
for (unsigned j = 0; j < i; ++j) {
if (!processedSubranges.contains(offsets[j].subrange))
result.append({ offsetSoFar, offsets[i].value, offsets[j].subrange->type, offsets[j].subrange->marker });
@@ -94,6 +95,9 @@
if (offsets[i].kind == Offset::End)
processedSubranges.add(offsets[i].subrange);
}
+ // Fix up; sort the subranges so that they are in paint order.
+ if (overlapStrategy == OverlapStrategy::None)
+ std::sort(result.begin(), result.end(), [] (const MarkerSubrange& a, const MarkerSubrange& b) { return a.startOffset < b.startOffset || (a.startOffset == b.startOffset && a.type < b.type); });
return result;
}
Modified: trunk/Tools/TestWebKitAPI/Tests/WebCore/MarkerSubrange.cpp (226193 => 226194)
--- trunk/Tools/TestWebKitAPI/Tests/WebCore/MarkerSubrange.cpp 2017-12-20 19:29:08 UTC (rev 226193)
+++ trunk/Tools/TestWebKitAPI/Tests/WebCore/MarkerSubrange.cpp 2017-12-20 19:59:35 UTC (rev 226194)
@@ -174,7 +174,7 @@
EXPECT_EQ(expectedSubranges[i], results[i]);
}
-TEST(MarkerSubrange, DISABLED_SubdivideGrammarAndSelectionOverlapFrontmost)
+TEST(MarkerSubrange, SubdivideGrammarAndSelectionOverlap)
{
Vector<MarkerSubrange> subranges {
MarkerSubrange { 0, 40, MarkerSubrange::GrammarError },
@@ -183,10 +183,31 @@
};
Vector<MarkerSubrange> expectedSubranges {
MarkerSubrange { 0, 2, MarkerSubrange::GrammarError },
+ MarkerSubrange { 2, 40, MarkerSubrange::GrammarError },
MarkerSubrange { 2, 40, MarkerSubrange::Selection },
MarkerSubrange { 40, 50, MarkerSubrange::Selection },
+ MarkerSubrange { 50, 60, MarkerSubrange::GrammarError },
MarkerSubrange { 50, 60, MarkerSubrange::Selection },
};
+ auto results = subdivide(subranges);
+ ASSERT_EQ(expectedSubranges.size(), results.size());
+ for (size_t i = 0; i < expectedSubranges.size(); ++i)
+ EXPECT_EQ(expectedSubranges[i], results[i]);
+}
+
+TEST(MarkerSubrange, SubdivideGrammarAndSelectionOverlapFrontmost)
+{
+ Vector<MarkerSubrange> subranges {
+ MarkerSubrange { 0, 40, MarkerSubrange::GrammarError },
+ MarkerSubrange { 2, 60, MarkerSubrange::Selection },
+ MarkerSubrange { 50, 60, MarkerSubrange::GrammarError },
+ };
+ Vector<MarkerSubrange> expectedSubranges {
+ MarkerSubrange { 0, 2, MarkerSubrange::GrammarError },
+ MarkerSubrange { 2, 40, MarkerSubrange::Selection },
+ MarkerSubrange { 40, 50, MarkerSubrange::Selection },
+ MarkerSubrange { 50, 60, MarkerSubrange::Selection },
+ };
auto results = subdivide(subranges, OverlapStrategy::Frontmost);
ASSERT_EQ(expectedSubranges.size(), results.size());
for (size_t i = 0; i < expectedSubranges.size(); ++i)
@@ -193,7 +214,7 @@
EXPECT_EQ(expectedSubranges[i], results[i]);
}
-TEST(MarkerSubrange, DISABLED_SubdivideGrammarAndSelectionOverlapFrontmostWithLongestEffectiveRange)
+TEST(MarkerSubrange, SubdivideGrammarAndSelectionOverlapFrontmostWithLongestEffectiveRange)
{
Vector<MarkerSubrange> subranges {
MarkerSubrange { 0, 40, MarkerSubrange::GrammarError },