Changeset 234811 in webkit

Timestamp:

Aug 13, 2018 12:16:10 PM (6 years ago)

Author:

Antti Koivisto

Message:

Meaning of OptionSet::contains is unclear when used with OptionSet argument
​https://bugs.webkit.org/show_bug.cgi?id=188501

Reviewed by Anders Carlsson.

Source/WebCore:

• 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):

Source/WTF:

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.

Tools:

• TestWebKitAPI/Tests/WTF/OptionSet.cpp:

(TestWebKitAPI::TEST):

Location:

trunk

Files:

• Source/WTF/ChangeLog (modified) (1 diff)
• Source/WTF/wtf/OptionSet.h (modified) (1 diff)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/dom/DocumentMarkerController.cpp (modified) (1 diff)
• Source/WebCore/dom/DocumentMarkerController.h (modified) (1 diff)
• Source/WebCore/rendering/RenderElement.cpp (modified) (1 diff)
• Source/WebCore/rendering/RenderLayer.cpp (modified) (1 diff)
• Source/WebKit/WebProcess/Plugins/PluginView.cpp (modified) (1 diff)
• Tools/ChangeLog (modified) (1 diff)
• Tools/TestWebKitAPI/Tests/WTF/OptionSet.cpp (modified) (1 diff)

trunk/Source/WTF/ChangeLog

@@ +1 @@
+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 Anders Carlsson.
+
+        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-13  Commit Queue  <commit-queue@webkit.org>
 

trunk/Source/WTF/wtf/OptionSet.h

@@ -97 +97 @@
     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;
     }
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+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 Anders Carlsson.
+
+        * 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-13  Commit Queue  <commit-queue@webkit.org>
 

trunk/Source/WebCore/dom/DocumentMarkerController.cpp

@@ -45 +45 @@
 inline bool DocumentMarkerController::possiblyHasMarkers(OptionSet<DocumentMarker::MarkerType> types)
 {
-    return m_possiblyExistingMarkerTypes.contains(types);
+    return m_possiblyExistingMarkerTypes.containsAny(types);
 }
 

trunk/Source/WebCore/dom/DocumentMarkerController.h

@@ -64 +64 @@
     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();
     }

trunk/Source/WebCore/rendering/RenderElement.cpp

@@ -1362 +1362 @@
     // 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();
 

trunk/Source/WebCore/rendering/RenderLayer.cpp

@@ -4789 +4789 @@
     // 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);

trunk/Source/WebKit/WebProcess/Plugins/PluginView.cpp

@@ -1848 +1848 @@
 
     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.

trunk/Tools/ChangeLog

@@ +1 @@
+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 Anders Carlsson.
+
+        * TestWebKitAPI/Tests/WTF/OptionSet.cpp:
+        (TestWebKitAPI::TEST):
+
 2018-08-13  Commit Queue  <commit-queue@webkit.org>
 

trunk/Tools/TestWebKitAPI/Tests/WTF/OptionSet.cpp

@@ -380 +380 @@
 }
 
+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