WebDriver: W3C test case actions/key.py::test_lone_keyup_sends_no_events is failing
authorbburg@apple.com <bburg@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 14 May 2018 19:09:12 +0000 (19:09 +0000)
committerbburg@apple.com <bburg@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 14 May 2018 19:09:12 +0000 (19:09 +0000)
https://bugs.webkit.org/show_bug.cgi?id=185577
<rdar://problem/40185478>

Reviewed by Timothy Hatcher.

This test is failing because it expects Release Actions to not emit any
events if nothing has changed from the initial state. Because the two code paths
for creating empty states don't actually produce the same empty state, a difference
in location was detected between the two empty states. This generates a mousemove.

To fix this, unify the code that creates an empty state. For mouse input sources, always
initialize the location to (0, 0) so that the mouse input source always has
a location that is valid to click at.

* UIProcess/Automation/SimulatedInputDispatcher.h:
Extract the type enum out of the class to avoid circular definitions of
SimulatedInputSource and SimulatedInputSourceState.

* UIProcess/Automation/SimulatedInputDispatcher.cpp:
(WebKit::SimulatedInputSourceState::emptyStateForSourceType):
Take the input source type when generating an empty state. We always want location
set for a mouse input source, but not set it for other input sources like keys.

(WebKit::SimulatedInputKeyFrame::keyFrameToResetInputSources):
(WebKit::SimulatedInputDispatcher::transitionInputSourceToState):
(WebKit::SimulatedInputSource::create):
(WebKit::SimulatedInputSource::SimulatedInputSource):
(WebKit::SimulatedInputSourceState::emptyState): Deleted.
* UIProcess/Automation/WebAutomationSession.cpp:
(WebKit::WebAutomationSession::WebAutomationSession):
(WebKit::WebAutomationSession::inputSourceForType const):
(WebKit::simulatedInputSourceTypeFromProtocolSourceType):
(WebKit::WebAutomationSession::performInteractionSequence):
* UIProcess/Automation/WebAutomationSession.h:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@231767 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebKit/ChangeLog
Source/WebKit/UIProcess/Automation/SimulatedInputDispatcher.cpp
Source/WebKit/UIProcess/Automation/SimulatedInputDispatcher.h
Source/WebKit/UIProcess/Automation/WebAutomationSession.cpp
Source/WebKit/UIProcess/Automation/WebAutomationSession.h

index 6cd2a3e..37351f5 100644 (file)
@@ -1,3 +1,41 @@
+2018-05-14  Brian Burg  <bburg@apple.com>
+
+        WebDriver: W3C test case actions/key.py::test_lone_keyup_sends_no_events is failing
+        https://bugs.webkit.org/show_bug.cgi?id=185577
+        <rdar://problem/40185478>
+
+        Reviewed by Timothy Hatcher.
+
+        This test is failing because it expects Release Actions to not emit any
+        events if nothing has changed from the initial state. Because the two code paths
+        for creating empty states don't actually produce the same empty state, a difference
+        in location was detected between the two empty states. This generates a mousemove.
+
+        To fix this, unify the code that creates an empty state. For mouse input sources, always
+        initialize the location to (0, 0) so that the mouse input source always has
+        a location that is valid to click at.
+
+        * UIProcess/Automation/SimulatedInputDispatcher.h:
+        Extract the type enum out of the class to avoid circular definitions of
+        SimulatedInputSource and SimulatedInputSourceState.
+
+        * UIProcess/Automation/SimulatedInputDispatcher.cpp:
+        (WebKit::SimulatedInputSourceState::emptyStateForSourceType):
+        Take the input source type when generating an empty state. We always want location
+        set for a mouse input source, but not set it for other input sources like keys.
+
+        (WebKit::SimulatedInputKeyFrame::keyFrameToResetInputSources):
+        (WebKit::SimulatedInputDispatcher::transitionInputSourceToState):
+        (WebKit::SimulatedInputSource::create):
+        (WebKit::SimulatedInputSource::SimulatedInputSource):
+        (WebKit::SimulatedInputSourceState::emptyState): Deleted.
+        * UIProcess/Automation/WebAutomationSession.cpp:
+        (WebKit::WebAutomationSession::WebAutomationSession):
+        (WebKit::WebAutomationSession::inputSourceForType const):
+        (WebKit::simulatedInputSourceTypeFromProtocolSourceType):
+        (WebKit::WebAutomationSession::performInteractionSequence):
+        * UIProcess/Automation/WebAutomationSession.h:
+
 2018-05-14  Michael Catanzaro  <mcatanzaro@igalia.com>
 
         -Wmemset-elt-size warning in LibWebRTCSocket constructor
index f47174b..912118c 100644 (file)
 
 namespace WebKit {
 
+SimulatedInputSourceState SimulatedInputSourceState::emptyStateForSourceType(SimulatedInputSourceType type)
+{
+    SimulatedInputSourceState result { };
+    switch (type) {
+    case SimulatedInputSourceType::Null:
+    case SimulatedInputSourceType::Keyboard:
+        break;
+    case SimulatedInputSourceType::Mouse:
+    case SimulatedInputSourceType::Touch:
+        result.location = WebCore::IntPoint();
+    }
+
+    return result;
+}
+
+
 SimulatedInputKeyFrame::SimulatedInputKeyFrame(Vector<StateEntry>&& entries)
     : states(WTFMove(entries))
 {
@@ -66,12 +82,8 @@ SimulatedInputKeyFrame SimulatedInputKeyFrame::keyFrameToResetInputSources(HashS
     Vector<SimulatedInputKeyFrame::StateEntry> entries;
     entries.reserveCapacity(inputSources.size());
 
-    for (auto& inputSource : inputSources) {
-        auto emptyState = SimulatedInputSourceState::emptyState();
-        // Ensure we reset the location.
-        emptyState.location = WebCore::IntPoint();
-        entries.uncheckedAppend(std::pair<SimulatedInputSource&, SimulatedInputSourceState> { inputSource.get(), WTFMove(emptyState) });
-    }
+    for (auto& inputSource : inputSources)
+        entries.uncheckedAppend(std::pair<SimulatedInputSource&, SimulatedInputSourceState> { inputSource.get(), SimulatedInputSourceState::emptyStateForSourceType(inputSource->type) });
 
     return SimulatedInputKeyFrame(WTFMove(entries));
 }
@@ -228,11 +240,11 @@ void SimulatedInputDispatcher::transitionInputSourceToState(SimulatedInputSource
     };
 
     switch (inputSource.type) {
-    case SimulatedInputSource::Type::Null:
+    case SimulatedInputSourceType::Null:
         // The maximum duration is handled at the keyframe level by m_keyFrameTransitionDurationTimer.
         eventDispatchFinished(std::nullopt);
         break;
-    case SimulatedInputSource::Type::Mouse: {
+    case SimulatedInputSourceType::Mouse: {
         resolveLocation(a.location.value_or(WebCore::IntPoint()), b.location, b.origin.value_or(MouseMoveOrigin::Viewport), b.nodeHandle, [this, &a, &b, eventDispatchFinished = WTFMove(eventDispatchFinished)](std::optional<WebCore::IntPoint> location, std::optional<AutomationCommandError> error) mutable {
             if (error) {
                 eventDispatchFinished(error);
@@ -253,7 +265,7 @@ void SimulatedInputDispatcher::transitionInputSourceToState(SimulatedInputSource
         });
         break;
     }
-    case SimulatedInputSource::Type::Keyboard:
+    case SimulatedInputSourceType::Keyboard:
         // The "dispatch a key{Down,Up} action" algorithms (§17.4 Dispatching Actions).
         if ((!a.pressedCharKey && b.pressedCharKey) || (!a.pressedVirtualKey && b.pressedVirtualKey))
             m_client.simulateKeyboardInteraction(m_page, KeyboardInteraction::KeyPress, b.pressedVirtualKey, b.pressedCharKey, WTFMove(eventDispatchFinished));
@@ -262,7 +274,7 @@ void SimulatedInputDispatcher::transitionInputSourceToState(SimulatedInputSource
         else
             eventDispatchFinished(std::nullopt);
         break;
-    case SimulatedInputSource::Type::Touch:
+    case SimulatedInputSourceType::Touch:
         // Not supported yet.
         ASSERT_NOT_REACHED();
         eventDispatchFinished(AUTOMATION_COMMAND_ERROR_WITH_NAME(NotImplemented));
index d04cdde..e451ebb 100644 (file)
@@ -57,6 +57,13 @@ using MouseButton = WebMouseEvent::Button;
 using MouseInteraction = Inspector::Protocol::Automation::MouseInteraction;
 using MouseMoveOrigin = Inspector::Protocol::Automation::MouseMoveOrigin;
 
+enum class SimulatedInputSourceType {
+    Null, // Used to induce a minimum duration.
+    Keyboard,
+    Mouse,
+    Touch,
+};
+
 struct SimulatedInputSourceState {
     std::optional<CharKey> pressedCharKey;
     std::optional<VirtualKey> pressedVirtualKey;
@@ -66,32 +73,25 @@ struct SimulatedInputSourceState {
     std::optional<WebCore::IntPoint> location;
     std::optional<Seconds> duration;
 
-    static SimulatedInputSourceState emptyState() { return SimulatedInputSourceState(); }
+    static SimulatedInputSourceState emptyStateForSourceType(SimulatedInputSourceType);
 };
 
 struct SimulatedInputSource : public RefCounted<SimulatedInputSource> {
 public:
-    enum class Type {
-        Null, // Used to induce a minimum duration.
-        Keyboard,
-        Mouse,
-        Touch,
-    };
-
-    Type type;
+    SimulatedInputSourceType type;
 
     // The last state associated with this input source.
     SimulatedInputSourceState state;
 
-    static Ref<SimulatedInputSource> create(Type type)
+    static Ref<SimulatedInputSource> create(SimulatedInputSourceType type)
     {
         return adoptRef(*new SimulatedInputSource(type));
     }
 
 private:
-    SimulatedInputSource(Type type)
+    SimulatedInputSource(SimulatedInputSourceType type)
         : type(type)
-        , state(SimulatedInputSourceState::emptyState())
+        , state(SimulatedInputSourceState::emptyStateForSourceType(type))
     { }
 };
 
index 83c9fc2..b9b4ddc 100644 (file)
@@ -78,9 +78,9 @@ WebAutomationSession::WebAutomationSession()
     , m_loadTimer(RunLoop::main(), this, &WebAutomationSession::loadTimerFired)
 {
     // Set up canonical input sources to be used for 'performInteractionSequence' and 'cancelInteractionSequence'.
-    m_inputSources.add(SimulatedInputSource::create(SimulatedInputSource::Type::Mouse));
-    m_inputSources.add(SimulatedInputSource::create(SimulatedInputSource::Type::Keyboard));
-    m_inputSources.add(SimulatedInputSource::create(SimulatedInputSource::Type::Null));
+    m_inputSources.add(SimulatedInputSource::create(SimulatedInputSourceType::Mouse));
+    m_inputSources.add(SimulatedInputSource::create(SimulatedInputSourceType::Keyboard));
+    m_inputSources.add(SimulatedInputSource::create(SimulatedInputSourceType::Null));
 }
 
 WebAutomationSession::~WebAutomationSession()
@@ -1404,7 +1404,7 @@ SimulatedInputDispatcher& WebAutomationSession::inputDispatcherForPage(WebPagePr
     }).iterator->value;
 }
 
-SimulatedInputSource* WebAutomationSession::inputSourceForType(SimulatedInputSource::Type type) const
+SimulatedInputSource* WebAutomationSession::inputSourceForType(SimulatedInputSourceType type) const
 {
     // FIXME: this should use something like Vector's findMatching().
     for (auto& inputSource : m_inputSources) {
@@ -1668,17 +1668,17 @@ void WebAutomationSession::performKeyboardInteractions(const String& handle, con
 }
 
 #if USE(APPKIT) || PLATFORM(GTK)
-static SimulatedInputSource::Type simulatedInputSourceTypeFromProtocolSourceType(Inspector::Protocol::Automation::InputSourceType protocolType)
+static SimulatedInputSourceType simulatedInputSourceTypeFromProtocolSourceType(Inspector::Protocol::Automation::InputSourceType protocolType)
 {
     switch (protocolType) {
     case Inspector::Protocol::Automation::InputSourceType::Null:
-        return SimulatedInputSource::Type::Null;
+        return SimulatedInputSourceType::Null;
     case Inspector::Protocol::Automation::InputSourceType::Keyboard:
-        return SimulatedInputSource::Type::Keyboard;
+        return SimulatedInputSourceType::Keyboard;
     case Inspector::Protocol::Automation::InputSourceType::Mouse:
-        return SimulatedInputSource::Type::Mouse;
+        return SimulatedInputSourceType::Mouse;
     case Inspector::Protocol::Automation::InputSourceType::Touch:
-        return SimulatedInputSource::Type::Touch;
+        return SimulatedInputSourceType::Touch;
     }
 
     RELEASE_ASSERT_NOT_REACHED();
@@ -1701,7 +1701,7 @@ void WebAutomationSession::performInteractionSequence(const String& handle, cons
         ASYNC_FAIL_WITH_PREDEFINED_ERROR(FrameNotFound);
 
     HashMap<String, Ref<SimulatedInputSource>> sourceIdToInputSourceMap;
-    HashMap<SimulatedInputSource::Type, String, WTF::IntHash<SimulatedInputSource::Type>, WTF::StrongEnumHashTraits<SimulatedInputSource::Type>> typeToSourceIdMap;
+    HashMap<SimulatedInputSourceType, String, WTF::IntHash<SimulatedInputSourceType>, WTF::StrongEnumHashTraits<SimulatedInputSourceType>> typeToSourceIdMap;
 
     // Parse and validate Automation protocol arguments. By this point, the driver has
     // already performed the steps in §17.3 Processing Actions Requests.
@@ -1725,8 +1725,8 @@ void WebAutomationSession::performInteractionSequence(const String& handle, cons
         if (!parsedInputSourceType)
             ASYNC_FAIL_WITH_PREDEFINED_ERROR_AND_DETAILS(InvalidParameter, "An input source in the 'inputSources' parameter has an invalid 'sourceType'.");
 
-        SimulatedInputSource::Type inputSourceType = simulatedInputSourceTypeFromProtocolSourceType(*parsedInputSourceType);
-        if (inputSourceType == SimulatedInputSource::Type::Touch)
+        SimulatedInputSourceType inputSourceType = simulatedInputSourceTypeFromProtocolSourceType(*parsedInputSourceType);
+        if (inputSourceType == SimulatedInputSourceType::Touch)
             ASYNC_FAIL_WITH_PREDEFINED_ERROR_AND_DETAILS(NotImplemented, "Touch input sources are not yet supported.");
 
         if (typeToSourceIdMap.contains(inputSourceType))
index bd24ddc..3ffe3b3 100644 (file)
@@ -188,7 +188,7 @@ public:
     // Event Simulation Support.
     bool isSimulatingUserInteraction() const;
     SimulatedInputDispatcher& inputDispatcherForPage(WebPageProxy&);
-    SimulatedInputSource* inputSourceForType(SimulatedInputSource::Type) const;
+    SimulatedInputSource* inputSourceForType(SimulatedInputSourceType) const;
 
 #if PLATFORM(MAC)
     bool wasEventSynthesizedForAutomation(NSEvent *);