Title: [230773] trunk/Source/WebKit
Revision
230773
Author
bb...@apple.com
Date
2018-04-18 12:17:08 -0700 (Wed, 18 Apr 2018)

Log Message

Unreviewed, rolling out r230743.
https://bugs.webkit.org/show_bug.cgi?id=184747

causes mouse clicks to not work on some platforms (Requested
by brrian on #webkit).

Reverted changeset:

"Web Automation: simulated mouse interactions should not be
done until associated DOM events have been dispatched"
https://bugs.webkit.org/show_bug.cgi?id=184462
https://trac.webkit.org/changeset/230743

Patch by Commit Queue <commit-qu...@webkit.org> on 2018-04-18

Modified Paths

Diff

Modified: trunk/Source/WebKit/ChangeLog (230772 => 230773)


--- trunk/Source/WebKit/ChangeLog	2018-04-18 19:04:07 UTC (rev 230772)
+++ trunk/Source/WebKit/ChangeLog	2018-04-18 19:17:08 UTC (rev 230773)
@@ -1,3 +1,18 @@
+2018-04-18  Commit Queue  <commit-qu...@webkit.org>
+
+        Unreviewed, rolling out r230743.
+        https://bugs.webkit.org/show_bug.cgi?id=184747
+
+        causes mouse clicks to not work on some platforms (Requested
+        by brrian on #webkit).
+
+        Reverted changeset:
+
+        "Web Automation: simulated mouse interactions should not be
+        done until associated DOM events have been dispatched"
+        https://bugs.webkit.org/show_bug.cgi?id=184462
+        https://trac.webkit.org/changeset/230743
+
 2018-04-18  Brent Fulgham  <bfulg...@apple.com>
 
         Avoid crash if ITP Debug mode is on, but ResourceLoadStatistics are not being used

Modified: trunk/Source/WebKit/Platform/Logging.h (230772 => 230773)


--- trunk/Source/WebKit/Platform/Logging.h	2018-04-18 19:04:07 UTC (rev 230772)
+++ trunk/Source/WebKit/Platform/Logging.h	2018-04-18 19:17:08 UTC (rev 230773)
@@ -51,7 +51,6 @@
     M(KeyHandling) \
     M(Layers) \
     M(Loading) \
-    M(MouseHandling) \
     M(Network) \
     M(NetworkCache) \
     M(NetworkCacheSpeculativePreloading) \

Modified: trunk/Source/WebKit/UIProcess/Automation/WebAutomationSession.cpp (230772 => 230773)


--- trunk/Source/WebKit/UIProcess/Automation/WebAutomationSession.cpp	2018-04-18 19:04:07 UTC (rev 230772)
+++ trunk/Source/WebKit/UIProcess/Automation/WebAutomationSession.cpp	2018-04-18 19:17:08 UTC (rev 230773)
@@ -53,15 +53,6 @@
 
 namespace WebKit {
 
-String AutomationCommandError::toProtocolString() const
-{
-    String protocolErrorName = Inspector::Protocol::AutomationHelpers::getEnumConstantValue(type);
-    if (!message.has_value())
-        return protocolErrorName;
-
-    return makeString(protocolErrorName, errorNameAndDetailsSeparator, message.value());
-}
-    
 // ยง8. Sessions
 // https://www.w3.org/TR/webdriver/#dfn-session-page-load-timeout
 static const Seconds defaultPageLoadTimeout = 300_s;
@@ -592,20 +583,6 @@
                 callback->sendFailure(unexpectedAlertOpenError);
             }
         }
-        
-        if (!m_pendingMouseEventsFlushedCallbacksPerPage.isEmpty()) {
-            for (auto key : copyToVector(m_pendingMouseEventsFlushedCallbacksPerPage.keys())) {
-                auto callback = m_pendingMouseEventsFlushedCallbacksPerPage.take(key);
-                callback(std::nullopt);
-            }
-        }
-
-        if (!m_pendingKeyboardEventsFlushedCallbacksPerPage.isEmpty()) {
-            for (auto key : copyToVector(m_pendingKeyboardEventsFlushedCallbacksPerPage.keys())) {
-                auto callback = m_pendingKeyboardEventsFlushedCallbacksPerPage.take(key);
-                callback(std::nullopt);
-            }
-        }
     });
 }
     
@@ -721,23 +698,13 @@
         callback->sendSuccess(JSON::Object::create());
 }
 
-void WebAutomationSession::mouseEventsFlushedForPage(const WebPageProxy& page)
-{
-    if (auto callback = m_pendingMouseEventsFlushedCallbacksPerPage.take(page.pageID())) {
-        if (m_pendingMouseEventsFlushedCallbacksPerPage.isEmpty())
-            m_simulatingUserInteraction = false;
-
-        callback(std::nullopt);
-    }
-}
-
 void WebAutomationSession::keyboardEventsFlushedForPage(const WebPageProxy& page)
 {
     if (auto callback = m_pendingKeyboardEventsFlushedCallbacksPerPage.take(page.pageID())) {
+        callback->sendSuccess(JSON::Object::create());
+
         if (m_pendingKeyboardEventsFlushedCallbacksPerPage.isEmpty())
             m_simulatingUserInteraction = false;
-
-        callback(std::nullopt);
     }
 }
 
@@ -1431,34 +1398,12 @@
         if (!parsedButton)
             ASYNC_FAIL_WITH_PREDEFINED_ERROR_AND_DETAILS(InvalidParameter, "The parameter 'button' is invalid.");
 
-        auto mouseEventsFlushedCallback = [protectedThis = WTFMove(protectedThis), callback = WTFMove(callback), page = page.copyRef(), x, y](std::optional<AutomationCommandError> error) {
-            if (error)
-                callback->sendFailure(error.value().toProtocolString());
-            else {
-                callback->sendSuccess(Inspector::Protocol::Automation::Point::create()
-                    .setX(x)
-                    .setY(y - page->topContentInset())
-                    .release());
-            }
-        };
+        platformSimulateMouseInteraction(page, viewPosition, parsedInteraction.value(), parsedButton.value(), keyModifiers);
 
-        auto& callbackInMap = m_pendingMouseEventsFlushedCallbacksPerPage.add(page->pageID(), nullptr).iterator->value;
-        if (callbackInMap)
-            callbackInMap(AUTOMATION_COMMAND_ERROR_WITH_NAME(Timeout));
-        callbackInMap = WTFMove(mouseEventsFlushedCallback);
-
-        // This is cleared when all mouse events are flushed.
-        m_simulatingUserInteraction = true;
-
-        platformSimulateMouseInteraction(page, viewPosition, parsedInteraction.value(), parsedButton.value(), keyModifiers);
-        
-        // If the event location was previously clipped and does not hit test anything in the window, then it will not be processed.
-        // For compatibility with pre-W3C driver implementations, don't make this a hard error; just do nothing silently.
-        // In W3C-only code paths, we can reject any pointer actions whose coordinates are outside the viewport rect.
-        if (callbackInMap && !page->isProcessingMouseEvents()) {
-            auto callbackToCancel = m_pendingMouseEventsFlushedCallbacksPerPage.take(page->pageID());
-            callbackToCancel(std::nullopt);
-        }
+        callback->sendSuccess(Inspector::Protocol::Automation::Point::create()
+            .setX(x)
+            .setY(y - page->topContentInset())
+            .release());
     });
 #endif // USE(APPKIT) || PLATFORM(GTK)
 }
@@ -1528,17 +1473,10 @@
     if (!actionsToPerform.size())
         ASYNC_FAIL_WITH_PREDEFINED_ERROR_AND_DETAILS(InternalError, "No actions to perform.");
 
-    auto keyboardEventsFlushedCallback = [protectedThis = makeRef(*this), callback = WTFMove(callback), page = makeRef(*page)](std::optional<AutomationCommandError> error) {
-        if (error)
-            callback->sendFailure(error.value().toProtocolString());
-        else
-            callback->sendSuccess();
-    };
-
     auto& callbackInMap = m_pendingKeyboardEventsFlushedCallbacksPerPage.add(page->pageID(), nullptr).iterator->value;
     if (callbackInMap)
-        callbackInMap(AUTOMATION_COMMAND_ERROR_WITH_NAME(Timeout));
-    callbackInMap = WTFMove(keyboardEventsFlushedCallback);
+        callbackInMap->sendFailure(STRING_FOR_PREDEFINED_ERROR_NAME(Timeout));
+    callbackInMap = WTFMove(callback);
 
     // This is cleared when all keyboard events are flushed.
     m_simulatingUserInteraction = true;

Modified: trunk/Source/WebKit/UIProcess/Automation/WebAutomationSession.h (230772 => 230773)


--- trunk/Source/WebKit/UIProcess/Automation/WebAutomationSession.h	2018-04-18 19:04:07 UTC (rev 230772)
+++ trunk/Source/WebKit/UIProcess/Automation/WebAutomationSession.h	2018-04-18 19:17:08 UTC (rev 230773)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2016-2018 Apple Inc. All rights reserved.
+ * Copyright (C) 2016, 2017 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -76,21 +76,6 @@
 class WebPageProxy;
 class WebProcessPool;
 
-struct AutomationCommandError {
-public:
-    AutomationCommandError(Inspector::Protocol::Automation::ErrorMessage type)
-        : type(type) { }
-
-    AutomationCommandError(Inspector::Protocol::Automation::ErrorMessage type, const String& message)
-        : type(type)
-        , message(message) { }
-    
-    String toProtocolString() const;
-
-    Inspector::Protocol::Automation::ErrorMessage type;
-    std::optional<String> message;
-};
-
 class WebAutomationSession final : public API::ObjectImpl<API::Object::Type::AutomationSession>, public IPC::MessageReceiver
 #if ENABLE(REMOTE_INSPECTOR)
     , public Inspector::RemoteAutomationTarget
@@ -113,7 +98,6 @@
     void documentLoadedForFrame(const WebFrameProxy&);
     void inspectorFrontendLoaded(const WebPageProxy&);
     void keyboardEventsFlushedForPage(const WebPageProxy&);
-    void mouseEventsFlushedForPage(const WebPageProxy&);
     void willClosePage(const WebPageProxy&);
     void handleRunOpenPanel(const WebPageProxy&, const WebFrameProxy&, const API::OpenPanelParameters&, WebOpenPanelResultListenerProxy&);
     void willShowJavaScriptDialog(WebPageProxy&);
@@ -251,8 +235,7 @@
     HashMap<uint64_t, RefPtr<Inspector::BackendDispatcher::CallbackBase>> m_pendingNormalNavigationInBrowsingContextCallbacksPerFrame;
     HashMap<uint64_t, RefPtr<Inspector::BackendDispatcher::CallbackBase>> m_pendingEagerNavigationInBrowsingContextCallbacksPerFrame;
     HashMap<uint64_t, RefPtr<Inspector::BackendDispatcher::CallbackBase>> m_pendingInspectorCallbacksPerPage;
-    HashMap<uint64_t, Function<void(std::optional<AutomationCommandError>)>> m_pendingKeyboardEventsFlushedCallbacksPerPage;
-    HashMap<uint64_t, Function<void(std::optional<AutomationCommandError>)>> m_pendingMouseEventsFlushedCallbacksPerPage;
+    HashMap<uint64_t, RefPtr<Inspector::BackendDispatcher::CallbackBase>> m_pendingKeyboardEventsFlushedCallbacksPerPage;
 
     uint64_t m_nextEvaluateJavaScriptCallbackID { 1 };
     HashMap<uint64_t, RefPtr<Inspector::AutomationBackendDispatcherHandler::EvaluateJavaScriptFunctionCallback>> m_evaluateJavaScriptFunctionCallbacks;

Modified: trunk/Source/WebKit/UIProcess/Automation/WebAutomationSessionMacros.h (230772 => 230773)


--- trunk/Source/WebKit/UIProcess/Automation/WebAutomationSessionMacros.h	2018-04-18 19:04:07 UTC (rev 230772)
+++ trunk/Source/WebKit/UIProcess/Automation/WebAutomationSessionMacros.h	2018-04-18 19:17:08 UTC (rev 230773)
@@ -38,8 +38,6 @@
 #define STRING_FOR_PREDEFINED_ERROR_MESSAGE(errorMessage) Inspector::Protocol::AutomationHelpers::getEnumConstantValue(VALIDATED_ERROR_MESSAGE(errorMessage))
 #define STRING_FOR_PREDEFINED_ERROR_MESSAGE_AND_DETAILS(errorMessage, detailsString) makeString(Inspector::Protocol::AutomationHelpers::getEnumConstantValue(VALIDATED_ERROR_MESSAGE(errorMessage)), errorNameAndDetailsSeparator, detailsString)
 
-#define AUTOMATION_COMMAND_ERROR_WITH_NAME(errorName) AutomationCommandError(Inspector::Protocol::Automation::ErrorMessage::errorName)
-
 // Convenience macros for filling in the error string of synchronous commands in bailout branches.
 #define SYNC_FAIL_WITH_PREDEFINED_ERROR(errorName) \
 do { \

Modified: trunk/Source/WebKit/UIProcess/WebPageProxy.cpp (230772 => 230773)


--- trunk/Source/WebKit/UIProcess/WebPageProxy.cpp	2018-04-18 19:04:07 UTC (rev 230772)
+++ trunk/Source/WebKit/UIProcess/WebPageProxy.cpp	2018-04-18 19:17:08 UTC (rev 230773)
@@ -293,32 +293,21 @@
 }
 
 #if !LOG_DISABLED
-static const char* webMouseEventTypeString(WebEvent::Type type)
-{
-    switch (type) {
-    case WebEvent::MouseDown:
-        return "MouseDown";
-    case WebEvent::MouseUp:
-        return "MouseUp";
-    case WebEvent::MouseMove:
-        return "MouseMove";
-    default:
-        ASSERT_NOT_REACHED();
-        return "<unknown>";
-    }
-}
-
 static const char* webKeyboardEventTypeString(WebEvent::Type type)
 {
     switch (type) {
     case WebEvent::KeyDown:
         return "KeyDown";
+    
     case WebEvent::KeyUp:
         return "KeyUp";
+    
     case WebEvent::RawKeyDown:
         return "RawKeyDown";
+    
     case WebEvent::Char:
         return "Char";
+    
     default:
         ASSERT_NOT_REACHED();
         return "<unknown>";
@@ -1911,20 +1900,6 @@
     if (!isValid())
         return;
 
-    m_mouseEventQueue.append(event);
-    if (m_mouseEventQueue.size() == 1) // Otherwise, called from DidReceiveEvent message handler.
-        processNextQueuedMouseEvent();
-}
-    
-void WebPageProxy::processNextQueuedMouseEvent()
-{
-    if (!isValid())
-        return;
-
-    ASSERT(!m_mouseEventQueue.isEmpty());
-
-    const NativeWebMouseEvent& event = m_mouseEventQueue.first();
-    
     if (m_pageClient.windowIsFrontWindowUnderMouse(event))
         setToolTip(String());
 
@@ -1931,8 +1906,22 @@
     // NOTE: This does not start the responsiveness timer because mouse move should not indicate interaction.
     if (event.type() != WebEvent::MouseMove)
         m_process->responsivenessTimer().start();
+    else {
+        if (m_processingMouseMoveEvent) {
+            m_nextMouseMoveEvent = std::make_unique<NativeWebMouseEvent>(event);
+            return;
+        }
 
-    LOG(MouseHandling, " UI process: sent mouseEvent from handleMouseEvent");
+        m_processingMouseMoveEvent = true;
+    }
+
+    // <https://bugs.webkit.org/show_bug.cgi?id=57904> We need to keep track of the mouse down event in the case where we
+    // display a popup menu for select elements. When the user changes the selected item,
+    // we fake a mouse up event by using this stored down event. This event gets cleared
+    // when the mouse up message is received from WebProcess.
+    if (event.type() == WebEvent::MouseDown)
+        m_currentlyProcessedMouseDownEvent = std::make_unique<NativeWebMouseEvent>(event);
+
     m_process->send(Messages::WebPage::MouseEvent(event), m_pageID);
 }
 
@@ -4839,26 +4828,9 @@
     m_process->send(Messages::WebPage::SetTextForActivePopupMenu(index), m_pageID);
 }
 
-bool WebPageProxy::isProcessingMouseEvents() const
-{
-    return !m_mouseEventQueue.isEmpty();
-}
-
 NativeWebMouseEvent* WebPageProxy::currentlyProcessedMouseDownEvent()
 {
-    // <https://bugs.webkit.org/show_bug.cgi?id=57904> We need to keep track of the mouse down event in the case where we
-    // display a popup menu for select elements. When the user changes the selected item, we fake a mouseup event by
-    // using this stored mousedown event and changing the event type. This trickery happens when WebProcess handles
-    // a mousedown event that runs the default handler for HTMLSelectElement, so the triggering mousedown must be the first event.
-
-    if (m_mouseEventQueue.isEmpty())
-        return nullptr;
-    
-    auto& event = m_mouseEventQueue.first();
-    if (event.type() != WebEvent::Type::MouseDown)
-        return nullptr;
-
-    return &event;
+    return m_currentlyProcessedMouseDownEvent.get();
 }
 
 void WebPageProxy::postMessageToInjectedBundle(const String& messageName, API::Object* messageBody)
@@ -5267,24 +5239,15 @@
     case WebEvent::NoType:
         break;
     case WebEvent::MouseMove:
+        m_processingMouseMoveEvent = false;
+        if (m_nextMouseMoveEvent)
+            handleMouseEvent(*std::exchange(m_nextMouseMoveEvent, nullptr));
+        break;
     case WebEvent::MouseDown:
-    case WebEvent::MouseUp: {
-        LOG(MouseHandling, "WebPageProxy::didReceiveEvent: %s (queue empty %d)", webMouseEventTypeString(type), m_mouseEventQueue.isEmpty());
-
-        // Retire the last sent event now that WebProcess is done handling it.
-        MESSAGE_CHECK(!m_mouseEventQueue.isEmpty());
-        NativeWebMouseEvent event = m_mouseEventQueue.takeFirst();
-        MESSAGE_CHECK(type == event.type());
-
-        if (!m_mouseEventQueue.isEmpty()) {
-            LOG(MouseHandling, " UI process: handling a queued mouse event from didReceiveEvent");
-            processNextQueuedMouseEvent();
-        } else if (auto* automationSession = process().processPool().automationSession())
-            automationSession->mouseEventsFlushedForPage(*this);
-
         break;
-    }
-
+    case WebEvent::MouseUp:
+        m_currentlyProcessedMouseDownEvent = nullptr;
+        break;
     case WebEvent::MouseForceChanged:
     case WebEvent::MouseForceDown:
     case WebEvent::MouseForceUp:
@@ -5317,8 +5280,7 @@
 
         MESSAGE_CHECK(type == event.type());
 
-        bool canProcessMoreKeyEvents = !m_keyEventQueue.isEmpty();
-        if (canProcessMoreKeyEvents) {
+        if (!m_keyEventQueue.isEmpty()) {
             LOG(KeyHandling, " UI process: sent keyEvent from didReceiveEvent");
             m_process->send(Messages::WebPage::KeyEvent(m_keyEventQueue.first()), m_pageID);
         }
@@ -5332,7 +5294,7 @@
             m_uiClient->didNotHandleKeyEvent(this, event);
 
         // Notify the session after -[NSApp sendEvent:] has a crack at turning the event into an action.
-        if (!canProcessMoreKeyEvents) {
+        if (m_keyEventQueue.isEmpty()) {
             if (auto* automationSession = process().processPool().automationSession())
                 automationSession->keyboardEventsFlushedForPage(*this);
         }
@@ -5936,10 +5898,15 @@
     m_pendingLearnOrIgnoreWordMessageCount = 0;
 
     // Can't expect DidReceiveEvent notifications from a crashed web process.
-    m_mouseEventQueue.clear();
     m_keyEventQueue.clear();
     m_wheelEventQueue.clear();
     m_currentlyProcessedWheelEvents.clear();
+
+    m_nextMouseMoveEvent = nullptr;
+    m_currentlyProcessedMouseDownEvent = nullptr;
+
+    m_processingMouseMoveEvent = false;
+
 #if ENABLE(TOUCH_EVENTS) && !ENABLE(IOS_TOUCH_EVENTS)
     m_touchEventQueue.clear();
 #endif

Modified: trunk/Source/WebKit/UIProcess/WebPageProxy.h (230772 => 230773)


--- trunk/Source/WebKit/UIProcess/WebPageProxy.h	2018-04-18 19:04:07 UTC (rev 230772)
+++ trunk/Source/WebKit/UIProcess/WebPageProxy.h	2018-04-18 19:17:08 UTC (rev 230773)
@@ -685,10 +685,7 @@
     void setBackgroundColor(const WebCore::Color& color) { m_backgroundColor = color; }
 #endif
 
-    bool isProcessingMouseEvents() const;
-    void processNextQueuedMouseEvent();
     void handleMouseEvent(const NativeWebMouseEvent&);
-
     void handleWheelEvent(const NativeWebWheelEvent&);
     void handleKeyboardEvent(const NativeWebKeyboardEvent&);
 
@@ -1938,7 +1935,6 @@
     WebCore::ResourceRequest m_decidePolicyForResponseRequest;
     bool m_shouldSuppressAppLinksInNextNavigationPolicyDecision { false };
 
-    Deque<NativeWebMouseEvent> m_mouseEventQueue;
     Deque<NativeWebKeyboardEvent> m_keyEventQueue;
     Deque<NativeWebWheelEvent> m_wheelEventQueue;
     Deque<std::unique_ptr<Vector<NativeWebWheelEvent>>> m_currentlyProcessedWheelEvents;
@@ -1946,6 +1942,10 @@
     Deque<NativeWebGestureEvent> m_gestureEventQueue;
 #endif
 
+    bool m_processingMouseMoveEvent { false };
+    std::unique_ptr<NativeWebMouseEvent> m_nextMouseMoveEvent;
+    std::unique_ptr<NativeWebMouseEvent> m_currentlyProcessedMouseDownEvent;
+
 #if ENABLE(TOUCH_EVENTS)
     struct TouchEventTracking {
         WebCore::TrackingType touchForceChangedTracking { WebCore::TrackingType::NotTracking };
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to