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 };