Attachment 347002 Details for Bug 188501 – patch

[patch] patch

optionset-contains-any-all.patch (text/plain), 9.36 KB, created by Antti Koivisto on 2018-08-13 02:20:48 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Antti Koivisto

Created: 2018-08-13 02:20:48 PDT

Size: 9.36 KB

patch

obsolete

>Index: Source/WTF/ChangeLog
>===================================================================
>--- Source/WTF/ChangeLog	(revision 234792)
>+++ Source/WTF/ChangeLog	(working copy)
>@@ -1,3 +1,22 @@
>+2018-08-13  Antti Koivisto  <antti@apple.com>
>+
>+        Meaning of OptionSet::contains is unclear when used with OptionSet argument
>+        https://bugs.webkit.org/show_bug.cgi?id=188501
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        The existing behavior is "contains any" but it is not very clear from the name.
>+
>+        * wtf/OptionSet.h:
>+        (WTF::OptionSet::contains const):
>+
>+        This is now for testing a single option only.
>+
>+        (WTF::OptionSet::containsAny const):
>+        (WTF::OptionSet::containsAll const):
>+
>+        Add separate functions for OptionSet argument.
>+
> 2018-08-10  Ryosuke Niwa  <rniwa@webkit.org>
> 
>         [macOS] Multiple third party apps crash due to the thread safety check in TimerBase::setNextFireTime
>Index: Source/WTF/wtf/OptionSet.h
>===================================================================
>--- Source/WTF/wtf/OptionSet.h	(revision 234792)
>+++ Source/WTF/wtf/OptionSet.h	(working copy)
>@@ -96,9 +96,19 @@ public:
> 
>     constexpr explicit operator bool() { return !isEmpty(); }
> 
>-    constexpr bool contains(OptionSet optionSet) const
>+    constexpr bool contains(T option) const
>     {
>-        return m_storage & optionSet.m_storage;
>+        return containsAny({ option });
>+    }
>+
>+    constexpr bool containsAny(OptionSet optionSet) const
>+    {
>+        return !!(*this & optionSet);
>+    }
>+
>+    constexpr bool containsAll(OptionSet optionSet) const
>+    {
>+        return (*this & optionSet) == optionSet;
>     }
> 
>     constexpr friend bool operator==(OptionSet lhs, OptionSet rhs)
>Index: Source/WebCore/ChangeLog
>===================================================================
>--- Source/WebCore/ChangeLog	(revision 234794)
>+++ Source/WebCore/ChangeLog	(working copy)
>@@ -1,3 +1,23 @@
>+2018-08-13  Antti Koivisto  <antti@apple.com>
>+
>+        Meaning of OptionSet::contains is unclear when used with OptionSet argument
>+        https://bugs.webkit.org/show_bug.cgi?id=188501
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        No new tests (OOPS!).
>+
>+        * dom/DocumentMarkerController.cpp:
>+        (WebCore::DocumentMarkerController::possiblyHasMarkers):
>+        * dom/DocumentMarkerController.h:
>+        (WebCore::DocumentMarkerController::hasMarkers const):
>+        * platform/FileSystem.h:
>+        (WebCore::FileSystem::openAndLockFile):
>+        * rendering/RenderElement.cpp:
>+        (WebCore::RenderElement::selectionColor const):
>+        * rendering/RenderLayer.cpp:
>+        (WebCore::RenderLayer::paintForegroundForFragments):
>+
> 2018-08-12  Sihui Liu  <sihui_liu@apple.com>
> 
>         CrashTracer: com.apple.WebKit.Storage at WebCore::IDBServer::UniqueIDBDatabase::connectionClosedFromClient(WebCore::IDBServer::UniqueIDBDatabaseConnection&)
>Index: Source/WebCore/dom/DocumentMarkerController.cpp
>===================================================================
>--- Source/WebCore/dom/DocumentMarkerController.cpp	(revision 234792)
>+++ Source/WebCore/dom/DocumentMarkerController.cpp	(working copy)
>@@ -44,7 +44,7 @@ namespace WebCore {
> 
> inline bool DocumentMarkerController::possiblyHasMarkers(OptionSet<DocumentMarker::MarkerType> types)
> {
>-    return m_possiblyExistingMarkerTypes.contains(types);
>+    return m_possiblyExistingMarkerTypes.containsAny(types);
> }
> 
> DocumentMarkerController::DocumentMarkerController(Document& document)
>Index: Source/WebCore/dom/DocumentMarkerController.h
>===================================================================
>--- Source/WebCore/dom/DocumentMarkerController.h	(revision 234792)
>+++ Source/WebCore/dom/DocumentMarkerController.h	(working copy)
>@@ -63,7 +63,7 @@ public:
>     void copyMarkers(Node* srcNode, unsigned startOffset, int length, Node* dstNode, int delta);
>     bool hasMarkers() const
>     {
>-        ASSERT(m_markers.isEmpty() == !m_possiblyExistingMarkerTypes.contains(DocumentMarker::allMarkers()));
>+        ASSERT(m_markers.isEmpty() == !m_possiblyExistingMarkerTypes.containsAny(DocumentMarker::allMarkers()));
>         return !m_markers.isEmpty();
>     }
>     bool hasMarkers(Range&, OptionSet<DocumentMarker::MarkerType> = DocumentMarker::allMarkers());
>Index: Source/WebCore/rendering/RenderElement.cpp
>===================================================================
>--- Source/WebCore/rendering/RenderElement.cpp	(revision 234792)
>+++ Source/WebCore/rendering/RenderElement.cpp	(working copy)
>@@ -1361,7 +1361,7 @@ Color RenderElement::selectionColor(CSSP
>     // If the element is unselectable, or we are only painting the selection,
>     // don't override the foreground color with the selection foreground color.
>     if (style().userSelect() == UserSelect::None
>-        || (view().frameView().paintBehavior().contains(OptionSet<PaintBehavior>(PaintBehavior::SelectionOnly) | PaintBehavior::SelectionAndBackgroundsOnly)))
>+        || (view().frameView().paintBehavior().containsAny({ PaintBehavior::SelectionOnly, PaintBehavior::SelectionAndBackgroundsOnly })))
>         return Color();
> 
>     if (std::unique_ptr<RenderStyle> pseudoStyle = selectionPseudoStyle()) {
>Index: Source/WebCore/rendering/RenderLayer.cpp
>===================================================================
>--- Source/WebCore/rendering/RenderLayer.cpp	(revision 234792)
>+++ Source/WebCore/rendering/RenderLayer.cpp	(working copy)
>@@ -4788,7 +4788,7 @@ void RenderLayer::paintForegroundForFrag
>     
>     // We have to loop through every fragment multiple times, since we have to repaint in each specific phase in order for
>     // interleaving of the fragments to work properly.
>-    bool selectionOnly = localPaintingInfo.paintBehavior.contains(OptionSet<PaintBehavior>(PaintBehavior::SelectionAndBackgroundsOnly) | PaintBehavior::SelectionOnly);
>+    bool selectionOnly = localPaintingInfo.paintBehavior.containsAny({ PaintBehavior::SelectionAndBackgroundsOnly, PaintBehavior::SelectionOnly });
>     paintForegroundForFragmentsWithPhase(selectionOnly ? PaintPhase::Selection : PaintPhase::ChildBlockBackgrounds, layerFragments,
>         context, localPaintingInfo, localPaintBehavior, subtreePaintRootForRenderer);
>     
>Index: Source/WebKit/WebProcess/Plugins/PluginView.cpp
>===================================================================
>--- Source/WebKit/WebProcess/Plugins/PluginView.cpp	(revision 234792)
>+++ Source/WebKit/WebProcess/Plugins/PluginView.cpp	(working copy)
>@@ -1847,7 +1847,7 @@ bool PluginView::shouldCreateTransientPa
>         return false;
> 
>     if (FrameView* frameView = frame()->view()) {
>-        if (frameView->paintBehavior().contains(OptionSet<PaintBehavior>(PaintBehavior::SelectionOnly) | PaintBehavior::SelectionAndBackgroundsOnly | PaintBehavior::ForceBlackText)) {
>+        if (frameView->paintBehavior().containsAny({ PaintBehavior::SelectionOnly, PaintBehavior::SelectionAndBackgroundsOnly, PaintBehavior::ForceBlackText})) {
>             // This paint behavior is used when drawing the find indicator and there's no need to
>             // snapshot plug-ins, because they can never be painted as part of the find indicator.
>             return false;
>Index: Tools/ChangeLog
>===================================================================
>--- Tools/ChangeLog	(revision 234794)
>+++ Tools/ChangeLog	(working copy)
>@@ -1,3 +1,13 @@
>+2018-08-13  Antti Koivisto  <antti@apple.com>
>+
>+        Meaning of OptionSet::contains is unclear when used with OptionSet argument
>+        https://bugs.webkit.org/show_bug.cgi?id=188501
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * TestWebKitAPI/Tests/WTF/OptionSet.cpp:
>+        (TestWebKitAPI::TEST):
>+
> 2018-08-12  Zalan Bujtas  <zalan@apple.com>
> 
>         [LFC] Float prev/next sibling should prevent top/bottom margin collapsing with parent.
>Index: Tools/TestWebKitAPI/Tests/WTF/OptionSet.cpp
>===================================================================
>--- Tools/TestWebKitAPI/Tests/WTF/OptionSet.cpp	(revision 234792)
>+++ Tools/TestWebKitAPI/Tests/WTF/OptionSet.cpp	(working copy)
>@@ -379,4 +379,32 @@ TEST(WTF_OptionSet, OperatorAnd)
>     }
> }
> 
>+TEST(WTF_OptionSet, ContainsAny)
>+{
>+    OptionSet<ExampleFlags> set { ExampleFlags::A, ExampleFlags::B };
>+
>+    EXPECT_TRUE(set.containsAny({ ExampleFlags::A }));
>+    EXPECT_TRUE(set.containsAny({ ExampleFlags::B }));
>+    EXPECT_FALSE(set.containsAny({ ExampleFlags::C }));
>+    EXPECT_FALSE(set.containsAny({ ExampleFlags::C, ExampleFlags::D }));
>+    EXPECT_TRUE(set.containsAny({ ExampleFlags::A, ExampleFlags::B }));
>+    EXPECT_TRUE(set.containsAny({ ExampleFlags::B, ExampleFlags::C }));
>+    EXPECT_TRUE(set.containsAny({ ExampleFlags::A, ExampleFlags::C }));
>+    EXPECT_TRUE(set.containsAny({ ExampleFlags::A, ExampleFlags::B, ExampleFlags::C }));
>+}
>+
>+TEST(WTF_OptionSet, ContainsAll)
>+{
>+    OptionSet<ExampleFlags> set { ExampleFlags::A, ExampleFlags::B };
>+
>+    EXPECT_TRUE(set.containsAll({ ExampleFlags::A }));
>+    EXPECT_TRUE(set.containsAll({ ExampleFlags::B }));
>+    EXPECT_FALSE(set.containsAll({ ExampleFlags::C }));
>+    EXPECT_FALSE(set.containsAll({ ExampleFlags::C, ExampleFlags::D }));
>+    EXPECT_TRUE(set.containsAll({ ExampleFlags::A, ExampleFlags::B }));
>+    EXPECT_FALSE(set.containsAll({ ExampleFlags::B, ExampleFlags::C }));
>+    EXPECT_FALSE(set.containsAll({ ExampleFlags::A, ExampleFlags::C }));
>+    EXPECT_FALSE(set.containsAll({ ExampleFlags::A, ExampleFlags::B, ExampleFlags::C }));
>+}
>+
> } // namespace TestWebKitAPI

Flags:

andersca: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 188501: 347002 | 347019 | 347061